-
Notifications
You must be signed in to change notification settings - Fork 1.1k
add pico_status_led #2501
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
add pico_status_led #2501
Conversation
I think the color setting should be orthogonal to the On/Off and the color method should not mention WS2812 in their name - they should probably (and all the methods really) have a bool which says whether they do anything (so you can still use the library with no LEDs at all) I'm not quite sure about the color APIs right now - somewhat leaning towards uint32_t so you can read, and allow r,g,b or w,r,g,b methods - maybe make it a #define what WRGB LEDs do when you set r,g,b (either 0 for WRGB or auto set) |
Ah right. I was thinking you'd use the ws2812 as the blinking led. But just having a function to set the ws2812 colour might be better - as most boards seem to have both a simple led and a ws2812 led. |
There are definitely several boards with only a WS2812 and no normal LED though. EDIT: Hmmm, so given that this is a "status LED" library and not a "WS2812 library", and we're basically using the ws2812 as though it were a normal LED; perhaps it makes sense to only have the "on" colour configurable, and always have the "off colour" as blank? 🤔 Because otherwise it might not be clear that e.g. "red" is "on" and "turquoise" is "off"! 🌈 |
I'll get rid of the on off thing and just have a method that sets the RGB of a colour status led. |
IMHO being able to simply turn on/off an RGB LED would mean that a board with an RGB LED (using this status_led library) could be used identically (after a recompile of course) to a board with a normal LED? (e.g. the canonical "status-LED-blink" example) |
But most boards are configured with both. Who's to say that you shouldn't be able to control both individually? |
I'm not saying that you shouldn't be able to control both LEDs individually, I guess I was just kinda expecting a So, entirely IMVHO:
That way any example-code that's using the |
Let's keep it simple. If @kilograham disagrees we can change it |
fcada13
to
0e0dd15
Compare
This is somewhat similar to what i was expecting.
Sorry, i haven't given a lot of though yet to how i would like this to look |
looking nice - one comment in code |
looking great; a few comments in the code, but also I still think i.e. turning on/off a WS2812 LED should set it to the "currently chosen" color for on and zeros for off |
33b3c32
to
0899540
Compare
Another attempt pushed. Note that I don't have a WS2812 atm, so will test more on Monday. |
@kilograham Can you tell me if this is closer to what you want before I waste more time on it? |
The cyw43-driver enables LwIP if CYW43_LWIP is not defined. Allow LwIP to be disabled by default if CYW43_LWIP is not defined so we can use the driver to access the led without needing to link to LwIP. Add CYW43_LWIP_DEFAULT to enable this.
cbd4e4d
to
40417de
Compare
Functions for turning a binary led on and off and setting value of a colour status led. Set/get the colour used if the colored led is ON... pico_status_led_color_set_on_value pico_status_led_color_get_on_value Set the coloured led ON or OFF pico_status_led_color_set pico_status_led_color_get pico_status_led_set and pico_status_led_get call these functions if PICO_DEFAULT_LED_PIN is not defined or PICO_STATUS_LED_COLOR_ONLY is true. PICO_DEFAULT_LED_PIN_INVERTED and PICO_DEFAULT_WS2812_POWER_PIN are now used. Fixes raspberrypi#188
40417de
to
4dbc9e8
Compare
@kilograham I've tested this again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice thanks
fixes #188 |
Add a status led library to make adapting to a "normal" led and the cyw43 led a bit easier.
Bonus support for ws2812 leds.