LED-Verbesserungen / Rework

Bei mir ist auch ein Problem aufgetaucht. Kurz zur Erläuterung der Situation:
Wir haben eine Karte die alle Titel in einem zufälligen Unterverzeichnis abspielt. Damit wir wissen, welches Album (bzw. welcher Ordner) gespielt wird, haben wir einen Track vorangestellt, bei dem der Albumname vorgelesen wird. Bisher lief es so, dass dieser Track abgespielt wurde und anschließend alle Tracks die im Album enthalten sind.

Seit dem Update spielt er den kurzen Track, der den Albumnamen vorliest problemlos. Spielt dann den ersten Ton vom nächsten Track und stürzt ab und rebootet.

[ 24799 ]  RFID-Karte erkannt: (ISO-14443) ID: 8b-8f-cf-bd
[ 24801 ]  RFID-Karte empfangen: 139143207189
[ 24813 ]  Versuche ein zufälliges Unterzeichnis zu finden aus: /Kalle Klang u. die Floht�ne
[ 24837 ]  Zufällig ausgewähltes Unterverzeichnis: /Kalle Klang u. die Floht�ne/Sing mit mir Kinderlieder 3
[ 24863 ]  Playlist-Generierung: cached
[ 24914 ]  Freier Speicher: 81332
[ 24915 ]  Anzahl gültiger Files/Webstreams: 28
[ 24915 ]  Modus: Spiele alle Tracks (alphabetisch sortiert) des Ordners '/Kalle Klang u. die Floht�ne/Sing mit mir Kinderlieder 3'
[ 24930 ]  Neue Playlist empfangen mit 28 Titel(n)
[ 24930 ]  Free heap: : 82228
[ 24987 ]  info        : PSRAM found, inputBufferSize: 283615 bytes
[ 25059 ]  info        : buffers freed, free Heap: 82064 bytes
[ 25059 ]  info        : Reading file: "/Kalle Klang u. die Floht�ne/Sing mit mir Kinderlieder 3/00 - Albumname rot.mp3"
[ 25092 ]  info        : MP3Decoder has been initialized, free Heap: 58372 bytes
[ 25098 ]  '/Kalle Klang u. die Floht�ne/Sing mit mir Kinderlieder 3/00 - Albumname rot.mp3' wird abgespielt (1 von 28)
[ 25106 ]  info        : Content-Length: 17591
[ 25106 ]  info        : ID3 framesSize: 1034
[ 25107 ]  info        : ID3 version: 2.4
[ 25114 ]  info        : ID3 normal frames
[ 25164 ]  info        : Audio-Length: 16557
[ 25168 ]  info        : stream ready
[ 25168 ]  info        : syncword found at pos 0
[ 25172 ]  info        : Channels: 1
[ 25173 ]  info        : SampleRate: 22050
[ 25173 ]  info        : BitsPerSample: 16
[ 25176 ]  info        : BitRate: 40000
[ 25188 ]  info        : VBR recognized, audioFileDuration is estimated
[ 28076 ]  info        : Closing audio file
[ 28077 ]  info        : End of file "/Kalle Klang u. die Floht�ne/Sing mit mir Kinderlieder 3/00 - Albumname rot.mp3"[ 28079 ]  eof_mp3     : /Kalle Klang u. die Floht�ne/Sing mit mir Kinderlieder 3/00 - Albumname rot.mp3
[ 28447 ]  info        : buffers freed, free Heap: 82064 bytes
[ 28447 ]  info        : Reading file: "/Kalle Klang u. die Floht�ne/Sing mit mir Kinderlieder 3/01 - Das ist gerade, 
das ist schief.mp3"
[ 28470 ]  info        : MP3Decoder has been initialized, free Heap: 58348 bytes
[ 28478 ]  '/Kalle Klang u. die Floht�ne/Sing mit mir Kinderlieder 3/01 - Das ist gerade, das ist schief.mp3' wird abgespielt (2 von 28)
[ 28486 ]  info        : Content-Length: 2389521
[ 28486 ]  info        : ID3 framesSize: 146577
[ 28487 ]  info        : ID3 version: 2.3
[ 28494 ]  info        : ID3 normal frames
[ 28573 ]  id3data     : Album: Sing mit mir Kinderlieder 3
[ 28585 ]  id3data     : Artist: Kalle Klang & Die Flohtöne
[ 28598 ]  id3data     : Band: Kalle Klang & Die Flohtöne
[ 28610 ]  id3data     : PartOfSet: 1
[ 28622 ]  id3data     : Title: Das ist gerade, das ist schief
[ 28637 ]  id3data     : Track: 01
[ 28648 ]  id3data     : Year: 2017
[ 29800 ]  info        : Audio-Length: 2242944
[ 29806 ]  info        : stream ready
[ 29806 ]  info        : syncword found at pos 0
[ 29812 ]  info        : Channels: 1
[ 29812 ]  info        : SampleRate: 48000
[ 29812 ]  info        : BitsPerSample: 16
[ 29814 ]  info        : BitRate: 128000
Guru Meditation Error: Core  1 panic'ed (LoadProhibited). Exception was unhandled.
Core 1 register dump:
PC      : 0x400d3578  PS      : 0x00060d30  A0      : 0x800d6daf  A1      : 0x3ffdb7d0
A2      : 0x3ffc4c60  A3      : 0x0000000f  A4      : 0x3ffc4f40  A5      : 0x000000ff  
A6      : 0xff0000ff  A7      : 0x00ff0000  A8      : 0x800d35e3  A9      : 0x0000ff00
A10     : 0x0000000f  A11     : 0x3ffc4f4c  A12     : 0x3ffc4f4f  A13     : 0x3ffc4f26  
A14     : 0x00000063  A15     : 0x00000002  SAR     : 0x00000020  EXCCAUSE: 0x0000001c
EXCVADDR: 0xff000103  LBEG    : 0x400955be  LEND    : 0x400955c9  LCOUNT  : 0x00000000  

ELF file SHA256: 0000000000000000

Backtrace: 0x400d3578:0x3ffdb7d0 0x400d6dac:0x3ffdb810 0x400d798d:0x3ffdb830 0x40099402:0x3ffdb880
  #0  0x400d3578:0x3ffdb7d0 in CLEDController::showLeds(unsigned char) at .pio/libdeps/lolin_d32_pro_sdmmc_pe/FastLED/src/FastLED.cpp:269
      (inlined by) CFastLED::show(unsigned char) at .pio/libdeps/lolin_d32_pro_sdmmc_pe/FastLED/src/FastLED.cpp:59    
  #1  0x400d6dac:0x3ffdb810 in CFastLED::show() at .pio/libdeps/lolin_d32_pro_sdmmc_pe/FastLED/src/controller.h:171   
  #2  0x400d798d:0x3ffdb830 in Led_Task(void*) at .pio/libdeps/lolin_d32_pro_sdmmc_pe/FastLED/src/controller.h:171    
  #3  0x40099402:0x3ffdb880 in vPortTaskWrapper at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/freertos/port.c:355 (discriminator 1)

Rebooting...
  • Alle anderen Alben lassen sich problemlos abspielen.
  • Die einzelnen Tracks können ohne Probleme wiedergegeben werden.
  • Auch beim Auswählen der problematischen Alben in der Weboberfläche tritt das selbe Problem auf.

PS Ich bin wohl nicht der Einzige mit dem Problem: siehe hier

Bei mir kommt es zum gleichen Verhalten nur bei normalen Titeln. Interesanter weise auch nicht bei allen. Bei den betroffenen jedoch reproduzierbar auf allen ESPuinos.

Tut mir Leid, diesen Fall hatte ich total übersehen. Habe einen Fix erstellt der sicher bald im master sein wird:

Ist bereits drin und funktioniert aus meiner Sicht :+1:
Habe noch zwei angefügt.

Was mir glaube ich helfen würde, ist eine Kurzbeschreibung der Task-Variablen. Also sowas wie animationTimer, animationIndex, subAnimationIndex, staticLastBarLenghtPlaylist, staticLastTrack, staticBatteryLevel. Auch das Setzen des (sub)AnimationIndex auf irgendwelche Zahlen und warum es ausgerechnet einen Case 42 gibt :thinking:.

Sollten wir eigentlich Schimpfwörter in den Code aufnehmen?

:rofl:

Hi Torsten,
ja, das kann ich gerne machen. Am besten dokumentiere ich das im Code. Oder soll ich es hier im Post 1 zunächst erklären?
Ich kann ja beispielhaft mal den Code aus einer Animation in eine separate Funktion schieben (dafür müssten wir aber vermutlich ein paar Variablen der LED.cpp als member verwenden.
Statt den Werten kann ich auch ein Enum nehmen. Das ist vermutlich lesbarer. Aber 42 ist doch eine schöne Zahl. Oder etwa nicht :smiley: ?
Vielleicht findet sich ja ein Enumwert mit einem Schimpfwort :wink:

Kannst dir aussuchen, wie es für dich besser ist.
Code ist vermutlich einfacher.

Hi, ich hätte mir das auf diese Art vorgestellt:
Die einzelnen Animationen bekommen eingene Funktionen.
Diese haben dann alle benötigten Variablen.
Habe es testweise mal mit zwei Animaitonen auf diesem Branch gemacht:

Wenn das in eine Richtung geht die dir / euch gefällt, dann würde ich das auch noch für weitere Animationen so umsetzten. Vielleicht gibt es aber auch bessere Lösungen :slight_smile:

Besteht hier irgendein Interesse dass ich das Thema weiterverfolge oder nicht?

Wenn ich ehrlich bin:
Ich wollte eigentlich nur den aktuellen Code ein bisschen dokumentiert wissen. Der ist aber hier jetzt in großen Teilen wieder umstrukturiert. Ich kann das nicht dauernd nachtesten :woman_shrugging:.

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

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

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 :laughing:.

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

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 :slight_smile:
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 :slight_smile: .
Vielen Dank dir!