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).