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

Added support to use multiple pins for RGB LEDs. #22

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Relys
Copy link
Contributor

@Relys Relys commented Mar 14, 2024

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).

@Relys Relys force-pushed the feature-rgbled-multiple-pin branch from cb99f82 to 26aabd2 Compare March 14, 2024 22:59
@vedderb
Copy link
Owner

vedderb commented Mar 17, 2024

Can you change your editor settings so that it does not replace tabs with spaces?

@Relys
Copy link
Contributor Author

Relys commented Mar 18, 2024

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
optStripType=0 for WS2812
optStripType=1 for SK6812

What should I do for gpio_is_valid? Should I add that in hw_conf for list of valid pins?

@vedderb
Copy link
Owner

vedderb commented Mar 18, 2024

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.

@vedderb
Copy link
Owner

vedderb commented Mar 18, 2024

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.

@Relys
Copy link
Contributor Author

Relys commented Mar 21, 2024

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.

@vedderb
Copy link
Owner

vedderb commented Mar 21, 2024

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.

@Relys
Copy link
Contributor Author

Relys commented Mar 22, 2024

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

@Relys
Copy link
Contributor Author

Relys commented Mar 24, 2024

@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.

@vedderb
Copy link
Owner

vedderb commented Mar 25, 2024

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.

@Relys
Copy link
Contributor Author

Relys commented Apr 18, 2024

@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.

@vedderb
Copy link
Owner

vedderb commented Apr 19, 2024

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?

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.

2 participants