search

adafruit / circuitpython

CircuitPython - a Python implementation for teaching coding with microcontrollers
https://circuitpython.org
MIT License
3.96k stars 1.16k forks source link

Support GRB neopixels as well as RGB as the status indicator #8469

Open deshipu opened 8 months ago

deshipu commented 8 months ago

The Waveshare ESP32-S3-Zero board (https://github.com/adafruit/circuitpython/pull/8468) uses a Neopixel with different color order than RGB as the status LED, which means that it blinks green on error, and red on success. This is confusing for the users.

It would be nice to be able to specify a variable in the mpconfigboard.h for that board to swap the colors, so the errors are red and success is green.

I think the easiest way to do that is to add an ifdef in the supervisor/shared/status_leds.c file around line 257-259 (https://github.com/adafruit/circuitpython/blob/main/supervisor/shared/status_leds.c#L257-L259) with an alternate color order.

I could submit a PR for this, but it seems to be a good first issue, so maybe someone wants to take it.

deshipu commented 8 months ago

You don't need a ESP32-S3-Zero board to test it, any board with an on-board neopixel should work, you will just verify that the colors are swapped.

Axeia commented 8 months ago

I have added support for this to my fork:

https://github.com/Axeia/circuitpython-ESP32-S3-Zero/blob/1477bc403bb3e58a64dc1ede92567b44934dfee3/supervisor/shared/status_leds.c#L261-L266

(and a #define MICROPY_HW_NEOPIXEL_ORDER_GRB (1) line to mpconfigboard.h)

Disclaimer: I am definitely out my element in C as those are the first lines I've ever written in it. Initially I tried adding: #define MICROPY_HW_NEOPIXEL_GRB (&pin_GPIO21) to mpconfigboard.h as an alternative to: #define MICROPY_HW_NEOPIXEL (&pin_GPIO21) but came to the conclusion that it would require a fair few changes to supervisor/shared/status_leds.c as it checks if MICROPY_HW_NEOPIXEL is defined in multiple places (to include the MICROPY_HW_NEOPIXEL_COUNT file, in the deinit method and a few more places) and it seemed overly complicated. I figured just a single ifdef MICROPY_HW_NEOPIXEL_ORDER_GRB was a lot cleaner.

I didn't submit this as a PR yet as its part of my ESP32-S3-Zero board support PR which currently has the wrong USB PID. I'm waiting for Waveshare to get back to me if they'll sort out a USB PID or if I should try to do so myself

tannewt commented 8 months ago

That sounds like a good approach! Thanks for the update.