-
-
Notifications
You must be signed in to change notification settings - Fork 192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add new patch 0187-WebUI-UpgradeNewDeviceDialog #2824
base: master
Are you sure you want to change the base?
Add new patch 0187-WebUI-UpgradeNewDeviceDialog #2824
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Zustätzlich zu den Anmerkungen kannst du sicher unter "Checks" sehen das deine Änderungen leider mit vorherigen WebUI patches kollidieren. D.h. du änderst Dinge (in konkret der selben Zeile) die andere WebUI patches auch schon so oder ähnlich angepasst haben. Du musst das also bitte bereinigen in dem du in den anderen WebUI Patches suchst welche noch diese Dateien anpassen und ggf. in der selben Zeile änderungen vornehmen wie du auch. Und dann musst du dein WebUI Patch darauf anpassen. So das Prozedere.
Konkret scheint der auch über unterschiedliche "Line-Endings" zu motzen. D.h. du nutzt in deinem Editor ggf. CRLF Line-Endings und die Datei hat nur CR oder umgedreht. Siehe:
buildroot-external/patches/occu/0187-WebUI-UpgradeNewDeviceDialog.patch
Outdated
Show resolved
Hide resolved
buildroot-external/patches/occu/0187-WebUI-UpgradeNewDeviceDialog.patch
Outdated
Show resolved
Hide resolved
Fix german code, optimise text und resolve EOF-Problem
An welcher Stelle im Log finde ich diese Info? Ich habe das jetzt drei mal überflogen und nichts gefunden.
die cp_add_device.cgi ist sowohl im Original als auch in der geänderten Version bei mir CR-LF-Terminiert. |
Nun, im Grunde ist das ganze Prozedere eigentlich ganz einfach. Die patches im WebUI patch verzeichnis werden nacheinander abgearbeitet bzw. auf die originaldatei aus OCCU angewendet. Das bedeutet konkret, das wenn z.B. Patch 0001 in der Datei X in Zeile 1 etwas ändert und dann Patch 0002 danach in der selben Datei X in der näheren Umgebung von Zeile 1 etwas anpasst (weil es z.b. in der selben oder einer nahen Zeile etwas hinzufügen/ändern möchte), dann kommt es eben zu diesen Patch bzw. Merge Konflikten wie sie das Check Tool während des CI Builds auch ausgibt. Dann muss man im Grunde so vorgehen, dass man die Datei X in Patch 0002 nicht direkt aus OCCU als Originaldatei nimmt, sondern die Datei die nach dem Patch 0001 entsteht als "original" Datei des Patches 0002 nutzt. D.h. man patcht dann eben nicht dir originaldatei die man direkt aus OCCU nimmt, sondern die die nach Patch 0001 alleine erzeugt wird. Und das bezieht sich eben auf all die Patches die es da so gibt. Die werden alle nacheinander in der Reihenfolge Ihrer aufsteigenden Nummer angewendet und daher muss man nun im Grunde danach suchen welche vorherige patch die selbe Datei in der selben (oder ähnliches) Zeile etwas editiert und dann die gepatchte Datei als original datei des neuen Patches dann nutzen – oder eben seine änderungen/hinzufügungen in einer anderen Zeile stattfinden lassen. Hoffe das erklärt es etwas besser und bringt Klarheit ins Dunkle. Und wenn du am Schluss ähnlich wie der CI Build lokal testen willst ob dein neuer patch "clean" angewendet werden kann bevor du ihn hier als PR hinschickst, so kannst du im ausgecheckten RaspberryMatic git verzeichnis einfach
Hoffe nach der obigen Erklärung ist es nun etwas klarer wieso das ganze so ist wie es ist und auf was man da so achten muss bei der Entwicklung eines WebUI Patches. Und der Hintergrund des ganzen ist einfach, das man prinzipiell die Änderungen irgendwann leichter "upstream" – d.h. zu eQ3 schicken kann und diese dann leichter integriert werden können (auch wenn das leider wie die Erfahrung zeigt vermutlich nie passieren wird :) |
Ja so hatte ich da "überpatchen" auch gemeint und auch so umgesetzt. Meine Originaldateien stammen aus dem letzten Release und wenn ich mir die Commits seit dem anschaue, war da niemand mehr dran. Also ist das m.E. der aktuellste Stand der Datei, bevor der Patch angewendet werden soll. Ich finde in dem Log auch keinen Hinweis, dass er eine Zeile nicht finden konnte etc. Sondern lediglich das Problem mit den LineEndings, was ich aber jetzt nicht auf eine falsch augewählte Quelldatei zurückführe.
Ja, leider aber nur minimal. Ich gucke, dass ich mir nochmal ein neues System aufziehe und die LineEndings kontrolliere. Nicht, dass sich da etwas durch die SFTP-Übertragung an den Orginaldatei geändert hat und ich mit dem falschen Soll-Zustand abgleiche. |
Ok, hab mal versucht die Error ausgaben zu debuggen die die CI Ausgabe zeigt. Hier lokal lässt sich dein patch aber anwenden. Schau mir das gerade an und melde mich nochmal. |
Also es war in der Tat das falsche Line Ending in den Nun hab ich den PR hier mal selbst ausprobiert um zu schauen wie du die QR Code einlesefunktion gestaltet hast und leider muss ich sagen bin ich davon aktuell nicht so angetan. Kannst du mir mal bitte den genauen Use Case bzw. die Arbeitsweise des ablaufes der Eingabe des QR Codes hier hinterlegen, denn mit z.B. einem iOS Device seh ich da lediglich das da jetzt ein zusätzlich text-eingabe string existiert und man ja erstmal ein tool bräuchte um den QR Code in den text string zu konvertieren. Und ehrlich gesagt dachte ich, du hast sowas ähnliches wie ein html5 basiertes QRCode einscan Widget hinzugefügt oder ähnliches. Siehe z.B. hier für solch ein Element das man nutzen könnte: https://github.com/mebjas/html5-qrcode Alles andere halte ich ehrlich für keine so gute und intuitive Idee, wenn man da jetzt einfach nur den QRCode als umgewandelter Text irgendwie eingeben muss. Das ist nicht wirklich praktikabel IMHO. Und es müsste so so ähnliche wie in dem "html5-qrcode" widget laufen damit das auch für Otto-Normal-Verbraucher besser/intuitiver nutzbar ist. |
Description
Upgrade of NewDeviceDialog to use an QR-Code-Scanner for teaching new devices in offline-mode.
Related Issue
A possible solution for #2715
Types of changes
Alternate Designs
Possible Drawbacks
Verification Process
Release Notes
Contributing checklist