Skip to content
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

ws2812 timing is incorrect when using HARDWARE_NEOPIXEL #294

Closed
jimmo opened this issue Mar 31, 2023 · 4 comments · Fixed by lancaster-university/codal-nrf52#47
Closed
Assignees
Labels
bug Something isn't working
Milestone

Comments

@jimmo
Copy link

jimmo commented Mar 31, 2023

I'm helping a school group figure out an issue related to neopixels. The background is:

  • They have purchased hundreds of robots from Seeed Studio that previously used WS2812B and have noticed that newer robots have switched to APA104. (Note: Initially I was told it was a switch to APA102, which obviously would be a problem, but these are APA104, the WS2812-"compatible" ones although the timing specification is slightly different).
  • Before the switch (i.e. older robots), they had no issues with MicroPython or make:code on either v1 or v2 micro:bits.
  • After the switch, only v2 micro:bits running MicroPython fail.

Digging into this a bit more, it appears the issue is specifically with the CODAL implementation, I think likely only when HARDWARE_NEOPIXEL is enabled (which MicroPython uses the default, enabled). It appears that this isn't generating an 800kHz waveform, and although WS2812 seems to work, the other chips such as APA104 (and maybe SK6812) don't like it.

I was surprised that make:code was fine though, but seems that they use their own driver https://github.com/microsoft/pxt-neopixel which uses https://github.com/microsoft/pxt-ws2812b/ which appears to be a common implementation shared by v1 and v2 micro:bit. Curious why they don't use CODAL also?

v1 MicroPython is also fine because it uses the hand-written assembly code in https://github.com/bbcmicrobit/micropython/blob/master/source/lib/neopixelsend.s

I captured some traces, which are attached (can be opened in PulseView). microbit-traces.zip The summary below is:

v2-micropython:
0 (H/LL): 380 / 1615   501kHz
1 (HH/L): 625 / 1370   501kHz

v2-makecode:
0 (H/LL): 340 / 780    892kHz
1 (HH/L): 810 / 590    714kHz

v1-micropython:
0 (H/LL): 250 / 1000   800kHz
1 (HH/L): 810 / 440    800kHz

v1-makecode:
0 (H/LL): 250 / 1000   800kHz
1 (HH/L): 810 / 440    800kHz

v2-micropython-bitbang:
0 (H/LL): 270 / 920    840kHz
0 (H/LL): 370 / 910    781kHz
1 (H/LL): 830 / 480    760kHz

HH/L means the relatively long H followed by the short L (i.e. a zero bit), and H/LL means the one bit. Note the last one is for CODAL with HARDWARE_NEOPIXEL disabled and appears to have a small amount of jitter for the zero bit, hence two lines for the 0.

I noticed that the hardware implementation has #define WS2812B_PWM_FREQ 500000
https://github.com/lancaster-university/codal-nrf52/blob/master/inc/WS2812B.h#L37

Changed this to 800kHz and I think the results look more correct, although I think the T1H needs to be slightly longer? It does still work on a WS2812, need to test on APA104.

v2-micropython-800kHz:
0 (H/LL): 380 / 870   800kHz
1 (H/LL): 630 / 620   800kHz

Happy to send a PR, but would be good to understand if the 500kHz is intentional? @finneyj @microbit-carlos @dpgeorge
Thanks!

@microbit-carlos
Copy link
Collaborator

Hi @jimmo,
Thanks for the detailed analysis, a PR would be very welcomed!

I don't remember the details, but I suspect we probably gathered the timings of the types of Neopixels we knew about at the time, and got it working for those values. Maybe @finneyj remembers more?

(I'll move this issue to the codal-microbit-v2 repository, codal-nrf52 was definitely the correct/logical place to open the issue, but we manage a lot of the micro:bit-related work from that issue tracker, so it's easier for us to triage it over there)

@microbit-carlos microbit-carlos transferred this issue from lancaster-university/codal-nrf52 Mar 31, 2023
@microbit-carlos microbit-carlos added this to the v0.2.52 milestone Apr 20, 2023
@microbit-carlos microbit-carlos modified the milestones: v0.2.52, v0.2.53 May 5, 2023
@jimmo
Copy link
Author

jimmo commented Jun 6, 2023

Thanks @microbit-carlos -- I have sent a PR at lancaster-university/codal-nrf52#47

This works on all the neopixel and neopixel-like strips I have access to (which includes WS2812B, SK6812 RGBW, APA104), including the previously-not-working robots mentioned in the original message.

@JohnVidler
Copy link
Collaborator

Merged this in to -nrf52 - thanks for the PR :)

@JohnVidler JohnVidler self-assigned this Jun 12, 2023
@JohnVidler JohnVidler added the bug Something isn't working label Jun 12, 2023
@microbit-carlos microbit-carlos modified the milestones: v0.2.56, v0.2.55 Jun 15, 2023
@jimmo
Copy link
Author

jimmo commented Aug 22, 2023

hi @microbit-carlos & @JohnVidler, thanks for merging this.

Damien has just updated the MicroPython repo to use CODAL 0.2.58 (microbit-foundation/micropython-microbit-v2@f47cbe1) which includes this fix. Could you please let me know a rough ETA for when the online editor at python.microbit.org will incorporate this so I can let the schools know (there are hundred of these affected robots in use across several schools and they're looking forward to the fix!). Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants