hoylabs / OpenDTU-OnBattery

Software for ESP32 to talk to Hoymiles/TSUN/Solenso Inverters, VE.Direct devices, battery management systems, and related peripherals
GNU General Public License v2.0
301 stars 63 forks source link

Add webapp autocompile #1193

Closed spcqike closed 1 month ago

spcqike commented 1 month ago

da das Thema "fehlende webapp" in letzter Zeit immer häufiger auftritt, hier mein Versuch die Webapp automatisch als pre:script erstellen zu lassen.

das Skript scannt die Webapp und speichert eine lokale Hash-Datei. gibt es die Datei noch nicht, oder hat sich seit dem letzten Durchlauf eine Datei in der Webapp geändert, wird der install und build Prozess angestoßen.

@schlimmchen, ob der Build Prozess auch tatsächlich durchläuft, konnte ich bei mir nicht nachvollziehen. Ich habe ja aber auch nichts an der Webapp geändert. Beim ersten Durchlauf ohne Hash-Datei bekomme ich in VSCode folgenden zusätzlichen Log

Verbose mode can be enabled via `-v, --verbose` option

-- neu

checke webapp
webapp changed.
Webapp changed, rebuilding...
Changing directory to: webapp
yarn install v1.22.22
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
warning " > @vue/eslint-config-typescript@13.0.0" has incorrect peer dependency "eslint@^8.56.0".
warning "@vue/eslint-config-typescript > @typescript-eslint/eslint-plugin@7.2.0" has incorrect peer dependency "eslint@^8.56.0".
warning "@vue/eslint-config-typescript > @typescript-eslint/parser@7.2.0" has incorrect peer dependency "eslint@^8.56.0".
warning "@vue/eslint-config-typescript > @typescript-eslint/eslint-plugin > @typescript-eslint/type-utils@7.2.0" has incorrect peer dependency "eslint@^8.56.0".
warning "@vue/eslint-config-typescript > @typescript-eslint/eslint-plugin > @typescript-eslint/utils@7.2.0" has incorrect peer dependency "eslint@^8.56.0".
[4/4] Building fresh packages...
Done in 7.38s.
Build completed successfully.

-- weiter

Firmware Revision: ge8e32a9

ich wüsste nicht, warum der Buildbefehl nicht korrekt laufen sollte, aber hab es auf die Schnelle wie gesagt nicht vollumfänglich getestet.

Und ich weiß auch nicht, warum VSCode scheinbar 5 Datein ändert. eigentlich waren es nur 3. das neue Skript und die geänderte platformio.ini und .gitignore

Grüße

schlimmchen commented 1 month ago

Und ich weiß auch nicht, warum VSCode scheinbar 5 Datein ändert. eigentlich waren es nur 3. das neue Skript und die geänderte platformio.ini und .gitignore

Das hat nichts mit vscode zu tun, sondern höchstwahrscheinlich damit, dass du deine Arbeit von helgeerbe/master begonnen hast, statt von helgeerbe/development, willst aber nun (zurecht) nach helgeerbe/development mergen lassen.

spcqike commented 1 month ago

das könnte sein :)

Ich dachte irgendwie, dass das keine Probleme macht, da es ja kurz vorher erst ein Release gab und dev und master quasi gleich auf waren 🤷‍♂️

Besteht denn überhaupt Interesse an dem pre-Script? ich könnt es ja nochmal neu machen und auf den dev aufbauen. oder ich schließe es einfach (:

schlimmchen commented 1 month ago

Lass einfach stehen, ich bau das demnächst ein. Es gibt Dinge, die mir nicht passen, z.B. dass yarn erwartet wird, obwohl man ggf. auch "nur" npm hat (wie ich). Das einzige, was ich ggf. schwierig fände wäre, wenn das lange dauert. Ich vermute mal, dass die Prüfsummenbildung aber kein Problem ist. Naja, sie muss natürlich auf webapp/src beschränkt werden. Die ganzen Pakete will ich nicht überprüfen müssen. Außerdem will ich keinen neuen Build haben, wenn Zwischendatein sich geändert haben, etc.

spcqike commented 1 month ago

Klingt sinnvoll.

Ne das bilden der Prüfsumme war nicht so lang. Das ging recht flott.

So wie das Skript läuft, hab ich die Dokumentation halt gelesen. Sicherlich kannst du das optimieren und damit auch schneller machen (: bisher prüft es ja den gesamten webapp Ordner und nicht nur den darin befindlichen Src Ordner.

schlimmchen commented 1 month ago

rebased onto current development branch.

schlimmchen commented 1 month ago

Sehr cool, vielen Dank für die Idee und die Vorlage, @spcqike :heart: :rocket:

spcqike commented 1 month ago

freut mich, dass es dir gefällt. und wenn auch nur ein oder zwei Issues durch fehlende webapp Compilierung damit unterbunden werden können :)

Ich nehme an, du konntest es lokal laufen lassen und testen? Wie eingangs geschrieben, konnte ich das, in Ermangelung von einer geänderten Webapp, nicht :)

schlimmchen commented 1 month ago

Ja, natürlich. Und ich versteh dein Problem nicht^^ Editiere eine Datei, mach nen Buchstaben hinzu, und dann ist die WebApp "geändert" und das Skript muss erkennen, dass es eine Änderung gab. Tut es auch. 👍

schlimmchen commented 1 month ago

FWIW on my Debian yarn is called yarnpkg for some reason (naming conflict with some other existing Debian package?): https://packages.debian.org/buster/yarnpkg

@ranma Did you delete this comment? If so, why? We can certainly try yarnpkg if yarn is not found. Is that necessary?

I did not install nodejs from the debian archives and I somehow got yarn on one machine but not on all of them... It's not very straight-forward.

ranma commented 1 month ago

@ranma Did you delete this comment? If so, why?

No, the comment is on the commit itself: https://github.com/helgeerbe/OpenDTU-OnBattery/commit/cec4003f6e12c842889650fa669611ee1925933b#commitcomment-146347454

We can certainly try yarnpkg if yarn is not found. Is that necessary?

I just made a symlink for myself so it'll run fine trying to execute yarn now.

schlimmchen commented 1 month ago

Yeah, I recognized that, but it did not know yet that the (very lacking) Github app just won't show the comment 🙄

I will make the script test whether yarnpkg ist available and use that if yarn is not available.