template and static notifiers are a head scratcher

[tcurdt]

Thanks for the library and Merry Xmas.

Maybe it's my C++ that got so rusty but why would you use the template approach and static callbacks for the notifications?
As far as I can tell this makes it very hard to escape the static scope of the notifier class.
Usage wise a normal delegation seems like a much more user friendly approach.

Maybe you could expand in the example how you imagine this to work?

To continue playing I would need to trigger the next track here:

https://github.com/Makuna/DFMiniMp3/blob/master/examples/PlayMp3/PlayMp3.ino#L45

but for me that's not available in that scope.
Thoughts?

[seisfeld]

I'm far from being a c/c++ expert but this is how we do it in our project. We handle the whole playstate ourselves...

https://github.com/seisfeld/TonUINO/blob/60a75f9cb10261ba1296d2e75b43b470cbaebce4/tonuino.ino#L506-L507

https://github.com/seisfeld/TonUINO/blob/60a75f9cb10261ba1296d2e75b43b470cbaebce4/tonuino.ino#L1307 and onwards for the function...

Dunno if that is what you were looking for though (probably not ;)).

Merry Xmas!

[Schallbert]

I'm escaping the static callback by using a message property along with a getter function - see here my unfinished project https://github.com/Schallbert/Tonuino_UseEncoder/blob/d46ef63dfb1776a93ee6e23ad03a5acae6b6c0aa/lib/Mp3/DfMiniMp3/DFMiniMp3_implementation.h#L80

Merry Christmas!

[tcurdt]

Thanks for the input guys.

There are various (I would call them work arounds) and I am also just using an external to escape the scope.
But from an API perspective I don't find that particular elegant (to put it nicely).

I am basically wondering if there is a particular reason the original author went this way.
Maybe I am just missing the elegant way to do this with the template approach.

But now - back to preparing the food for later :-)

[Makuna]

This started as an "Issues", which it is not, and I moved it here to "Discussions" which it is.

What do you mean by delegate approach? C++ does not inherently support delegates (unmanaged c++) so you have approach in mind, please provide the context; code snippet example of a use pattern.

You need to understand the core goal of this library. Use as little memory and code space to achieve the functionality. This is due to being able to run on AVR Tiny model of chips with very little ram at all. Storing a context and/or callback takes memory. The template model used takes NO memory. If you the sketch writer want to consume the memory to make it cleaner, you can layer it to achieve it; but not every consumer of this library needs nor wants to do that.

There are some interesting techniques using STL; but STL is not available on many Arduinos.

[Schallbert]

To wrap this elegantly, the easiest solution might be using the Observer pattern if you can live with the overhead. I agree with @Makuna's reasoning about keeping this small and simple.

[tcurdt]

This started as an "Issues", which it is not, and I moved it here to "Discussions" which it is.

@Makuna I personally don't see as clear cut (see below) - but as you like.

What do you mean by delegate approach? C++ does not inherently support delegates (unmanaged c++) so you have approach in mind, please provide the context; code snippet example of a use pattern.

There are many ways https://stackoverflow.com/questions/9568150/what-is-a-c-delegate and I don't claim to know what is best in this situation. Seems like we all found ways around it. But it would be great to have an example where the notifications are handled more on an instance level and not in the static scope. Just imagine how you would approach multiple players per system.

Maybe consider this a feature request to expand the examples.

[Makuna]

@tcurdt On the quick topic of Issues versus Discussions. This has always been problematic for users to understand, but "issues" is meant for what I will call "BUGS" as most people understand that term more clearly. They are not questions, but a clear "this is wrong with the code", and include ALL the details to reproduce and what is expected. Note I stated "code", not design of code or API which is relative/opinionated.
What they are not is the place to ask questions, seek understanding, nor voice an opinion. As a repo owner this has been difficult to manage to keep this clear while being nice as most of us repo managers get way too many that are not issues or well formed issues. Without an easily discoverable feature like a forum it continued for years. NOW, Github finally added "discussions" to allow us to keep this very separated as it needs to be. This is where the conversations should always start; often even for real bugs.

Now back to this topic:
If you read that link, they talk about storing a pointer to functor (an STL common technique), A pointer takes dynamic memory; and in this case and many like that pointer talks about for something that will NEVER change once set in your application. This is a limitation of language and the only way to I know to remove that memory use is to use the template model such as I have done. While you (and many developers on full size computers, like that link point to) don't care about memory usage and probably shouldn't; this is something that is a concern when writing for a uController that may have as little as 32 bytes, not Kbytes, BYTES. Code space is not usually as big of a concern as its Read Only (other than when you flash or abuse it for EEPROM like support on the ESP does).

Now other techniques, like virtual functions also require the same pointer, but is hidden from user in the vtable which is RAM based.

Originally in my first draft, you passed the callback as you called mps.loop(myCallback) but I found that notifications can happen when you are waiting for a reply for call you made to get the count of tracks as its all asynchronous packets to the chip; but it still needs to be handled in a memory efficient way (as storing the event and waiting until the loop would take memory again). I thus ended up with the best compromise I know and until someone demonstrates a better approach that takes NO memory then it isn't going to change.

[tcurdt]

quick topic of Issues versus Discussions

This your view on issue tracking.
As a maintainer I myself handle it differently.
To me an issue can be a bug - but it doesn't have to be.

now back to this topic:

So how would you handle multiple players on a single system?

[Makuna]

Did you find something that didn't work with multiple instances in a single sketch?
If you need unique handling, you provide two notify classes, one for each instance. Otherwise reuse the same one.

[tcurdt]

It's seems a common need is to play the next track on a finished track notification. Given the notification is static and does not pass a reference to the player, the reference has to be provided externally. Which is the initial reason for starting the issue/discussion. This is even more problematic when sharing the class. How would I know which player finished that track?

[Makuna]

As I stated, if you need unique handling (reference to each player) then you provide two notify classes.

I am not sure it is possible to pass a reference to the player that is making the call, but I will look into this.

[tcurdt]

The question was how you envision to play the next track on a play finished notification.
It would be a great addition to the examples.

[Makuna]

I merged in changes that pass the mp3 object into the notification methods.

This changes the examples and requires previous code to update to how the notification methods have changed and the specific order that the types are defined at the top of the sketch.

[Makuna]

@tcurdt I updated the PlayMp3 example to use the notification to play the next track rather than what it did before.