Skip to content
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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Maik2208
Copy link
Contributor

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

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Alternate Designs

Possible Drawbacks

Verification Process

Release Notes

Contributing checklist

  • My code follows the code style of this project.
  • I have read the CONTRIBUTING and LICENSE document.
  • I fully agree to distribute my changes under Apache 2.0 license.

Copy link
Owner

@jens-maus jens-maus left a 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:

Bildschirmfoto 2024-08-14 um 10 07 52

Fix german code, optimise text und resolve EOF-Problem
@Maik2208
Copy link
Contributor Author

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.

An welcher Stelle im Log finde ich diese Info? Ich habe das jetzt drei mal überflogen und nichts gefunden.
Muss ich dann meine Änderung auf zwei Patches aufteilen oder komplett in den vorherigen Patch integrieren?
Auch verstehe ich noch nicht so ganz wieso. Also warum kann ich keine gepatchte Zeile überpatchen?

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:

die cp_add_device.cgi ist sowohl im Original als auch in der geänderten Version bei mir CR-LF-Terminiert.
Hab dann in der GitGui die Meldung "this diff contains a change in line endings from 'lf' to 'crlf'." gefunden und mit Hilfe von Google abstellen können. Geholfen hat es offensichtlich nicht. Ich verstehe es einfach nicht - sorry!!

@jens-maus
Copy link
Owner

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.

An welcher Stelle im Log finde ich diese Info? Ich habe das jetzt drei mal überflogen und nichts gefunden. Muss ich dann meine Änderung auf zwei Patches aufteilen oder komplett in den vorherigen Patch integrieren? Auch verstehe ich noch nicht so ganz wieso. Also warum kann ich keine gepatchte Zeile überpatchen?

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 make check eingeben und dann sollte genau der selbst Check passieren - d.h. er versucht die patches dann nacheinander anzuwenden und zu prüfen ob es irgendwelche Konflikte gibt. Und erst wenn das klappt dann solltest du den PR hier her schicken zum finalen prüfen lassen.

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:

die cp_add_device.cgi ist sowohl im Original als auch in der geänderten Version bei mir CR-LF-Terminiert. Hab dann in der GitGui die Meldung "this diff contains a change in line endings from 'lf' to 'crlf'." gefunden und mit Hilfe von Google abstellen können. Geholfen hat es offensichtlich nicht. Ich verstehe es einfach nicht - sorry!!

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 :)

@Maik2208
Copy link
Contributor Author

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.

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.

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.

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.

@jens-maus
Copy link
Owner

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.

@jens-maus jens-maus added the 💡 enhancement-ideas New feature or change request label Aug 17, 2024
@jens-maus jens-maus added this to the next release milestone Aug 17, 2024
@jens-maus
Copy link
Owner

Also es war in der Tat das falsche Line Ending in den cp_add_device.cgi Dateien die du in dem Patch hinterlegt hattest – man muss sicherstellen das das exakt selbe Line ending verwendet wird wie die Dateien die im OCCU liegen. Du hattest du aber (vmtl. weil dein editor die automatisch umwandelt) mit Unix (CR) line ending hinterlegt, das original hat aber leider CRLF (DOS) line endings und das muss man beibehalten und ja, leider ist in OCCU manchmal Unix und manchmal DOS line endings in verwendung. Da ist eQ3 leider nicht ganz konsequent in der Entwicklung.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💡 enhancement-ideas New feature or change request
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

2 participants