-
Notifications
You must be signed in to change notification settings - Fork 35
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
Added support to use multiple pins for RGB LEDs. #22
base: main
Are you sure you want to change the base?
Conversation
cb99f82
to
26aabd2
Compare
Can you change your editor settings so that it does not replace tabs with spaces? |
Sure changed it. Also added a new optional parameter to rgbledinit optStripType for different LED strip timings. // (rgbled-init pin num-leds optLedType optStripType) -> t, nil What should I do for gpio_is_valid? Should I add that in hw_conf for list of valid pins? |
Regarding the pin, on the ESP32-C3 gpio17 goes to the internal flash-chip, so it should not be attached to the LED driver by accident. At the moment there is not really support for other SOCs, but if you are using something else and want to prepare for it you can make an ifdef in the gpio_is_valid-function that checks the SOC. Also, I'm generally not so happy with this approach as it adds a lot of complexity for going from supporting one led-strip to two led-strips. It is possible to support any number of strips by separating out the pixels and keeping track of them manually. Then you have one pixel-array per strip, update it as desired and send it out on any pin when done. I can look into that in the next days. |
I spent a few hours today rewriting the rgbled-driver with what I had in mind. Here is an example how to use multiple led-strips now: https://github.com/vedderb/bldc/blob/master/lispBM/README.md#multiple-led-strips This change unfortunately breaks compatibility with the old driver, but I think it was worth doing as it still is the same beta and the old driver is broken in 6.02. Can you see if this works for what you had in mind? We can still add support for different timings if that is needed. |
The performance of having to call rgbled-init to switch between pins appears to have a pretty big overhead. I'm seeing CPU spikes of ~20% difference just adding a second init (with no update). Also, should optLedType be in rgbled-update instead of rgbled-buffer? That way you can preform operations in one rgbled-buffer for all strips and split them with bufcpy at the end if they are on different pins. |
Regarding performance, did you switch to the latest version? Disabling logging in sdkconf made a big difference as a lot of stuff was logged when initializing the driver. I measured that initializing the driver takes about 0.3 mS after disabling logging (and more than 10x longer before that). If the performance of init is a problem we can also make a much lighter version that just changes the pin, although that code would be a bit of a hack. The problem with having ledType in update is that the color order is determined when using rgbled-color. Then you would have to copy the buffer and re-order the colors before sending it out. |
Ok cool I'll try disabling logging in sdkconf. I bet that's what's spiking. I figured out how to handle ledType. I'm just keeping track of three separate buffers and then combining them when I update. Everything is working good! :D We still need to handle gpio_is_valid for different targets but that kind of relies on #23 since my current target is an original esp32-wroom |
@vedderb Would it be possible to add the optBrightness parameter to rgbled-color? It would be much less overhead and less messy to do it there instead of doing bitwise operations in lisp. |
We can add that option. It might also be good to add an option for gamma correction, or just enable that by default as it looks so much better. It might also be nice to add some other functions to operate on color buffers such as rotate, set brightness on all etc. |
@vedderb Update: Main thing I've noticed is that I have to set a sleep of 0.01-0.02 seconds after rgb-update before the next rgb-init when I switch pins or else sometimes I can get the wrong pin updated. :/ Would using separate RMT drivers per pin fix this? I had already written code to manage that. |
I think I tried exactly that and made sure that it waits for the previous transmission to finish before re-initializing the pin. Do you have an example with two LED-strips that demonstrates the problem? |
Support for using multiple GPIO pins to drive LEDs. It will dynamically determine how many RMT channels the ESP32 hardware has and manage them mapping them to GPIO pins user inits. Fully backwards compatible with two optional pin parameters.
// (rgbled-init pin num-leds optLedType optStripType) -> t, nil
optStripType=0 for WS2812
optStripType=1 for SK6812
// (rgbled-deinit optPIN) -> t, nil
// (rgbled-color led-num color optBrightness optPIN) -> t, nil
My only question is what do we do regarding gpio_is_valid? This should probably be defined in hwconf as I had to add pin 17 for my hardware configuration (not included in this PR).