Hi,
Ich hatte in dem anderen Thread schon geschrieben, ich bin kein großer Fan von impliziten „Zusätzen“ an Strings um Information zu transportieren. Das ist anfällig darauf, dass man vergisst, wieso etwas gemacht wurde und dann geändert wird. Plus, die Interpretation von „//“ ist abhängig von der Implementation (das hatte ich nicht gewusst, eigentlich dacht ich, das in POSIX/Linux 2 oder mehr / als einer gewertet werden → ich mag Stackoverflow, da lernt man immer was neues dazu ):
Nach ein wenig Stöbern würde ich vorschlagen, dass wir etweder ein std::pair<String, bool> (mit der Info Pathname / isDirectory) oder direkt den Pointer auf den struct dirent zurückgeben. Bei dem Pointer sind dann alle Informationen schon enthalten. Der relative Pfad (bzw nur der Dateiname) kommt uns in Zukunft zu gute. Aktuell unterstützen wir aber keine relativen Pfade, aber ein concat ist recht leicht zu implementieren.
Dazu würde ich eine neue Funktion in unserer FS.h empfehlen, das macht das Aktuell halten einfacher als wenn die Patches in bestehenden Funktionen eingreifen.
Finde das klingt gut. @tueddy was hälst du davon? Wenn das Filesystem schon direkt diese Information liefert kann der Code noch kompakter werden und man spart sich den doppelten Code in Web.cpp und SdCard.cpp.
Falls du es gut findest aber gerade nicht selbst machen willst kann ich mir das auch im Filesystem anschauen…
Ich würde auch einen PR bei arduino-esp32 machen damit das offiziell werden kann. Eine überladene Funktion hat da evt. die besten Chancen. So oder so es sollte verwendbar sein.
Jupp, wenn du versuchst es in den Upstream zu bekommen macht es Sinn dass Interface nicht zu stark zu ändern. Mit der Funktion kann auch ich gut leben.
Sehr cool! Gerade mal aufgespielt und was mir sofort positiv auffällt, ist dass es nicht mehr die lange „Verzögerung“ beim ersten mal abspielen eines Ordners gibt (die Generierung des Playlist-Caches).
Ich kann auch im Betrieb nichts negatives bezüglich der Geschwindigkeit feststellen. Auch das löschen von Dateien und wieder neu hinzufügen funktioniert super (nur die aktuell spielende Datei sollte man nicht löschen )…
Konnte bei mir mit 50 Titeln in einem Ordner testen und konnte keinerlei „Verzögerung“ oder ähnlich beim Start festellen.
Gefällt mir sehr viel besser als mit den PlaylistCacheFiles! Hammer!!
Also ja, tatsächlich deutlich besser ohne .
Das define CACHED_PLAYLIST_ENABLE könnte man in diesem Zuge auch gleich ganz raus nehmen. Ich kann mir morgen auch nochmal die originalen Zeiten zum Vergleich anschauen. Das sollte ganz spannend sein.
Gute Nacht zusammen!
Hattest du bei den Messungen Musik abgespielt? Weil die Unterschiede zu tueddy scheinen doch sehr hoch zu sein (oder nutzt du SPI). Würde mich nur interessieren, da du so große Untershiede hast. Kann natürlich auch an der SD-Karte selbst liegen .
Ohne der Cache sind die Werte aber deutlich besser (wahrscheinlich weil es einfach fixer geht den FAT32 Directory table auszulesen als die Linked List der Datei). Sehr überzeugend . Muss nur die Zeit finden meine Playlist Branch nochmal anzugreifen… RL funkt da gerade leider ein wenig dazwischen…
Messungen sind jeweils zum Start des Abspielens implementiert. Ich verwende das normale Kit ohne SPI.
Aber ich finde die Unterschiede gar nicht so gravierend. Dass das erstellen der Playlist und speichern länger dauert wenn es mehr Dateien sind und die SD nur langsam schreibt passt.
Beim lesen des Chache-Files bei ca. doppelter Anzahl an Einträgen auch etwa doppelt so lange wirkt auch plausibel.
Und beim lesen der Table ohne das Cache file ebenfalls etwa doppelt so lange bei doppelt so vielen Einträgen…
Bin sicher das die Messungen passen, die SD-Karten sind unterschiedlich schnell und befüllt, @Joe91 hat 50 Dateien, ich 24 im Ordner gehabt.
Habe das auch noch mit SPI getestet, da ist der Turbo nochmal größer. Bei SPI hatte ich mit einer stark befüllten Karten Watchdog Reset (weil das Laden des Explorer-Trees ewig gedauert hat), jetzt flutscht das.
Aus meiner Sicht ist der Playlist Cache dann obsolet. Ich werde ihn im kommenden PR aber nicht entfernen sondern nur deaktivieren…
Ich habe ein Bug / Verbesserungsvorschlag. In getNextFileName solltest du definitiv prüfen, ob du NULL bei isDir bekommen hast. Wenn es NULL ist, versuchst du Adresse 0x00 zu schreiben, generierst damit eine Exception und der Code stürzt ab.
Eine einfache Möglichkeit wäre eine Prüfung in Zeile 562 vor den Schreiben auf NULL:
name += fname;
// check entry is a directory
if(isDir) {
*isDir = (file->d_type == DT_DIR);
}
return name;
Plus die Formatierung deines neuen Codes passt nicht mit dem restlichen in der Lib zusammen (du nutzt Tabs statt Leerzeichen ).
@laszloh Danke für den Hinweis, hab’s aufgenommen.
Huhuhu, es gibt gute Chancen das diese Funktion Teil des nächsten Arduino Release wird. Mein Pull-Request ist bereits „approved“. Könnte also mit 2.0.8 kommen, bis dahin kann man aber auch gut mit der angepassten FS Bibliothek leben
@laszloh Vielen Dank für die Code-Review, hast ja noch einen Bug gefunden!
Keine Ahnung warum in der Historie da Änderungen in der Readme angezeigt werden. Vermute das kam durch ein Sync vom Dev-Branch und Torsten hatte diese Datei aktualisiert.
Am Ende taucht die aber nicht mehr auf und ist synchron zur aktuellen Readme:
Tolle Arbeit. Seit einigen Tagen habe ich es laufen und ich hoffe es kommt noch in den Master bevor ich nächste Woche die Box meiner Enkelin zum Update erhalte. Cached_Playlist ist deaktiviert und ich denke auch nicht mehr erforderlich. Gefühlt ist da kein Unterschied.
):
)…
.
.