Reworking the library to make it full run-time definable

[vortigont]

Hi @Makuna,

first of all - thanks for your efforts on implementing this lib. Nice work done there!

Just wanted to share my experience. When I started looking for the library for DFPlayer, yours was the only one I found with any kind of callback events out there. Maybe there are other that I've missed? Also found this analyzer based on your work that helped me with debugging.

Now about the things that did not fit me for my projects - you lib is based on class template for callbacks and for player model used.
Class templates are very flexible thing to use in many cases but at the same time it has very important thing to consider - each class based on different template arguments has a different signature.
So it requires a compile-time definition for:

• chip type used for player control
• mandatory callback class definition
• timeout value (have no idea why this is a class template argument at all)

Also demand for the call-back class to have all members static is a very limiting factor that makes it really cumbersome to use with OOP.

I'm developing an application where all parameters for DFPlayer must be run-time definable without the need to rebuild/flash the project. I.e. a user opens a WebUI, set rx/tx pins, chip type and some optional parameters and here it goes applying changes on the go.

So started adopting lib's code and ended up reworking it in many aspects. Now it meet my target and allows to:

• define chip model type on class instance instantiation
• attach user-provided chip implementation handler instance (reserved for future use)
• attach functional callbacks on-demand for specific group of events, callback policy is no longer mandatory
• removed static members from derived chip specific classes
• replaced some C style implementations with STL objects

I do realize that such kind of changes are to radical to current library design and not sure if it worth any PRs to your repo or keep it as a forked project.
I do not have much chip variances to do heavy testing but at least it runs pretty fine with the one I have at hand.

Pls find the forked code in this branch. Would be happy to discuss any of it.

Thanks!

[Makuna]

Some of the changes you made are not supportable across all platforms.
I also didn't see a single example updated to use your version. Did I miss something?
Don't confuse OOP with runtime dynamic configuration. This project is OOP, it is just not runtime dynamic.

• std:: is not available on many Arduino platforms, this is why the simple queue was created.
• Using Virtual causes memory usage that eats into the sketches size and memory, this library was meant for the smallest uC.
• Stream is a horrible interface. It maybe common on Arduino, but it is definitely not something to embrace. Further this project is used outside Arduino where it is avoided. Not all serial interfaces may want to support all the virtual methods and overhead. At least one use has a wrapper around UART on the device. Not even sure why you made this change if your goal was just runtime dynamic configuration.
• static isn't required in the template classes, don't confuse static and template use. Example, see serial class template usage.

Sorry, but the changes you made would not be the direction I would have used to solve the runtime dynamic configuration.

[vortigont]

I also didn't see a single example updated to use your version. Did I miss something?

It's just a draft that I've made in two days of adopting the code to my project, examples were out of scope (for now).

Don't confuse OOP with runtime dynamic configuration. This project is OOP, it is just not runtime dynamic.

I never stated that OOP is an equivalence for "runtime dynamic". "runtime dynamic configuration" is just a requirement for a specific project or task, not related to language model. I was saying that mandatory static callbacks in your template class is cumbersome with OOP like calling capturing lambdas or other object instance members, though not impossible.

std:: is not available on many Arduino platforms

Arduino is not a platform, MCU platform is AVRx, STMx, Espressif, SAMD21, nrfXX, etc... Arduino is just a "framework" around respective platform SDK and tool-chain. So STL support depends on compiler feature-set (or standard version) implementation for a tool-chain. Though Arduino CORE API does not use STL indeed, many of the platform-specific Arduino COREs use STL. And most of MCUs around for the last decade has tool-chains with c++11, even atinys has it. So I'm not sure what do you mean by "many", I would rather say "only a few does not support it". Though you right, that it's better to keep c++11 compat and avoid constructions like std::make_unique, thanks!

Using Virtual causes memory usage that eats into the sketches size and memory, this library was meant for the smallest uC

Arduino core uses it a lot. If you are concerned about a dozen bytes for two virtual methods than Arduino CORE is not your friend in the first place. Using native platform SDKs will save you much more ram/bin, orders of magnitude. I do not understand this concern.

Stream is a horrible interface, it is definitely not something to embrace. Not all serial interfaces may want to support all the virtual methods and overhead.

Stream is a parent class both for HardwareSerial and SoftwareSerial, and you DO use it yourself in your code. setTimeout() is method from Stream, not from hw/sw serial. Same applies to readBytes(). If you'd rather use abstractions around UART hal and avoid Stream, then why using it's methods? Otherwise end-user will re-implement it anyway around platform specific uart hal.

static isn't required in the template classes, don't confuse static and template use

I never said that also. You do not have static member functions in your template<> class DFMiniMp3, I was referring to static const members of your Mp3ChipBase derivatives.

Sorry, but the changes you made would not be the direction I would have used to solve the runtime dynamic configuration.

If I was implementing it from scratch I would go in even more abstracted way and segregate data transfer, queues and logic handling into isolated instances. I but I think it does not worth the efforts for a simple tiny player control. My goal was to allow run-time definitions for all available variants of player options for contemporary MCUs where reading/storing run-time configuration data is not a problem. Mandatory static callbacks also does not fit well with object instances created on-demand. I was not targeting for every possible two decade old mcu's around. Obviously it looks pretty strange to use run-time setting/changing configs on things like atiny or msp430.

OK, I got your point. My case is not that all-the-way-compatible but it's more flexible for up-to-date implementation projects with most popular MCUs around.
Hope you can implement something like this in a more appropriate and legacy-compatible way sometime.

Sorry for the noise and thanks for the feedback anyway!
Cheers!

[Makuna]

Look at the Digispark ATTINY85, 512B of ram. Bytes! A chip I often use on low power small projects, and I am not the only one. So yes, bytes are a concern.