vedderb / vesc_express

The source code for the VESC Express
GNU General Public License v3.0
39 stars 31 forks source link

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

Open Relys opened 6 months ago

Relys commented 6 months ago

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

vedderb commented 6 months ago

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

Relys commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 https://github.com/vedderb/vesc_express/pull/23 since my current target is an original esp32-wroom

Relys commented 5 months ago

@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 commented 5 months ago

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 commented 5 months ago

@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 commented 4 months ago

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?