RFC: Migration zu neuer Weboberfläche

Software Feature Request
21

@Christian Danke für Deine Ideen, ich stimme dir zu 90% zu. Ich glaube, wir wollen alle so ziemlich das Gleiche erreichen. Aber bitte in kleinen Schritten, denn alles andere ist schon gescheitert.

Mein Pull-Request entfernt zunächst nur den Template-Prozessor und nicht mehr.

Ich schlage das Laden der Einstellungen per Websocket vor & habe mich dazu entschieden, weil das aus meiner Sicht schneller ist (Verbindung bereits geöffnet). Man kann das aber auch mit einer Zeile auf GET hier ändern, habe damit kein Problem. Der JSON Payload ist dann eh gleich, bin da total wertfrei.

Filetree & Paginierung sind hier komplett außen vor (Entwicklung in kleinen Schritten). Außerdem spielt das eigentlich keine Rolle mehr, da das Dateisystem mittlerweile um den Faktor 20 beschleunigt wurde und schnell genug lädt, Pagiinierung nicht mehr notwendig.

Kurzum: Eine schöne REST-API + Websocket Echtzeitbenachrichtigung ist erst der übernächste Schritt . Ich möchte das nur vorbereiten, dann können die Web&Backend-Experten ran und eine schöne API entwerfen :wink:

Gern noch mehr Review da das schon einige Zeilen sind…

1 „Gefällt mir“
22

Das ist schon klar. Ich hatte diese Diskussion aber auch als „Brainstorming“ verstanden um den Rahmen, das Zielbild zu diskutieren - bevor man mit Änderungen anfängt (oder parallel dazu).
Wenn dem so nicht ist, bin ich einfach still :wink:

Ich hatte die Richtung ESP->Client gemeint. Aber ja, wie es dann übertragen wird fast wurscht. Nur warum reden wir über unterschiedliche „Endpoints“ wenn dann doch alles per Websockt gemacht wird.
Und ja, auch das sollten kleine Schritte sein - aber am Ende machst Du diese kleinen Schritte zweimal, wenn man das noch einmal ändern möchte.

Hier geht es um Speicher und nicht um Geschwindigkeit. Aber ja, auch das ist unabhängig und kann separat betrachtet werden.

Noch einmal, das Vorgehen halte ich für komplett richtig und es stimmt auch, dass ich damals zu viel gleichzeitig geändert habe.

23

@Christian Ah OK, Du bist schon einen Schritt weiter :wink: Die Diskussion dazu macht Sinn & Brainstorming auch!

Gibt es noch konkrete Vorschläge für den laufenden PR (Template-processor)? Natürlich wollen wir keine Schritte zweimal machen.
Bsp. zusätzliches Feature-Objekt: Gibt es einen Vorteil? Das würde den JSON Payload größer machen. Jetzt frage ich im JavaScript einfach ab ob es jeweilige Objekt gibt oder nicht

24

Hier oder im Github PR?

25

Geht Beides, für Code Verbesserungen würde ich direkt Anmerkungen in Github & für größere Sachen das Forum hier vorschlagen…

26

Ich kann gerade nicht testen, aber ich hätte zwei Ideen:

JSONToSettings

Könnte man nicht zur Verschlankung aus

uint8_t iVol = doc["general"]["iVol"].as<uint8_t>();
gPrefsSettings.putUInt("initVolume", iVol) 
if (gPrefsSettings.getUInt("initVolume", 0) != iVol ) {
				Log_Println("Failed to save general settings", LOGLEVEL_ERROR);
				return false;
			}

So etwas pro Setting machen

if (gPrefsSettings.putUInt("initVolume", doc["general"]["iVol"].as<uint8_t>()) == 0 {
				return false;
			}

Ich finde die Zuordnung der unterschiedlichen Bezeichnungen unübersichtlich.
In einem nächsten Schritt wäre es ohnehin gut, wenn man die Bezeichnungen anpassen würde.
Also nicht iVol = initVolume.

Und bei

static void settingsToJSON(JsonObject obj, String section) {
	if ((section == "") || (section == "current")) {
		// current values
		JsonObject curObj = obj.createNestedObject("current");
		curObj["volume"].set(AudioPlayer_GetCurrentVolume());
		curObj["rfidTagId"] = String(gCurrentRfidTagId);
		curObj["hostname"] = Wlan_GetHostname();
	}

die Benennung jeweils anpassen:

static void settingsToJSON(JsonObject obj, String section) {
	if ((section == "") || (section == "current")) {
		// current values
		JsonObject newObj = obj.createNestedObject("current");
		newObj["volume"].set(AudioPlayer_GetCurrentVolume());
		newObj["rfidTagId"] = String(gCurrentRfidTagId);
		newObj["hostname"] = Wlan_GetHostname();
	}

Gibt hier vermutlich kein richtig oder falsch. Ist mir einfach aufgefallen.

2 „Gefällt mir“
27

Ich würde sehr gerne drüber schauen, habe aber maximal am Wochenende mal Zeit. Wenn der PR noch ein paar Tage offen ist, schaue ich drüber. Die Beschreibung klingt aber sinnvoll!

Es spricht meiner Meinung nach auch nichts dagegen, zweigleisig zu fahren und einen Pfad später zu löschen (oder auch drin lassen). Aber das später.

Christians Feedback stimme ich mindestens da zu, dass mir „iVol“ auch nicht so gut gefällt, und sprechende Namen besser sind.

2 „Gefällt mir“
28

Kein Problem, ich lasse den PR noch ein wenig länger offen, sollen ja Alle zufrieden sein :wink:

Die Bennennung habe ich bewusst nicht geändert um den PR so klein wie möglich zu halten. Es geht ja hier nur um die Ersetzung des Template-Prozessors & nicht mehr.
„iVol“ bleibt zunächst unverändert und wird hier bereits verwendet.
Stimme aber auch zu - Das ist nicht elegant, ich hatte auch Probleme die einzelnen Einstellungen richtig zuzuordnen.
Aber evt. machen wir die Neubenennung im nächsten Schritt, evt. mit einer definierten API Beschreibung?

29

Habe Eure Anmerkungen/Wünsche umgesetzt und den PR aktualisiert:

@Christian die Verschlankung ist drin und spart einige Zeilen. Funktioniert aber nicht für leere Strings, daher habe ich es bei den Strings zunächst so belassen.

Die Umbenennung war jetzt auch schnell gemacht und kann natürlich noch geändert werden. Lesbarkeit vor Kürze, so sieht das JSON jetzt aus:

{
	"current": {
		"volume": 2,
		"rfidTagId": "086114072041"
	},
	"general": {
		"initVolume": 5,
		"maxVolumeSp": 21,
		"maxVolumeHp": 18,
		"sleepInactivity": 10
	},
	"wifi": {
		"hostname": "Espuino",
		"scanOnStart": true
	},
	"neopixel": {
		"initBrightness": 16,
		"nightBrightness": 2
	},
	"battery": {
		"warnLowVoltage": 3.400000095,
		"indicatorLow": 3,
		"indicatorHi": 4.199999809,
		"voltageCheckInterval": 10
	},
	"ftp": {
		"username": "esp321",
		"password": "mutti",
		"maxUserLength": 9,
		"maxPwdLength": 9
	},
	"mqtt": {
		"enable": false,
		"clientID": "ESP32-ESPuino",
		"server": "192.168.2.43",
		"port": 1883,
		"username": "mqtt-user",
		"password": "geheim",
		"maxUserLength": 15,
		"maxPwdLength": 15,
		"maxClientIdLength": 15,
		"maxServerLength": 31
	},
	"bluetooth": {
		"deviceName": "My POGS Wireless Headphone",
		"pinCode": "0000"
	}
}

ClientID oder ClientId? Kommentare gern hier.

2 „Gefällt mir“
30

Der PR ist ja schin länger offen und wurde durch die Review geschickt, Danke an @SZenglein !
Einige Kleinigkeiten habe ich noch korrigiert, z.B.wird jetzt auch die WLAN-Liste beim Laden der Webseite über das Websocket gefüllt, konsistent zum Laden der Einstellungen.

Habe Alles getestet soweit es geht & würde das die nächsten Tage in den DEV-Branch übernehmen…

2 „Gefällt mir“
31

Als letztes habe ich mir den „/info“ Endpunkt vorgeknöpft um die Trennung von Frontend/Backend weiter voranzutreiben.
Dazu liefert „/info“ jetzt nur noch die Werte als Application/JSON zurück und die Ausgabe/Formatierung wird im dann Frontend gemacht.
Damit verschiebt sich auch die Sprachumschaltung konsequent ins Frontend. Analog zum /settings Endpunkt kann man sich alle Informationen zurückgeben lassen oder nur einen einzelnen Bereich wie „battery“ oder „memory“ z.B. mit dem Aufruf http://espuino.local/info?section=battery

Wer möchte kann hier nochmal drüberschauen:

So sieht das /info-JSON dann aus:

{
   "software":{
      "version":"Software-revision: 20230804-1",
      "git":"Git-revision: 5f09f28-dirty",
      "arduino":"2.0.9",
      "idf":"v4.4.4"
   },
   "hardware":{
      "model":"ESP32-D0WD-V3",
      "revision":3,
      "freq":240
   },
   "memory":{
      "freeHeap":74144,
      "largestFreeBlock":3407860,
      "freePSRam":3478599
   },
   "wifi":{
      "ip":"192.168.1.55",
      "rssi":-67
   },
   "audio":{
      "playtimeTotal":35106000,
      "playtimeSinceStart":462000
   },
   "battery":{
      "currVoltage":3.938959122,
      "chargeLevel":77.03112793
   }
}

Das Infofenster bleibt nach der Änderung nahezu gleich:

grafik
grafik1136×1012 82.5 KB

2 „Gefällt mir“
32

Ist geplant dass über so einen Endpoint auch den Inhalt der backup.txt auszulesen um die im Frontend zu bearbeiten? Ich weiß, dass das schonmal irgendwo angesprochen wurde ich weiß nur nicht mehr in welchem Thread.

33

Es ist bereits möglich, Dateien von der SD-Karte zu laden und schreiben, daher kannst du das manuell ja schon. Ein Editor für die backup.txt direkt im Frontend ist wohl eher nicht geplant.

34

Ich überlegte das selbst hinzuzufügen. Die Frage zielte eher dahin ob eine Schnittstelle zum auslesen der RFIDs geplant ist. Dann müsste es nur im Frontend integriert werden.

35

Das war wohl dieser Thread:

Die Backup.txt kann man mit rechter Maustaste „Download“ oder diesem Befehl laden:
http://espuino.local/explorerdownload?path=/backup.txt

Die kann dann mit einem Texteditor bearbeitet und evt. wieder eingespielt werden.
@chrischan Zu welchem Zweck willst Du eine extra RFID-Bearbeitung in der Weboberfläche einbauen?
Weil das manuelle Triggern von Aktionen ist ja jetzt schon möglich.

1 „Gefällt mir“
36

Um evtl vorhandene Relikte entfernen zu können. Ich habe eine Box die mein Patenkind bekommt. Damit auch dessen Eltern das machen können ohne die Frustrationsgrenze zu suchen. Ich weiß nicht wie speicherintensiv lange Zuordnungslisten bei der Verarbeitung von aufgelegten Karten ist. Mein Gedanke war es nicht unnötig lang werden zu lassen.

37

Mit dem letzten Commit ist der „/restart“ & „/shutdown“ Endpunkt nun von GET auf POST geändert. Die bestehenden Endpunkte sollten nun einheitlich sein…

Aus meiner Sicht sind die unterschiedlichen Startmethoden des Webservers im Accesspoint / Management Modus jetzt überflüssig. ich meine Web_Init() und webserverStart()
Die derzeit getrennten Accesspoint / Management Seiten könnten doch über die gleichen Methoden von webserverStart() laufen. Nur beim Ausliefern der Index.html Datei müsste man im Moment noch unterscheiden. Damit würde einiges an redundanten Code rausfliegen (Also komplett Web_Init()).
Was meint Ihr dazu?

Es fehlt noch komplett ein Endpunkt zum Lesen/Schreiben/Modifikation von RFID-Tags. Das wird bisher über das Websocket mit „rfidAssign“ und „rfidMod“ hier und hier gemacht. Gibt es Vorschläge wie man das zusätzlich mit HTTP-Endpunkt(en) umsetzt?

Wenn wir sowas haben könnte auch die bestehende Web-UI profitieren: Beim Auflegen einer bereits zugewiesenen Karte könnte die gespeicherte Zuweisung wieder angezeigt werden (URL & Modus bzw. Modifikation wird vorausgefüllt wie jetzt das RFID-Feld)

Eine Auflistung aller im NVS gespeicherten Tags scheint mit Arduino-Preferences gar nicht möglich?

Edit: Das wäre mein Vorschlag für die Vereinheitlichung vom accesspoint/management im Webserver:

38

Ich glaube, bei der Migration ist ein wenig was kaputt gegangen - zumindest wird bei mir im Dev-Branch die Option initBrightness bzw. nightBrightness zwar korrekt gespeichert und geladen - aber dann nicht verwendet, sondern immer nur der Default-Wert von 16.
Scheint mir auf den ersten Blick daran zu liegen, dass in der Web.cpp das ganze mit gPrefsSettings.putUInt()/gPrefsSettings.getUInt() gespeichert/geladen wird aber in der Led.cpp mit gPrefsSettings.getUChar() geladen wird.

2 „Gefällt mir“
39

@kkF Danke für Deinen Bugreport!
Das muss natürlich einheitlich sein, verstehe aber nicht ganz warum der Integer-Wert zuvor als UChar gespeichert wurde, UInt wäre doch logischer, also Fix in Led.cpp?

40

UChar braucht nur 1 Byte
Uint min. 2 wenn nicht sogar 4

grenzenlose Speicherverschwendung :wink:

2 „Gefällt mir“