Wäre halt noch ein Schritt weiter in eine mögliche Ziel-Struktur, die verständlich und logisch gekapselt ist.
Aber in diesem Fall werde ich einfach hier ein paar erklärende Worte schreiben und das gut sein lassen 
…
2 „Gefällt mir“
Ich bin mir nicht sicher ob es mit der neuen Led.cpp zu tun hat, aber seit einiger Zeit ist in der WebGui der prozentuale Wert für die Batteriespannung völlig falsch.
Bisschen weit hergeholt so was hier zu posten, oder?
Vielleicht beschriebst du in einem neuen Post genau was sich komisch verhält?
Dass die Batterie-Messung extrem schwankt ist zumindest bei mir schon von vorn herein so habe mich damit aber noch nicht näher befasst…
Zurück zum Thema:
ich würde gerne Brainstormen wie die LED.cpp weiter umgebaut werden könnte und lesbarer, besser wartbar und erweiterbarer wird.
Eine erste Idee dazu wäre die einzelnen Animationen in separate Methoden auslagern, die jeweils zyklisch aufgerufen werden solange diese Animation aktiv ist. Alles was für die Animation relevant ist spielt sich dann nur noch in der jeweiligen Methode ab.
Gibt es weitere Ideen / Vorschläge / Anmerkungen?
3 „Gefällt mir“
Definitiv eine gute Idee mit den einzelnen Funktionen. Auf jeden fall sollten wir die Aufrufe und die rückgabewerte standartisieren
Eine Idee von mir, für dessen Teil ich auch schon ein PR gemacht hatte ist die Verwendung von CRGBSet’s. Gemeinsam mit dem weggang von FastLED.clear() abstrahiert das den Zugriff auf das LED Array. Dann könnten die Funktionen das CRGBSet bekommen und „ohne wissen“ über ihre Position in dem WS2812 Array auf die LEDs zugreifen (CPixelView unterstützt kein Offset, also ist es aktuell leider nur die halbe Miete).
Ich hatte schon einige kleine Tests gemacht, wie man das CRGBSet (eigentlich CPixelView) erweitern könnte um ihn direkt offset & direction zu übergeben. Damit müssten die Animationen sich nicht mehr um die Umrechnung der relativen LED adresse in eine Absolute kümmern.
Die Verwendung von PixelViews ebnet den Weg um weitere Teile eines LED Streifens entweder als statische LEDs oder für andere Animationen zu nutzen.
Zukunftsmusik (aka, noch nicht ganz zu Ende gedacht):
Für die Erweiterbarkeit (oder für die Animationen) könnten wir eine statische „Plugin System“ überlegen. In c++ gibt es das CRTP System und gemeinsam mit std::variant könnte eine zur Compile Zeit definierte Vererbung (d.h. ohne vTables und zusätzlichen Overhead beim Aufruf) erstellt werden.
Das ganze hab ich mir noch nicht ganz für die LEDs überlegt (CRTP war mir eher für die RFID Treibern untergekommen), aber ich werfe es mal als Idee in den Ring. Dürft ihr auch gerne zerfleischen 
.
2 „Gefällt mir“
Ich habe hiermit mal gestartet:
Alle Animationen haben eigene Funktionen.
Diese würde ich dann mal Stück für Stück optimieren.
Was denkst du so über den Ansatz?
habe auch schonmal das mit dem „immer Lautstärke anzeigen“ und dein Bugfix mit dem Mulitbutton mit reingebaut …
2 „Gefällt mir“
Ich hab das nur mal grob überflogen und finde das ist schon deutlich übersichtlicher und besser lesbar. Wenn daraus ein PR werden soll, dann würde ich aber vorschlagen das auf den aktuellen Dev-Branch aufzusetzen und die Reworks strikt von irgendwelchen sonstigen Features bzw. funktionellen Änderungen zu trennen.
Langfristig wäre es dann sicher sinnvoll das noch in eine Klassen-Struktur zu packen. Also eine Klasse für die LEDs (Singleton, bündelt alle Funktionen und Variablen zur Anzeige), eine für die Animationen (eine Instanz pro Animationstyp, ggf. mit Parent/Interface-Klasse), etc.
Meine C+±Kenntnisse sind ein bisschen eingerostet, aber ich denke hier bietet es sich schon an ein bisschen objektorientiert zu denken…
2 „Gefällt mir“
Ja, auf den ersten Blick schaut das deutlich besser aus. Ein Punkt, der bei der Lösung auf uns zukommen wird, sind viele unused Warnungen für startNewAnimation (können aber mit __attribute__((unused)) von gcc oder [[maybe_unused]] von c++17 bekämpft werden).
Da das schon einiges an Code Änderung ist würde ich auch vorschlagen nur auf die Dev-Branch aufzusetzten und die Änderungen lokal zu Led.cpp & Led.h zu halten. Sobald diese durch sind, werde ich meine PR’s und die 3 Bugfixes (shutdown animation, Error-Indicator und Volume) an deine PR anpassen.
Lagfristige Lösung bin ich bei fschrempf, Objektorientierung sollte unser Ziel sein. Wie genau wir die einzelnen Klassen dann unterbringen, können wir ausschnapsen :). Auf jeden fall macht die Kapselung in Klassen auch das Testen einfacher (als Beispiel, hier ist das Playlist-Interface mit allen abgeleiteten Klassen und ihren unit tests).
And now something totally different
In der Zwischenzeit habe ich auch meinen alten Code für den Led RingBuffer ausgepackt, abgestaubt und mit unit tests versehen. Die Klasse leitet sich von CPixelView ab und sollte alles in allem mit dieser (und damit mit CRGBSet’s) kompatibel sein.
Die Branch beinhaltet einen abgeleitete Version vom CPixelViewer, der neben direction auch mit dem Offset umgehen kann. Diese ist aktuell (noch) nicht in die Led.cpp dieser Branch eingebunden. Ich werde morgen (bzw am Wochenende) schauen, ob ich die Klasse einfach in die Led.cpp einfach einbinden kann (wobei ich nicht viel Aufwand aktuell in die Integration rein stecken werde, da Joe92’s PR alles umkrempeln wird).
2 „Gefällt mir“
Ja, genau. Ich merke auch gerade dass mein Vorschlag zur Aufteilung von gestern Abend auf den zweiten Blick nicht wirklich sinnvoll/ausgereift ist.
Ist jetzt im dev, du kannst also den Bugfix mit dem dem button rein machen .
Vielen Dank dir!