Add temporary effect from outside of effect manager. #545
Replies: 9 comments 7 replies
-
Could something like the Memento Design Pattern let you make a stack of
such patterns to push and pop and have checkpoints along the way along with
easy trades without full locking and such?
…On Sat, Nov 25, 2023, 9:04 PM Joe Schneider ***@***.***> wrote:
I want to be able to add a temporary effect from outside of effect
manager. I propose adding method to the effect manager that effects an
effect and then sets that as _tempEffect. Then there is a corresponding
method that sets the temp effect to null. Similar to ClearGlobalColor but
only touched the temp effect. This would allow temp effects being triggered
by an IR remote (such as the DIY1 or FADE3 buttons). I assume it could be
helpful for triggering temporary effects from the web interface as well.
But, I deal mainly with the IR and don't mess with the web interface as
much. I have implemented the following in effectmanager.h in my sandbox:
void SetTemporaryEffect (std::shared_ptr<LEDStripEffect> tempEffect)
{
_tempEffect = tempEffect;
}
void ClearTemporaryEffect()
{
_tempEffect = nullptr;
}
—
Reply to this email directly, view it on GitHub
<#545>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACCSD32ERZKIRMK7NZI7733YGKPUVAVCNFSM6AAAAAA72P6LK2VHI2DSMVQWIX3LMV43ERDJONRXK43TNFXW4OZVHA4TAMBUGE>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Beta Was this translation helpful? Give feedback.
-
@robertlipe I had to google MDP. I am a simpleton with a severe lack proper code design practices. MDP is a little over my head and I think more complicated than how the effect manager works. I prefer the relatively straightforward functionality. The reason for this idea is that while I can add an effect to the queue, get it's index number and trigger it, I would much prefer to just set a temporary effect and let it disappear once I am done with it. This can sort of already be gone by applying the global color, but I want to be able to temporarily activate whatever effect I have coded in the IR remote or maybe an effect that I might want to alter push from the web interface. On the 44 button remote I have, I have 6 DIY buttons. I currently have those mapped to trigger effects 1-6 in the effect manger. But, someone might want those to trigger a special effect outside of the effect queue. Also I am thinking of using FADE3 and FADE7 to implement a palette fill effect that fades through 3 or 7 colors. I need to be able to create and push a temporary effect for that. I need to assume that the effects queue can change at any moment (effects copied, added, or removed) and not hard code references to specific effect indexes. |
Beta Was this translation helpful? Give feedback.
-
I think I'd have to see a full use case before I make up my mind about this. I'd be particularly interested in how the generic ability to directly set the temporary effect would combine with what you want to use it for - using it to "hard set" specific effects, that may or may not be in the regular effect list. I'd expect the exact effects one wants to trigger to vary from person to person, and I think this would have to be "solved" in a way that is also somewhat generic. Then there are things like effect persistence. Currently, the temporary effect is not persisted - it's temporary by nature -, but if we start using it to actually select the effect someone wants to look at "forever", that may not be sustainable. But if we start persisting, then what do we do with cases where that doesn't make sense - like with the Mesmerizer splash screen effect? Again, I think this needs some more "body" before I can say yes or no to it. Maybe you could just implement what you have in mind in a branch in your fork, so we can look at it there? |
Beta Was this translation helpful? Give feedback.
-
The biggest use case at the moment is if a person actually does want to show the global color RIGHT NOW when they press a color button on their remote. I am glad that we have moved effect manager to its current state of global color, but I know some people might still want the immediacy of the color showing. But, overall, I am trying to mimic the abilities of most other generic RGB light strip remote controls, especially the ones with more than 24 buttons. Those usually start showing whatever pre-programmed effect is assigned to a button RIGHT NOW. I am also trying to allow maximum customization of the IR remote experience and bypass the effects queue when desired. But, there is a greater problem I am trying to avoid. I can accomplish this be simply adding a disabled effect, remembering its index, and then using a button to call that index. But I also know there are other mechanisms for changing the effect manager queue, such as the web interface or receiving API calls. So, I can't explicitly trust that an effect will continue to exist in the queue or stay at its effect index, or that the effect index itself will exist. Telling effect manager to trigger an effect at an index that does not exist causes an uncaught exception and device reboot. This "solution" is also bad for memory management. I have been too busy making changes to other areas of the code to make a concrete example, besides the color fill effect. Once the other pull requests get processed, I will start moving my existing projects to my new PersonalProjects branch. There I will have my custom remote control code implemented and that will have examples of what I am trying to accomplish. Those examples can be ported to the stock remote control handling code. I know this would be waaaaaayyyyyyy down the road, but I also imagine a scenario where the web server might also trigger a temporary effect. Maybe a person previews what an effect might look like before it is added to the queue or before changing an effect in queue. Creativity and real world needs open up the possibilities. Currently, the only way to set the temporary effect is with the splash screen initializer. TLDR; I think it is useful to allow a person to set the temporary effect beyond the initializer for the splash effect use case. |
Beta Was this translation helpful? Give feedback.
-
I will also add this: If a person relies on effects.cpp to intuit what an effect index number is and hard codes that index so they can activate that effect, that assumption might be nullified by a later commit. Example: Dave submitted a PR for changing the order of effects for mesmerizer. If I had set my remote code to activate an effect at index 4, Dave's commit and my subsequent sync with main would mess up which effect is triggered. |
Beta Was this translation helpful? Give feedback.
-
Memento allows stacking backups and restores and implementing from any
point and any limit of depth. The code is pretty close to what
effecrmanager already does, albeit in a strong Json accent. Want to add a
previous as well as a Next? Ok. Remote or network or timeout or error card
wants to display a new one? Just push/append/emplace it to the stack. It's
not based on effect number.
This is how databases implement rollbacks, editors allow undo, and I on.
I understood that effect numbers were immutable once assigned and couldn't
be changed, even in a code upgrade.
…On Thu, Nov 30, 2023, 7:55 AM Joe Schneider ***@***.***> wrote:
I will also add this: If a person relies on effects.cpp to intuit what an
effect index number is and hard codes that index so they can activate that
effect, that assumption might be nullified by a later commit. Example: Dave
submitted a PR for changing the order of effects for mesmerizer. If I had
set my remote code to activate an effect at index 4, Dave's commit and my
subsequent sync with main would mess up which effect is triggered.
—
Reply to this email directly, view it on GitHub
<#545 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACCSD32CNDISINHYJGGUJFDYHCF45AVCNFSM6AAAAAA72P6LK2VHI2DSMVQWIX3LMV43SRDJONRXK43TNFXW4Q3PNVWWK3TUHM3TOMJYGE3DI>
.
You are receiving this because you were mentioned.Message ID:
<PlummersSoftwareLLC/NightDriverStrip/repo-discussions/545/comments/7718164
@github.com>
|
Beta Was this translation helpful? Give feedback.
-
@revision29 The point is that the PR you opened proposes to add functions to a core class in the main tree of project, where the main tree codebase then doesn't use them - that effectively makes the functions dead code on arrival. Concerning indexes and effect numbers:
At some point there was a discussion about adding an effect instance UUID, and I even opened a PR for adding it: #332. I closed it quite a while later because it didn't seem to gather much enthusiasm in the end. |
Beta Was this translation helpful? Give feedback.
-
I added a use case in the PR, but let me explain my thinking more clearly. A person might want to have 5 effects that change every 10 minutes or so. Those are added to the effect queue in effects.cpp. But, a person might want to program their remote to display certain custom effects not in the queue. The other way to accomplish this is to add disabled effects to the queue.
The downside to the alternate workaround is that effects are unnecessarily stored in memory and the index number can't be assumed or trusted since index numbers can change dynamically. It is much simpler and less error prone to just define a temporary effect and send it to effect manager since it is transient in nature. |
Beta Was this translation helpful? Give feedback.
-
One last thing for now: after post the walls of text above, my memory kicked back in gear. The |
Beta Was this translation helpful? Give feedback.
-
I want to be able to add a temporary effect from outside of effect manager. I propose adding method to the effect manager that effects an effect and then sets that as _tempEffect. Then there is a corresponding method that sets the temp effect to null. Similar to ClearGlobalColor but only touched the temp effect. This would allow temp effects being triggered by an IR remote (such as the DIY1 or FADE3 buttons). I assume it could be helpful for triggering temporary effects from the web interface as well. But, I deal mainly with the IR and don't mess with the web interface as much. I have implemented the following in effectmanager.h in my sandbox:
Beta Was this translation helpful? Give feedback.
All reactions