refactor(driver): move 6POS tracker into board-level ADC callback#7309
Open
raphaelcoeffic wants to merge 3 commits intomainfrom
Open
refactor(driver): move 6POS tracker into board-level ADC callback#7309raphaelcoeffic wants to merge 3 commits intomainfrom
raphaelcoeffic wants to merge 3 commits intomainfrom
Conversation
The WS2812 LED strip was driven from two contexts without any serialisation: the FreeRTOS refresh timer (50ms) and Lua's applyRGBLedColors. Both called ws2812_update directly, racing on the DMA arming sequence, and the DMA ISR read the same _led_colors buffer that callers were mid-writing — producing torn pixels and partial-frame flicker. Switch to a lock-free single-driver design: - Board (rgb_leds.cpp) now owns a back buffer (writes) and a front buffer (DMA). Setters touch only the back buffer; the timer task is the sole arm point for ws2812_update at runtime. - A seqlock-style counter (_commit_seq) publishes batches: setters mark the LSB to flag mid-batch; rgbLedColorApply rounds up to the next even value to publish; the timer copies back→front only when the seq is even, differs from the last consumed value, and DMA is idle. Compiler barriers prevent reordering of plain back-buffer writes around the volatile seq updates. - Pre-OS callers (charging UI) bypass the timer: rgbLedColorApply detects scheduler_is_running() == false and flushes synchronously. To make the board the sole entry point, the no-buf ws2812_set_color / get_color / get_state are replaced by buffer-parameterised variants; all direct callers in horus/taranis/pl18/pa01/rm-h750/jumper-h750 boards and the generic_stm32 fsLedRGB now go through rgbSetLedColor. fsLedRGB and pa01's status-LED helpers gain explicit rgbLedColorApply calls to preserve their fire-and-forget semantics. pa01's separate _led_charge_colors buffer and ad-hoc ws2812_init are removed; rgbLedHwInit is exposed so the charging UI can bring up the WS2812 hardware before the scheduler is running while still sharing the rgb_leds.cpp front buffer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
getSixPosAnalogValue() ran in the mixer task and wrote directly to the WS2812 back buffer. On V12/V14/V16 that made the mixer a second writer racing with Lua's setRGBLedColor / applyRGBLedColors batches — the double-buffer seqlock only serialises publication, not concurrent back-buffer writes. Split the function: keep the sticky position tracker in the ADC hot path (a single volatile uint8_t write, read by other tasks as an atomic byte), and move the LED paint into a new weak rgbLedOnUpdate() hook called by the rgb_leds.cpp refresh timer. The 6POS LEDs (indices 0..5) now have the LED timer task as their sole writer, while Lua addresses map through BLING_LED_STRIP_START=6 onto the remaining range — so the two writable ranges are disjoint and cannot race. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 6POS sticky-position state machine lived in hal/adc_driver.cpp:getAnalogValue() behind a SIXPOS_SWITCH_INDEX special case. Every reader (cli, diaganas, switches, inactivity_timer, …) re-triggered the tracker, which is wasteful and ran state updates from contexts that were never meant to drive them. Move the tracker into sixPosUpdateFromAdc(), called once per conversion from the board adc_wait_completion callback in analog_inputs.cpp. This matches the pattern used by the ADS79xx SPI ADC integration and keeps all ADC post-processing in one well-defined layer. getAnalogValue is now a plain array read with no special cases, and the 6POS entry point is no longer a board-level API. rgbLedOnUpdate() still reads _six_pos_state in the LED timer task, unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
After I fixed it this way, I no longer encountered the issue of random key presses during my testing. Let's see if this method works.
|
pfeerick
added
the
needs: rebase
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #7304. The 6POS sticky-position tracker lived in
hal/adc_driver.cpp:getAnalogValue()behind aSIXPOS_SWITCH_INDEXspecial case. Every reader (cli,diaganas,switches,inactivity_timer, …) re-triggered the state machine, which is wasteful and ran state updates from contexts that were never meant to drive them.This moves the tracker into
sixPosUpdateFromAdc(), invoked once per conversion from the boardadc_wait_completioncallback inboards/generic_stm32/analog_inputs.cpp. The pattern matches the existing ADS79xx SPI ADC integration, keeping all ADC post-processing in one well-defined layer.getAnalogValue()becomes a plain array read — noSIXPOS_SWITCH_INDEXbranch.getSixPosAnalogValueis no longer a board-level API; it's internal torgb_6pos.cpp.rgbLedOnUpdate()still reads_six_pos_statefrom the LED timer task, unchanged.Stacked on #7304 — merge that first. Targets base
ws2812-double-bufferinghere; retarget tomainonce #7304 lands.Test plan
make firmwarebuilds on V16 (RADIO_V16, Horus-family 6POS)make firmwarebuilds on V14 (RADIO_V14, Taranis-family 6POS)make firmwarebuilds on tx16s (non-6POS sanity check)