drivers/ws281x: Add gpio_ll and timer based driver #19891
Conversation
|
Coming from #19859, I tested this out on an Adafruit ItsyBitsy nRF52840 with an 8-LED strip. It works sometimes (if I just set the colors once instead of in a loop) but overall it's pretty glitchy for me. Seems like timing issues, but I could be doing something wrong of course. To keep things as simple as possible, I used the following test program, which should just repeatedly set all the LEDs to red, but instead I get a random strobe effect: MakefileUSEMODULE += ws281x
USEMODULE += xtimer
TIMER = 2
FREQ = 16000000
# ...main.cint main(void) {
// NeoPixel strip
ws281x_t neopixel;
uint8_t neo_state[8];
ws281x_params_t neo_params = {
.buf = neo_state,
.numof = 8,
.pin = GPIO_PIN(0, 27)
};
if(ws281x_init(&neopixel, &neo_params) != 0) {
printf("failed to initialize NeoPixel\n");
return 3;
}
xtimer_ticks32_t last_wakeup = xtimer_now();
while(true) {
color_rgb_t neo_color = {255, 0, 0};
for(int i = 0; i < 8; i++)
ws281x_set(&neopixel, i, neo_color);
ws281x_write(&neopixel);
xtimer_periodic_wakeup(&last_wakeup, 100 * US_PER_MS);
}
return 0;
} |
|
Thanks for testing. The product you link is unspecific as to whether
WS2812B or SK6812 LEDs are in there; do you happen to know? (I hope it's
WS2812B, as that'd explain differences in behavior).
It'd be helpful if you could do 3 things:
* In ./drivers/ws281x/timer_gpio_ll.c, change DEBUG from 0 to 1, and
send the output. (And keep it that way for the next tests).
* In drivers/ws281x/include/ws281x_constants.h, increase the DATA_ONE_NS
in one test (say, to 750), and
* decrease DATA_ZERO_NS in another test (say, to 275). Do things work
stably here? What is the debug output?
|
True true, how we used to do it. I will just let it run in the night I suppose and save a commit drop. |
| * depending on the precise low and high times. A value of 16MHz works well. | ||
| * */ | ||
| #ifndef WS281X_TIMER_FREQ | ||
| #define WS281X_TIMER_FREQ 16000000 |
There was a problem hiding this comment.
| #define WS281X_TIMER_FREQ 16000000 | |
| #define WS281X_TIMER_FREQ MHZ(16) |
reads a bit nicer
There was a problem hiding this comment.
It seems like using the MHZ macro causes some conflicts with the ESP so I think it best to just leave it out.
timer_set has no documented restriction on this being not null, other implementations explicitly tolerate it (rpx0xx checks inside the ISR, but doing it at init time keeps the ISR slim). This is useful when using a timer just to read, without any action when it triggers (the action is taken depending on read values, eg. in a thread context).
The macro's presence is documented in `timer_init`, but was missing from this platform.
7ceabdb
to
6a0b1c5
Compare
|
Nice, thanks for the contribution and reviews! Our |
|
Thank you all for pushing this over the finish line! |
Contribution description
This adds a driver for WS281x style LEDs (tested here with SK6812 RGBW), which is based on gpio_ll and timers (with a small addition).
Review-wise, I'd ask for this to be reviewed as a whole. PR-wise (for change log etc) it may make sense to factor out the timer additions into a second PR that's then fast-tracked and only contains the first commits, but this is all better understood as a whole.
Changes are:
There is a general alternative: Using SPI, PWM or really any other suitable general peripheral. But this works (albeit blockingly), and should be somewhat portable (although I wouldn't know what to test it on), and can still serve as a reference when doing anything more fancy.
Testing procedure
make -C tests/drivers/ws281x BOARD=particle-xenon all flash term PARTICLE_MONOFIRMWARE=1or whatever your board needsIssues/PRs references
Closes: #19859
This might also be a band-aid for #19158 -- but given there is that neat PIO, that's probably the way to go.