Skip to content

Deprecate and remove ws2812 effects (and provide an in-Lua replacement) #3122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nwf opened this issue May 23, 2020 · 8 comments
Open

Deprecate and remove ws2812 effects (and provide an in-Lua replacement) #3122

nwf opened this issue May 23, 2020 · 8 comments

Comments

@nwf
Copy link
Member

nwf commented May 23, 2020

There is no reason whatsoever that this module should be in C; timers and ws2812 frame buffers are easily managed from Lua. Worse, its existence complicates implementing #2916, which is obviously a good idea for the health of the entire tree.

If it were up to me, I'd just rip it out wholesale right now. But I recognize it's not, so I wonder if someone who cares about the module would like to write Lua equivalents of each of its effects for inclusion in lua_examples.

If it turns out that there are operations that cannot be easily achieved from Lua, then we should augment the buffer interface until that's not true, and I would gladly add such things to my nascent #2916 implementation.

@HHHartmann
Copy link
Member

The ws2812_effects module consists of two main functionalities which are interwoven and should be separated.

  1. the player, which repeatedly updates the LED strip with the newly calculated effect.
  2. the effects themselves.

So there should be a player and separate effects which can be passed to the player.
It should also support all (3 I think) different Led Modules.
The effects should probably have an interface consisting of init(params) and next_buffer next_step(old_buffer) to calculate the next step.

Originally I was planning to adapt the module also, but I agree that both should be doable in Lua.

Should we sneak in a deprecation warning in the module then before the release?

@marcelstoer Do you have statistics of the usage in the cloud builder of this module?

@nwf
Copy link
Member Author

nwf commented May 23, 2020

@HHHartmann Sneaking a deprecation warning seems like a good plan to me. Since we don't have a replacement module ready, point at this issue?

FWIW, I've been mocking up a proposal for lua_examples that I'll push in just a moment. You are more than welcome to take it and mutate it to your heart's desire. :)

nwf added a commit to nwf/nodemcu-firmware that referenced this issue May 23, 2020
This is more of a suggestion than a viable submission; someone please
take this from me.

See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue May 23, 2020
This probably should not yet land in the tree, since it's sort of rude
if anyone's actually using this thing.

See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue May 26, 2020
This is more of a suggestion than a viable submission; someone please
take this from me.

See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue May 26, 2020
This probably should not yet land in the tree, since it's sort of rude
if anyone's actually using this thing.

See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Jun 4, 2020
This is more of a suggestion than a viable submission; someone please
take this from me.

See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Jun 4, 2020
This is work in progress towards
nodemcu#2916

This includes some untested hackery to keep the deprecated
ws2812_effects compiling.  We anticipate this module going away soon,
however.  See nodemcu#3122
@nwf nwf added this to the Next release milestone Jun 12, 2020
@nwf nwf mentioned this issue Jun 13, 2020
4 tasks
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Jun 13, 2020
This is work in progress towards
nodemcu#2916

While here, document the ws2812 UART-based overlapping with mainline
execution.  Fixes
nodemcu#3140

This includes some untested hackery to keep the deprecated
ws2812_effects compiling.  We anticipate this module going away soon,
however.  See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Jun 13, 2020
This is more of a suggestion than a viable submission; someone please
take this from me.

See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Jun 13, 2020
This is work in progress towards
nodemcu#2916

While here, document the ws2812 UART-based overlapping with mainline
execution.  Fixes
nodemcu#3140

This includes some untested hackery to keep the deprecated
ws2812_effects compiling.  We anticipate this module going away soon,
however.  See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Jun 13, 2020
This is more of a suggestion than a viable submission; someone please
take this from me.

See nodemcu#3122
@HHHartmann
Copy link
Member

I have been thinking about this. It certainly would be good to allow individual effects in Lua but on the other hand executing some effects that change many pixels individually will take around factor 100 longer to execute in Lua than in C. But then, looking at the existing effects at most the flickering effects modify several pixels, but not that many, so that might be neglected.

@marcelstoer marcelstoer removed this from the Next release milestone Jun 20, 2020
@nwf
Copy link
Member Author

nwf commented Jun 20, 2020

If there are bulk editing combinators that would be useful to have in C, we should add them to pixbuf. I'm not opposed to a "miscellaneous special purpose pixbuf functions" module, either (pixbuf.morefun?) if it's necessary. I mostly want the timer callbacks and pixbuf references handled in Lua and not in C. Actual math that's gotta go fast can, of course, remain in C.

Failing any of that, I'd still move to merge #3158 if someone can test that ws2812_effects still works with it.

nwf added a commit to nwf/nodemcu-firmware that referenced this issue Jun 21, 2020
This is work in progress towards
nodemcu#2916

While here, document the ws2812 UART-based overlapping with mainline
execution.  Fixes
nodemcu#3140

This includes some untested hackery to keep the deprecated
ws2812_effects compiling.  We anticipate this module going away soon,
however.  See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Jun 24, 2020
This is work in progress towards
nodemcu#2916

While here, document the ws2812 UART-based overlapping with mainline
execution.  Fixes
nodemcu#3140

This includes some untested hackery to keep the deprecated
ws2812_effects compiling.  We anticipate this module going away soon,
however.  See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Jun 24, 2020
This is more of a suggestion than a viable submission; someone please
take this from me.

See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Jun 28, 2020
This is work in progress towards
nodemcu#2916

While here, document the ws2812 UART-based overlapping with mainline
execution.  Fixes
nodemcu#3140

This includes some untested hackery to keep the deprecated
ws2812_effects compiling.  We anticipate this module going away soon,
however.  See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Jun 28, 2020
This is more of a suggestion than a viable submission; someone please
take this from me.

See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Jun 30, 2020
This is work in progress towards
nodemcu#2916

While here, document the ws2812 UART-based overlapping with mainline
execution.  Fixes
nodemcu#3140

This includes some untested hackery to keep the deprecated
ws2812_effects compiling.  We anticipate this module going away soon,
however.  See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Jun 30, 2020
This is more of a suggestion than a viable submission; someone please
take this from me.

See nodemcu#3122
@skycoders
Copy link
Contributor

@nwf @HHHartmann Just noticing this topic: I originally implemented the effects library. Unfortunately there is no time to work on it right now, so fine with anything you do to bring the overall led support forward. Reason for the C implementation is that the performance in lua on the 8266 has been unacceptable. I initially implemented this in user code, but the array operations and copying them down to the buffer would not allow smooth animations. This should be considered when changing the implementation. I ran that on 300px strips. If you need someone to test a new implementation, let me know.

@nwf
Copy link
Member Author

nwf commented Jul 11, 2020

@skycoders Hi!

I suspect what we will find is that there are transforms we want to add to pixbuf (nee ws2812.buffer) to make it a suitable substrate to replace the effects library, so that performance remains acceptable, but I have not spent time looking at exactly what those would be. New day job 'n all that. Anyway, I'm hopeful that a fairly straightforward set of bitblitting primitives beyond what we already have would go a long way.

I don't happen to have any 300px strips, but that shouldn't stop us from using a 300 pixel buffer for testing.

When I or, more likely, @HHHartmann have a concrete proposal, we'll be sure to ask; thanks for offering! :)

@nwf nwf changed the title Deprecate and remove ws2812 effects Deprecate and remove ws2812 effects (and provide an in-Lua replacement) Jul 11, 2020
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Jul 18, 2020
This is work in progress towards
nodemcu#2916

While here, document the ws2812 UART-based overlapping with mainline
execution.  Fixes
nodemcu#3140

This includes some untested hackery to keep the deprecated
ws2812_effects compiling.  We anticipate this module going away soon,
however.  See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Sep 23, 2020
This is work in progress towards
nodemcu#2916

While here, document the ws2812 UART-based overlapping with mainline
execution.  Fixes
nodemcu#3140

This includes some untested hackery to keep the deprecated
ws2812_effects compiling.  We anticipate this module going away soon,
however.  See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Sep 23, 2020
This is work in progress towards
nodemcu#2916

While here, document the ws2812 UART-based overlapping with mainline
execution.  Fixes
nodemcu#3140

This includes some untested hackery to keep the deprecated
ws2812_effects compiling.  We anticipate this module going away soon,
however.  See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Sep 26, 2020
This is work in progress towards
nodemcu#2916

While here, document the ws2812 UART-based overlapping with mainline
execution.  Fixes
nodemcu#3140

This includes some untested hackery to keep the deprecated
ws2812_effects compiling.  We anticipate this module going away soon,
however.  See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Sep 26, 2020
The new pixbuf module has more functionality than the ws2812-specific
buffer it replaces.

This is work in progress towards
nodemcu#2916

While here, document the ws2812 UART-based overlapping with mainline
execution.  Fixes
nodemcu#3140

This includes some untested hackery to keep the deprecated
ws2812_effects compiling.  We anticipate this module going away soon,
however.  See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Nov 18, 2020
The new pixbuf module has more functionality than the ws2812-specific
buffer it replaces.

This is work in progress towards
nodemcu#2916

While here, document the ws2812 UART-based overlapping with mainline
execution.  Fixes
nodemcu#3140

This includes some untested hackery to keep the deprecated
ws2812_effects compiling.  We anticipate this module going away soon,
however.  See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Dec 20, 2020
The new pixbuf module has more functionality than the ws2812-specific
buffer it replaces.

This is work in progress towards
nodemcu#2916

While here, document the ws2812 UART-based overlapping with mainline
execution.  Fixes
nodemcu#3140

This includes some untested hackery to keep the deprecated
ws2812_effects compiling.  We anticipate this module going away soon,
however.  See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Dec 22, 2020
The new pixbuf module has more functionality than the ws2812-specific
buffer it replaces.

This is work in progress towards
nodemcu#2916

While here, document the ws2812 UART-based overlapping with mainline
execution.  Fixes
nodemcu#3140

This includes some untested hackery to keep the deprecated
ws2812_effects compiling.  We anticipate this module going away soon,
however.  See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Dec 22, 2020
The new pixbuf module has more functionality than the ws2812-specific
buffer it replaces.

This is work in progress towards
nodemcu#2916

While here, document the ws2812 UART-based overlapping with mainline
execution.  Fixes
nodemcu#3140

This includes some untested hackery to keep the deprecated
ws2812_effects compiling.  We anticipate this module going away soon,
however.  See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Dec 24, 2020
The new pixbuf module has more functionality than the ws2812-specific
buffer it replaces.

This is work in progress towards
nodemcu#2916

While here, document the ws2812 UART-based overlapping with mainline
execution.  Fixes
nodemcu#3140

This includes some untested hackery to keep the deprecated
ws2812_effects compiling.  We anticipate this module going away soon,
however.  See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Dec 24, 2020
The new pixbuf module has more functionality than the ws2812-specific
buffer it replaces.

This is work in progress towards
nodemcu#2916

While here, document the ws2812 UART-based overlapping with mainline
execution.  Fixes
nodemcu#3140

This includes some untested hackery to keep the deprecated
ws2812_effects compiling.  We anticipate this module going away soon,
however.  See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Dec 24, 2020
This is more of a suggestion than a viable submission; someone please
take this from me.

See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Dec 31, 2020
The new pixbuf module has more functionality than the ws2812-specific
buffer it replaces.

This is work in progress towards
nodemcu#2916

While here, document the ws2812 UART-based overlapping with mainline
execution.  Fixes
nodemcu#3140

This includes some untested hackery to keep the deprecated
ws2812_effects compiling.  We anticipate this module going away soon,
however.  See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Dec 31, 2020
This is more of a suggestion than a viable submission; someone please
take this from me.

See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Jan 5, 2021
The new pixbuf module has more functionality than the ws2812-specific
buffer it replaces.

This is work in progress towards
nodemcu#2916

While here, document the ws2812 UART-based overlapping with mainline
execution.  Fixes
nodemcu#3140

This includes some untested hackery to keep the deprecated
ws2812_effects compiling.  We anticipate this module going away soon,
however.  See nodemcu#3122
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Jan 5, 2021
The new pixbuf module has more functionality than the ws2812-specific
buffer it replaces.

This is work in progress towards
nodemcu#2916

While here, document the ws2812 UART-based overlapping with mainline
execution.  Fixes
nodemcu#3140

This includes some untested hackery to keep the deprecated
ws2812_effects compiling.  We anticipate this module going away soon,
however.  See nodemcu#3122
@stale
Copy link

stale bot commented Jul 8, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 8, 2021
@stale stale bot closed this as completed Jul 30, 2021
@marcelstoer
Copy link
Member

I am reopening this as https://nodemcu.readthedocs.io/en/release/modules/ws2812-effects/ points here.

@marcelstoer marcelstoer reopened this Dec 31, 2021
@stale stale bot removed the stale label Dec 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants