Es kam hier kürzlich das Thema auf, dass der RC522 aus dem Task rausgeholt wurde. Einerseits, weil es Ram spart und andererseits, weil vermutet wurde, dass der Leser dann besser funktioniert.
Für den RC522 habe ich das im Commit von eben gemacht und bitte euch, das mal zu testen. Also insbesondere im Hinblick darauf, ob der jetzt besser performt hinsichtlich der Reichweite. Ich habe auch die Intervallzeit auf 100 ms reduziert (war voher per Default 300 ms).
Jetzt wird sich der ein oder andere fragen: Und was ist mit dem PN5180?
Nun, den musste ich im Task lassen, weil er schon im Leerlauf gut 240 ms benötigt, um zu erkennen, ob eine neue Karte aufgelegt wurde. Wird nebenbei Musik abgespielt, so landen wir schnell bei 500 ms. Das ist einfach (beides) VIIIEL zu lange, um in loop() zu laufen. Vielleicht kann @tueddy hier noch etwas optimieren, aber so lange bleibt’s erstmal im Task.
Vielleicht hat ja jmd. noch einen alternativen Vorschlag.
Bezüglich der Laufzeitoptimierung fallen mir folgende Punkte ein.
Das Auslesen sollte mit einer State Machine umgesetzt werden, damit der ganze Code nicht einmal „am Stück“ läuft. Z.B.: rfid.cpp · GitHub
Es gibt ein paar delay()-Aufrufe in der PN5180-Lib, wo man sich überlegen könnte, ob die wirklich notwendig sind bzw. der Wert eventuell etwas heruntergesetzt werden könnte. Z.B.: im Kommentar steht 2µs, es wird aber 10ms gewartet.
Die Kommunikation mit dem PN5180 läuft trotz der 7 MHz SPI-Frequenz etwas langsam, da der Chip über die Busy-Leitung immer wieder signalisiert, dass er noch beschäftigt ist. Einige Teile der Lib könnten asynchron umgesetzt werden. Damit würde man sich das ständige Warten sparen.
Die loop-Funktion ist letztendlich auch nur ein Task mit einer Priorität von 1. Der Audio-Task hat eine Priorität von 2. Im Audio-Task selbst wird nirgendwo ein vTaskDelay aufgerufen. Das bedeutet, dass der Task damit fast zu 100% läuft und immer wieder den Loop-Task verdrängt. Ein vTaskDelay von z.B. 5ms im Audio-Task würde dem Loop-Task wieder etwas „Luft zum Atmen“ geben.
@tuniii: Erstmal danke für deine Ausführungen
Über (1) hatte ich auch nachgedacht, es jedoch dann jedoch nicht getestet, weil die Gesamtlaufzeit halt so lange ist, dass ich vermutet hätte, dass das Ganze nicht mehr fürchterlich „responsive“ (als Ganzes) ist. Unter die 230 ms wird man ja auch im Betrieb nicht kommen. Zum Vergleich: RC522 hatte ich mit 29 ms im Leerlauf gemessen.
2+3: Ok, da muss ich mal reinschauen.
4: Der Audio-Task ist recht zeitkritisch - viel Luft ist da nicht. Aber ja, sowas wie 5 ms wird vielleicht gehen.
Insgesamt gefällt mir das jetzt nicht, dass ich einen Leser im Task habe und einen nicht. Aber ich bin jetzt auch mal auf Feedback gespannt, ob das Rausnehmen aus dem Task tatsächlich die Performance verbessert. Deswegen habe ich es (übergangsweise?) jetzt mal so umgesetzt.
Also tatsächlich kriegt man die Laufzeit unter Belastung auf annähernd derer reduzierty die man im Leerlauf hat, wenn man 5ms vtaskdelay in Playaudio integriert.
Warum ist das so? Die laufen nicht auf dem gleichen Core?
Man könnte sich generell noch einmal die Tasks ansehen. Du hast die Tasks ja an CPUs „gepinnt“ und mir ist aufgefallen, dass Du den Watchdog immer wieder „manuell“ zurücksetzt. Ich meine (Nachbar-Forum) das hattest Du gemacht, weil der WDT immer wieder ausgelöst hat. Bin mir jetzt nicht mehr sicher welcher der WDT das war.
Da würde ich ansetzen. Nach meinem Verständnis müsste es auch ohne dieses Zurücksetzen klappen und der Scheduler soll sich darum kümmern, welche CPU gerade „Zeit“ hat. Steuerung dann über PRIOs und dem Scheduler die Möglichkeit zum Wechsel über Delays / Yields geben. Der Loop-Task ist (meine ich) auch fest an eine CPU gebunden. Den könnte man „leer“ machen und ausschließlich Tasks benutzen.
Zugegeben…größeres Thema. Da mich das interessiert, wollte ich mir das auch nach meinem Hardware-Thema ansehen…
Für mich ist das manuelle Zurücksetzen des WD auch nur ein Workaround. Neben den angelegten Tasks gibt es auch noch ein Idle-Task mit der Priorität von 0. Dieser kommt aber ohne ein vTaskDelay oder vTaskDelayUntil in den anderen Tasks nie zum Zuge und damit wird dann der Watchdog ausgelöst, weil das System bei 100% CPU-Last ist. Aktuell ist 100% CPU-Last ein Dauerzustand. Aber 100% CPU-Last sollten eigentlich nie erreicht werden. Eine geringere CPU-Auslastung benötigt auch weniger Strom und hat eine besser Akkulaufzeit zur Folge.
Ich könnte hier einiges an Erfahrung im Bereich Embedded einfließen lassen. Allerdings hat mich bisher die Gesamtstruktur des Projekts etwas abgeschreckt. Wäre es nicht denkbar einen Software-Freeze von 2-3 Wochen zu machen und die Funktionalitäten in einzelne Dateien (Header und Sourcen) auszulagern? Bitte nicht negativ auffassen, das ganze soll konstruktiv sein.
In meinem Fork hatte ich bereits angefangen die Struktur anzupassen, allerdings kam ich dann nicht mehr mit den Änderungen hinterher und hatte deshalb nicht mehr weitergemacht.
Können wir gerne machen. Mache doch mal einen Vorschlag hinsichtlich einer Struktur. Ich hatte es auch angefangen, aber letztlich war es viel mehr Arbeit, als ich dachte und am End habe ich es dann gelassen, weil ich zwischenzeitlich noch was Anderes implementieren wollte.
Zunächst könnte man die Funktionen und Variablen, welche eindeutig einem Feature zugeordnet werden können, in eigene Dateien auslagern. Dabei sollten die globalen Variablen als modul-global (static) deklariert werden und nur über Getter- und Setter-Funktionen „erreichbar“ sein.
Im Groben fallen mir folgenden Module (*.h + *.cpp) neben main.cpp ein:
RfidPn5180
RfidMfrc522
AudioPlayer
Battery
Buttons
Ftp
Logger
Mqtt
Web
Die Funktionen und globalen Variablen könnten jeweils den Modul-Namen vor dem Funktions-Namen haben. Z.B.: AudioPlayer_ Init().
Jedes Modul sollte dabei eine Init und eine zyklische Funktionen haben. Die zyklische Funktion implementiert dann die modul-spezifische State Machine.
Man könnte zudem noch schauen, dass man die Präprozessor-Anweisungen zur bedingten Kompilierung minimiert. Zum Beispiel:
Das meinte ich. Danke für die Erklärung. Ich glaube eben auch, dass wir 100% gar nicht benötigen.
Zum Thema Struktur bin ich komplett bei Euch. Ich komme ja von einem VS1053b Projekt. Dort hatte ich eine Aufteilung in verschiedene Header / Sourcen Files. Eigentlich wollte ich Stück für Stück die Funktionen vom ESPuino in meine Struktur übernehmen…Aber das war bisher zu aufwendig.
Wenn hier gerade das Thema Struktur ist, wollte ich mal wegen den main.cpp nachfragen. Das impliziert ja C++, geschrieben ist sie aber in C.
Der Hintergedanke dazu ist die Nutzung von constexpr und enum class in den settings.h.
Da bist du jetzt ein paar Stunden zu spät dran, denn das habe ich heute Morgen so implementiert Ich habe allerdings die aufgerufene Funktion etwas generischer benannt für den Fall, dass man da noch z.B. irgendwann mal ein Display adressieren will. Aber spricht ja nix dagegen, aus dieser Funktion eine Adapterfunktion zu machen und dann die jeweiligen Funktionen aufzurufen.
Zum Rest: Klingt nach einem Plan. Ich fasse das jetzt als Bewerbung auf Dann kannst dich gerne „austoben“. Ich haue dann außer Bugfixes die nächtsten drei Wochen nix raus.
Letztendlich wird es durch einen C++ Compiler gejagt. Viele der Libs sind auch in C++ geschrieben. Beides kann genutzt werden. Aktuell werden auch noch Variablen in den Header-Dateien angelegt. Die sollten dann auch raus bzw. ersetzte werden, da man ansonsten die Header-Datei nur einmal projektweit Inkludieren kann.
Umso besser
Ich kann das gerne so umsetzen. Ich bin noch bis einschließlich bis Montag mit einem anderen OSS-Projekt beschäftigt. Danach hätte ich Zeit das umzusetzen. Ich würde aber nochmal bzgl. des Code-Freezes Bescheid geben.
@tuniii Was ich vorher gerne noch machen würde, ist den PSRAM mehr zu nutzen. Da hatte ich demletzt schon eine Anpassung gemacht, aber das lässt sich noch verbessern. Das werde ich noch einchecken, wenn ich das getestet habe.
Also sag einfach Bescheid, wenn du loslegst mit dem Refactoring.
So, ich habe die PSRAM-Anpassungen eingecheckt. Im Grunde einfach nur Wrapper-Funktionen, die prüfen, ob PSRAM verfügbar ist. Wenn ja, dann wird er genutzt. Wenn nein, dann eben der Heap.
Da ich gerade schon über den Quellcode schaue. Wie sieht es aus ein paar Knöpfe umzubenennen?
NEXT_BUTTON → BUTTON_0
PREVIOUS_BUTTON → BUTTON_1
PAUSEPLAY_BUTTON → BUTTON_2
Bei DREHENCODER_BUTTON bin ich mir nicht sicher, sollte aber auch als BUTTON_3 genannt werden. Er wird ja als solcher behandelt.
Diese Änderungen sind die Folge des dynamische Button-Layouts.
Die Änderung bricht nicht existierende Konfigurationen, wenn man dort passende #ifdefs platziert: #ifndef BUTTON_0 #define BUTTON_0 NEXT_BUTTON #endif
Hatte ich auch schon drüber nachgedacht. Das ist für mich grundsätzlich in Ordnung. Die Implementierung ist auch schnell gemacht, aber in Sachen Doku muss ich dafür einiges an verschiedenen Stellen anpassen.
Ich nehme das in meine ToDo-Liste auf und warte aber erstmal ab, bis @tuniii das Refactoring fertig hat.
@biologist Ich würde morgen Abend damit anfangen. Einen Teil hatte ich ja bereits schon mal gemacht. Ich muss jetzt nur schauen, ob ich es aufgrund der vielen Änderungen noch einmal frisch mache, meinen bisherigen Stand recycle kann oder eine Mischung davon. Wenn es gut läuft, sollte es dann das Wochenende darauf fertig sein. Trotzdem würde ich mal mit 2 Wochen planen.
In der letzten Spalte sieht man die Auslastung der Tasks in Prozent. Beide CPU Cores ergeben zusammen 100%. IDLE0 läuft auf Core 0 und IDLE1 läuft auf Core1. Die loopTask läuft auf Core 1.
Man sieht, dass Core 1 am Anschlag ist und hauptsächlich loopTask drankommt.
Baut man nun ein vTaskDelay von 5ms in loopTask ein, dann sieht die Auslastung wie folgt aus:
Man sieht, dass der rfidhandling Task zuvor vom loopTask verdrängt wurde und ihm nun mehr CPU-Zeit zusteht. Das würde auch die bessere Empfindlichkeit für den RC522 erklären, welcher nun in loopTask abgearbeitet wird.
Ich habe allerdings die aufgerufene Funktion etwas generischer benannt für den Fall, dass man da noch z.B. irgendwann mal ein Display adressieren will. Aber spricht ja nix dagegen, aus dieser Funktion eine Adapterfunktion zu machen und dann die jeweiligen Funktionen aufzurufen.
Dann kannst dich gerne „austoben“. Ich haue dann außer Bugfixes die nächtsten drei Wochen nix raus.
