adafruit / Adafruit_CircuitPython_ESP32SPI

ESP32 as wifi with SPI interface
MIT License
101 stars 74 forks source link

Expose RGB LEDs in WiFiManager, instead of NeoPixels #47

Closed brentru closed 5 years ago

brentru commented 5 years ago

Some of the AirLift breakouts use RGB LEDS instead of NeoPixels. It'd be nice to have an status indicator LED within the WiFiManager for cases when NeoPixels aren't present.

The AirLift Breakout follows the pin ordering: 25 (Red), 26 (Green), 27 (Blue).

makermelissa commented 5 years ago

Do you know if we have a library to set the color of RGB leds? I had abstracted it so either a DotStar or Neopixel could be passed in. Probably not, but I’m just hoping it will be that easy to implement. :)

brentru commented 5 years ago

@makermelissa Yup! The LED pins can be defined in ESP32SPI (https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI/blob/master/adafruit_esp32spi/adafruit_esp32spi.py#L629)

and written to as outputs, in set_digital_write (https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI/blob/master/adafruit_esp32spi/adafruit_esp32spi.py#L647).

You can also PWM the RGB LEDs: https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI/blob/master/adafruit_esp32spi/adafruit_esp32spi.py#L659

Since it's the same across the airlift hardware, one could add a toggle for a RGB status light. There'll need to be a helper-RGB-LED method within WiFiManager too, to do some color-mixin'.

Here's a really basic example of setting and un-setting each individual RGB LED which works with all AirLift hardware with the RGB LEDs.

spi = busio.SPI(board.SCK, board.MOSI, board.MISO)
esp = adafruit_esp32spi.ESP_SPIcontrol(spi, esp32_cs, esp32_ready, esp32_reset)
import time
pins = [25, 26, 27]

for i in pins:
    esp.set_pin_mode(i, 1)
    esp.set_digital_write(i, 1)
    time.sleep(1)
    esp.set_digital_write(i, 0)
makermelissa commented 5 years ago

It might be good to create another class in this library for RGB leds that handles the color mixing that can be passed in like the DotStar or Neopixel.

brentru commented 5 years ago

Agreed!

anecdata commented 5 years ago

You folks probably know more than I do about upcoming products (e.g., Airlift FeatherWing), and I may easily be overcomplicating or missing a subtlety of the proposal. But right now, only the Airlift Breakout has separate R, G, and B LEDs. ESP32 Huzzah Breakout and ESP32 Feather do not. These are all valid NINA targets for ESP32SPI. ItsyBitsy, Feather M4, and Metro Airlift have either NeoPixel or Dotstar on-board (plus Red LED). Most recent Airlift FeatherWing #LEEK had only one LED that I could discern, but ladyada may well have embellished it since then.

Do we want to consider not combining the function of on-board MCU indicators with remote co-processor indicators, or at least make it optional (e.g., Keyword Argument)? Users could easily want to use the co-processor LEDs for different purposes than the main board indicators. As noted, Wi-Fi Manager uses main board NeoPixel or Dotstar. Plus, pin configs, same-board vs. separate board Airlift configs, and LED color availability vary quite a bit.

anecdata commented 5 years ago

Not to muddy the waters, but currently in a main board + separate co-processor board config, there is no way (that I know of) for the main board to know which co-processor form factor is in use (e.g., Airlift Breakout, ESP32 Feather, Airlift FeatherWing, ESP32 Huzzah).

The GPIO config does vary across co-processor boards currently, so in theory it's possible to feed that back to the main board.

anecdata commented 5 years ago

This is maybe much ado about nothing. I suppose one could just set status_pixel to None (or to the local Neo/Dot) if they didn't want to use the Airlift RGB LEDs with a non-Airlift separate main board.

P.S. Just tested to confirm color-pin mapping is G=25, R=26, B=27.

brentru commented 5 years ago

@anecdata All good points - I didn't consider a user wanting to use the status indicator RGBs for a different purpose other than showing the breakout's status. I agree on a user-defined status_pixel=None kwarg to indicate if they want to use, possibly status_pixel=rgb_leds to indicate the leds

anecdata commented 5 years ago

Sounds perfect. Sorry for all my rambling. It took me awhile to think (out loud) through all the configs and come back around to "this is a good concept and makes sense". Keeps all options open for users.

makermelissa commented 5 years ago

Another other option, if it doesn’t already exist, is to have a separate library for controlling RGB LEDs. It might be useful for hooking up an external RGB LED or any non-Adafruit boards that might have such an LED on it.

brentru commented 5 years ago

@makermelissa I did some work on this earlier today, I don't think much needs to be changed since you're already passing a tuple into the pixel fill method within _wifimanager

https://github.com/brentru/Adafruit_CircuitPython_ESP32SPI/commit/57cfce1844f2410ce3c136df3ce714d8b6271110

specifically here: https://github.com/brentru/Adafruit_CircuitPython_ESP32SPI/commit/57cfce1844f2410ce3c136df3ce714d8b6271110#diff-0e68a11e929848eefb03dcfc4cbf3497R232

makermelissa commented 5 years ago

I agree that it won’t take much. Also, it should allow for a 24-bit integer that can be easily be bit-shifted to get the RGB values.

brentru commented 5 years ago

@makermelissa why would you want to opt for that over using tuples, similarly to how the neopixels are used?

makermelissa commented 5 years ago

Not necessarily opt, but at least provide another option for users since DotStar and Neopixel can already handle both. Either way is fine really.

ladyada commented 5 years ago

yeah i agree with @makermelissa - having an adafruit_circuitpython_rgbled class would help with pulseio interfacing too, and inverted logic. iz a good idea! then this can mimic it

brentru commented 5 years ago

Ok, we'll hold off on this for now until we have RGBLED.

RGBLED Class work started in https://github.com/adafruit/CircuitPython_RGBLED, work being done on my branch https://github.com/brentru/CircuitPython_RGBLED

brentru commented 5 years ago

This has been addressed by https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI/pull/53 and released in latest (1.4.2).