Skip to content
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

Emulate PixelStrip to send data to WLED #104

Closed
wants to merge 5 commits into from

Conversation

Syfaro
Copy link
Contributor

@Syfaro Syfaro commented Feb 9, 2021

Hi there! This allows you to use WLED as a drop-in replacement. I'm not sure if this is a PR you're interested in, given there's a separate plugin for this feature.

The user configuration is definitely needing improvement still, but this is a working proof of concept that I'm using with my printer. Let me know if there's anything I'm missing. I've created this as a draft as it's a new and undiscussed feature, so definitely not ready to be merged yet.

Unrelated to this PR, it seems like this was an incorrect change: 95ea395#diff-b098827de765c020bc5e911116065437e43da9db99cffec1b85c89e3b589fdf7R249. I had to change "effects" back to "effect" to make progress effects work. I didn't change it in this PR because it is an unrelated issue and I'm not sure if something else is going wrong there.

@Syfaro Syfaro marked this pull request as draft February 9, 2021 19:47
@cp2004
Copy link
Owner

cp2004 commented Feb 9, 2021

Hmmm, I'm not sure if I want to add this here or not. I would rather keep them separate if possible - it adds an extra dimension to maintaining this plugin, support, etc. I created the WLED plugin from a feature request, and I intend to release it some point soon the plugin repository. Still some work to do but I will do it.

I don't get the feeling that WLED is a direct drop in replacement for the hardware LED strip - and it would need a lot more error handling, since it is going via the network. Given that there could be calls every few ms to set the colour of the strip, I want to know how this would hold up.

I would be interested in this over on the WLED plugin - I couldn't understand the socket stuff myself, so was going through the REST API using the python-wled module. This would allow something cool to happen there, with realtime control of the LEDs. I like what you have done, but I don't want it here, if that makes sense.

There's two reasonable problems I get the feeling of:

  • Implementing features that work on directly connected hardware that don't work on WLED, or vice versa
  • Users requesting other features of WLED to be controlled in this plugin - creating problem above

Both of these are likely to make things a lot more complicated in this plugin, not just from a code writing but also user experience point of view. And documenting, two separate ways of configuring the plugin - I can barely manage one without multiple issues a week.

Let me know your thoughts. Don't get me wrong, I like this implementation and think it's awesome, but I would rather keep WS2821x LED Status on a single track. I don't mind maintaining multiple plugins either - but having one 'jack of all trades' doesn't feel right to me.


For an actual review of the PR, it would also have to be Python 2 compatible to go in this plugin. More reason to put it in WLED plugin which is already locked to Py3 🙂

@cp2004
Copy link
Owner

cp2004 commented Feb 9, 2021

More reasons for it to be separate, would be that WLED plugin can be cross platform, whereas this one is locked to a Raspberry Pi, requires all the OS configuration setup, there's a fair amount that's tailored to the Pi.

A lot of this could be stripped out to make the WLED plugin a bit easier to work with.

In addition, we would have access to the wide range of effects that WLED has, which the plugin (as it does currently) would be able to use and could then be supplemented by this real-time control for certain things, like progress effects etc.

cp2004 added a commit that referenced this pull request Feb 9, 2021
Fix reported in #104

Co-Authored-By: Syfaro <syfaro@huefox.com>
@cp2004
Copy link
Owner

cp2004 commented Feb 9, 2021

Thanks for the heads up about the mistake in the work I did last night, I've since fixed that with 36243ab

@Syfaro
Copy link
Contributor Author

Syfaro commented Feb 9, 2021

Given that there could be calls every few ms to set the colour of the strip, I want to know how this would hold up.

I use WLED for holiday lights each year with UDP control, it reliably handles a few hundred LEDs being updated every ~25ms without issues.

I like what you have done, but I don't want it here, if that makes sense.

Yep, that does make sense. I can see the boundaries between the plugins and understand why they're like that.

I tested out the WLED plugin and it's much more limited right now compared to the progress bar, etc. effects currently available in this plugin. This was the fastest way for me to get those effects, and didn't result in significant code duplication. But, unfortunately, the fastest way often isn't the best or most maintainable way.

My main thought about having two similar plugins for doing effectively the same thing is that there would be identical code between them. Perhaps there could be some way to refactor animation logic into a shared library, although that's not an easy task.

Anyway, I'll close this PR. Perhaps if I get the time I'll see if I can contribute some features to the WLED plugin.

@Syfaro Syfaro closed this Feb 9, 2021
@cp2004
Copy link
Owner

cp2004 commented Feb 9, 2021

I use WLED for holiday lights each year with UDP control, it reliably handles a few hundred LEDs being updated every ~25ms without issues.

That's good to know - this is new to me, and that's a good sign that something like this could fit into the WLED plugin really well. I only recently got setup with WLED, and the more I use it the cooler it gets.

The WLED plugin is more limited, but I'm still working on it. I didn't initially know enough to do real-time control, but I think something like this could work really nicely there.

I guess now I just have to make the WLED plugin good enough to tempt you over there 😉 I look forward to the contributions in the future. If I can get the next release of this plugin done (which I am hoping to do so in a couple of weeks) then it will be full steam on the WLED plugin to get it release-ready.

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

Successfully merging this pull request may close these issues.

None yet

2 participants