Question about class-based effects

[JoaDick]

Is your feature request related to a problem? Please describe.
The current design of WLED with is working perfectly fine for effects that can calculate everything from scratch whenever their mode function gets called. However, many effects require data that persists between function calls. This can be achieved through the SEGENV macro, but doing so is quite cumbersome.

Describe the solution you'd like
The obvious idea to make life easier for those effects would be a class-based design. Then the persistent data could be implemented as member variables, and a function like MyEffect::show() would be called by WLED.

So, my question is: What are the reasons why such a class-based design hasn't found its way into WLED yet? Are there concerns because of code size or performance? Or compatibility issues with the existing effects? Or it just wasn't seen as important?

While implementing my first effect for WLED, I've studied a lot of its codebase - and some ideas came into my mind how such effect-classes could be integrated. It would be really interesting to know what your thoughts are on that topic, and what else you'd expect from such an upgraded design.

[DedeHai]

So, my question is: What are the reasons why such a class-based design hasn't found its way into WLED yet? Are there concerns because of code size or performance? Or compatibility issues with the existing effects? Or it just wasn't seen as important?

As you are aware, there have been attempts in this direction by @TripleWhy. What exactly the implications are regarding RAM use and general performance of such an approach I do not know. It may well be feasible (and beneficial) but unless there is a draft to evaluate it's hard to say for me what possible issues may be.

[TripleWhy]

I do have a working prototype (all effects converted to separate classes etc.), and from what I can tell, it's not slower. Faster, actually in most cases, with some optimizations this design enables.
And cleaner. So much cleaner.
It also allows for the RAM usage to be tightly controlled, so that you know exactly how much memory segments or effects can possibly allocate at most. Without this, some effects like to use much more than allowed, or allocate more space before old one is cleared etc. The tradeoff is that that RAM is assigned at compile time, even if you are never going to use any heavy effects.
And of course class effects would allow for some interesting features.

The prototype was kinda mostly finished at some point. Unfortunately, someone decided to develop some major, partially conflicting changes in a non-git-like way. So I had no way of synchronizing or identifying and looking at those changes early. To be fair I was made aware of that and decided to go ahead anyway, which is certainly my fault. (Because I did have the time at that point, and knew I wouldn't later.) When the other changes were finally merged, merging those into my prototype was quite time consuming and mentally taxing (because of the state the code base), and I gave up at some point. But I still plan to pick it up again eventually. We'll see how relevant this is by then :)

EDIT: regarding the "faster" statement: That observation was made before those major changes were merged, and those did contain some effectively similar optimizations. So the situation might be different nowadays.

[DedeHai]

It also allows for the RAM usage to be tightly controlled, so that you know exactly how much memory segments or effects can possibly allocate at most.

that is interesting, how does it work? Currently my updated RAM management allows for "as much as you need" until the set limit runs out.

The tradeoff is that that RAM is assigned at compile time

that sounds like a bad thing. Why is any RAM assigned to non-existing FX classes? or am I misunderstanding your statement?

[JoaDick]

Ah yes, @TripleWhy , I think I remember your PR from last year. It was using factories for creating the effect instances (which was quite welcomed), but would require the effects to be implemented in a pull-based way (which was not so much appreciated)? IIRC it looked quite impressive - even if I didn't understand why you implemented the polymorphism manually instead of using virtual methods.

But probably I should admit that there's more than curiosity behind my question. During the holidays I eventually had the time to implement a proof of concept which I've been thinking about for quite a while. It grew much faster than I expected, and could soon be ready for review. However, it would be odd to throw that at you all right out of nowhere.
And I would really like to know if there are any serious constraints from WLED's developer side, which have to be regarded!

My drivers for this PoC were the following:

• Having fun implementing that 😃
• I need a class for storing persistent effect data!!!
• It shall integrate into WLED on top of Segment, without too much changes in existing code.
• Migration of existing effects should be possible gradually, no "big switch".
• Preferably no breaking changes.
• A cleaner, simpler and better documented API for implemening new effects (by newbies) would be great.
• Maybe that's also an enabler for future, more serious refactorings of the Segment class?

I suspect that this will clash (once again) with @TripleWhy 's PoC. But maybe we could finally merge the best from both worlds. My PoC doesn't care for factories; it just abuses the mode functions - which could definitely be done better. And your idea with the sparse setups could be integrated via a specialized base class - just for those effect which can benefit from that, but not for all.

So, please tell me more about your thoughts and requirements, and I'll do my best to prepare a draft PR soon.