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

implement nrf show() non-blocking #229

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

implement nrf show() non-blocking #229

wants to merge 3 commits into from

Conversation

hathach
Copy link
Member

@hathach hathach commented Mar 4, 2020

  • if show() is called again, it will blockingly wait for previous show() to complete first (if not yet).
  • also drop the DWT fallback implementation since it is rarely used. We didn't run into scenario where there is no PWM device.

TODO: may piggy back multipel neopixel strip (instance) into 1 hw PWM later.

hathach added 2 commits March 4, 2020 21:51
…gly wait for previous show() complete (if not yet)

also drop the DWT fallback implementation since it is rarely used.
Copy link
Contributor

@PaintYourDragon PaintYourDragon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There’s a likely issue here with the non-blocking transfer and the interaction of the 'endTime' element and the canShow() function.

endTime is normally set at the end of a blocking data transfer. It indicates the time at which the last bit was actually transmitted. show() then returns, and calling code can proceed with other duties while the 300 microsecond NeoPixel latch time elapses…on the next call to show(), the code blocks (if necessary) until the latch is complete.

As currently written with non-blocking DMA, endTime is being set right after the DMA starts, so it contains the time of the first bit out, rather than the last. This might work with early NeoPixels (which had a shorter latch time) and/or shorter runs, but I anticipate problems with long strips and/or WS2812B pixels.

Can think of a couple ways this might be fixed…

  • After DMA starts, set endTime to micros() + (total bits * 5 / 4) (allowing the 32-bit value to roll over, it will still work). This is only an approximation but probably close enough.
  • Have DMA transfer completion trigger an interrupt, which sets endTime to micros(). This is tricky in that each NeoPixel instance has its own endTime variable, but interrupts usually exist outside the class scope.

(Ignore earlier idea about padding end with zero-bytes, since these still generate a short '1' bit. I deleted that here.)

@Swap-File
Copy link

I found that under certain condition, on mbed based nRF52840 boards using Adafruit_NeoPixel and ArduinoBLE at the same time could cause the ArduinoBLE stack to lock up occasionally with the error "[Info] Unhandled event".

Specifically, I had a central device writing 15 byte payloads to a peripheral at 20hz, while also writing to a string of 20 neopixels at 50hz on the peripheral.

The lockup would occur when BLE data came in during a LED show(), but not always.

It seemed like the lockup would happen if BLE data came in while show() was configuring the PWM & DMA transfer.

Adafruit's show() function for the nRF52840 architecture reconfigures the PWM and DMA transfer every time it is called, which assuming the led strip length and type isn't updated, isn't needed.

Here's my hacked up copy of the library that only configures the PWM and DMA transfer once, on begin().

It fixed all my lockup problems, and immediately returns after being called.

https://github.com/Swap-File/neopixel_nRF52840_dma/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants