piotr/edgetx-gx12
1
0
Fork 0
Code Issues 2 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(rgbled): double-buffer WS2812 strip to eliminate races (#7304)

Browse Source 
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Raphael Coeffic
2026-04-29 20:09:07 +10:00
committed by GitHub
co-authored by Claude Opus 4.7
parent 462b0351ef
commit ac4c4d7b6b
16 changed files with 244 additions and 169 deletions
Download Patch File Download Diff File Expand all files Collapse all files
radio/src/boards/generic_stm32/CMakeLists.txt
+1
View File
@@ -32,6 +32,7 @@ set(BOARD_LIB_SRC
boards/generic_stm32/switches.cpp
boards/generic_stm32/bor_level.cpp
boards/generic_stm32/rgb_leds.cpp
boards/generic_stm32/rgb_6pos.cpp
boards/generic_stm32/battery_voltage.cpp
boards/generic_stm32/led_driver.cpp
)
radio/src/boards/generic_stm32/led_driver.cpp
+2 -6
View File
@@ -24,10 +24,6 @@
#include "stm32_gpio.h"
#include "boards/generic_stm32/rgb_leds.h"
#include "board.h"
#if defined(LED_STRIP_GPIO)
#undef UNUSED
#include "stm32_ws2812.h"
#endif
#define __weak __attribute__((weak))
@@ -70,8 +66,8 @@ __weak void ledInit()
#if defined(FUNCTION_SWITCHES_RGB_LEDS)
__weak void fsLedRGB(uint8_t index, uint32_t color)
{
ws2812_set_color(index, GET_RED(color), \
GET_GREEN(color),GET_BLUE(color));
rgbSetLedColor(index, GET_RED(color), GET_GREEN(color), GET_BLUE(color));
rgbLedColorApply();
}
uint8_t getRGBColorIndex(uint32_t color)
radio/src/boards/generic_stm32/rgb_6pos.cpp
+84
View File
@@ -0,0 +1,84 @@
/*
* Copyright (C) EdgeTx
*
* Based on code named
* opentx - https://github.com/opentx/opentx
* th9x - http://code.google.com/p/th9x
* er9x - http://code.google.com/p/er9x
* gruvin9x - http://code.google.com/p/gruvin9x
*
* License GPLv2: http://www.gnu.org/licenses/gpl-2.0.html
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*/
#include "hal.h"
#if defined(SIXPOS_SWITCH_INDEX) && !defined(SIMU)
#include "rgb_leds.h"
#include <stdint.h>
// 6POS switch position tracking.
//
// The ADC path (mixer task) feeds raw samples into getSixPosAnalogValue()
// which maintains a sticky "last pressed" position — the switch reads
// ~0 when no button is held, so a plain ADC threshold would bounce back
// to position 0. The mixer uses the returned value (0..4096) as the pot
// position for the multipos source.
//
// The LED update is NOT done here: _six_pos_state is written by the
// mixer and consumed by rgbLedOnUpdate() below, which runs in the LED
// refresh timer task. That keeps the rgb_leds.cpp back buffer
// single-writer (the LED timer task) for the 6POS range, so Lua's
// applyRGBLedColors pattern cannot race with a mixer-context write.
static uint8_t _last_adc_state = 0;
static volatile uint8_t _six_pos_state = 0;
uint16_t getSixPosAnalogValue(uint16_t adcValue)
{
uint8_t current = 0;
if (adcValue > 3800) current = 6;
else if (adcValue > 3100) current = 5;
else if (adcValue > 2300) current = 4;
else if (adcValue > 1500) current = 3;
else if (adcValue > 1000) current = 2;
else if (adcValue > 400) current = 1;
if (_last_adc_state != current) {
_last_adc_state = current;
} else if (_last_adc_state != 0) {
_six_pos_state = _last_adc_state - 1;
}
return (4096 / 5) * _six_pos_state;
}
void rgbLedOnUpdate()
{
static int8_t last_rendered = -1;
uint8_t pos = _six_pos_state;
if (pos == last_rendered) return;
last_rendered = pos;
for (uint8_t i = 0; i < 6; i++) {
if (i == pos) {
rgbSetLedColor(i, SIXPOS_LED_RED, SIXPOS_LED_GREEN, SIXPOS_LED_BLUE);
} else {
rgbSetLedColor(i, 0, 0, 0);
}
}
rgbLedColorApply();
}
#endif // SIXPOS_SWITCH_INDEX && !SIMU
radio/src/boards/generic_stm32/rgb_leds.cpp
+95 -26
View File
@@ -30,46 +30,106 @@
#include "stm32_dma.h"
#include "stm32_gpio.h"
#include "hal/gpio.h"
#include "os/task.h"
#include "os/timer.h"
#include <string.h>
// Front buffer: read by the WS2812 DMA ISR. Mutated only inside
// _flush_and_update() and only when DMA is idle.
static uint8_t _led_colors[WS2812_BYTES_PER_LED * LED_STRIP_LENGTH];
// Back buffer: written by rgbSetLedColor (menu task / pre-OS, single writer).
// Read only inside _flush_and_update() when a new frame has been published.
static uint8_t _back_colors[WS2812_BYTES_PER_LED * LED_STRIP_LENGTH];
// Seqlock-style publish using a single counter:
// - LSB set (odd) → menu task is mid-batch, back buffer not consistent
// - LSB clear (even) → frame is published; back buffer is stable
// Setters mark the seq odd before touching the back buffer; rgbLedColorApply
// rounds up to the next even value to publish the frame. The timer copies
// only when seq is even AND differs from the last consumed value.
//
// All writes to _commit_seq come from the menu task (or pre-OS). The timer
// task is the sole reader. 32-bit aligned access is atomic on Cortex-M;
// no locks needed.
static volatile uint32_t _commit_seq = 0;
static uint32_t _consumed_seq = 0;
// Compiler barrier: prevent the compiler from reordering plain memory
// accesses around a seq update.
static inline void _compiler_barrier()
{
asm volatile("" ::: "memory");
}
// Mark the start of a back-buffer write batch (set LSB so the timer skips
// while we're modifying the buffer). Cheap no-op if already mid-batch.
static inline void _begin_batch()
{
_commit_seq |= 1u;
_compiler_barrier();
}
extern const stm32_pulse_timer_t _led_timer;
static timer_handle_t _refresh_timer = TIMER_INITIALIZER;
void rgbSetLedColor(uint8_t led, uint8_t r, uint8_t g, uint8_t b)
static void _flush_and_update()
{
ws2812_set_color(led, r, g, b);
}
uint32_t rgbGetLedColor(uint8_t led)
{
return ws2812_get_color(led);
}
bool rgbGetState(uint8_t led)
{
return ws2812_get_state(led);
}
void rgbLedColorApply()
{
ws2812_update(&_led_timer);
}
void rgbLedClearAll()
{
for (uint8_t i = 0; i < LED_STRIP_LENGTH; i++) {
ws2812_set_color(i, 0, 0, 0);
if (!ws2812_is_busy(&_led_timer)) {
uint32_t seq = _commit_seq;
// Skip if mid-batch (odd) or already consumed.
if ((seq & 1u) == 0u && seq != _consumed_seq) {
memcpy(_led_colors, _back_colors, sizeof(_led_colors));
_consumed_seq = seq;
}
}
ws2812_update(&_led_timer);
}
void rgbSetLedColor(uint8_t led, uint8_t r, uint8_t g, uint8_t b)
{
_begin_batch();
ws2812_set_color_in_buf(_back_colors, led, r, g, b);
}
uint32_t rgbGetLedColor(uint8_t led)
{
return ws2812_get_color_in_buf(_back_colors, led);
}
bool rgbGetState(uint8_t led)
{
return ws2812_get_state_in_buf(_back_colors, led);
}
void rgbLedColorApply()
{
// Publish: ensure the back-buffer write is complete, then advance to
// the next even value. (seq | 1) + 1 == round up to next even.
_compiler_barrier();
_commit_seq = (_commit_seq | 1u) + 1u;
if (!scheduler_is_running()) {
// Pre-OS: timer task isn't running, drive the flush synchronously.
_flush_and_update();
}
}
void rgbLedClearAll()
{
_begin_batch();
memset(_back_colors, 0, sizeof(_back_colors));
rgbLedColorApply();
}
__attribute__((weak)) void rgbLedOnUpdate() {}
static void _refresh_cb(timer_handle_t* timer)
{
(void)timer;
ws2812_update(&_led_timer);
rgbLedOnUpdate();
_flush_and_update();
}
static void rgbLedStart()
@@ -103,10 +163,19 @@ const stm32_pulse_timer_t _led_timer = {
.DMA_TC_CallbackPtr = nullptr,
};
static bool _hw_initialised = false;
void rgbLedHwInit()
{
if (_hw_initialised) return;
ws2812_init(&_led_timer, _led_colors, LED_STRIP_LENGTH, WS2812_GRB);
_hw_initialised = true;
rgbLedClearAll();
}
void rgbLedInit()
{
ws2812_init(&_led_timer, _led_colors, LED_STRIP_LENGTH, WS2812_GRB);
rgbLedClearAll();
rgbLedHwInit();
rgbLedStart();
}
radio/src/boards/generic_stm32/rgb_leds.h
+10
View File
@@ -24,6 +24,10 @@
#include <stdint.h>
void rgbLedInit();
// Initialise the WS2812 hardware without starting the periodic refresh
// timer. Useful for pre-OS contexts (e.g. charging UI) that need the LED
// strip up before the FreeRTOS scheduler is running.
void rgbLedHwInit();
void rgbLedStop();
void rgbSetLedColor(uint8_t led, uint8_t r, uint8_t g, uint8_t b);
@@ -33,3 +37,9 @@ void rgbLedClearAll();
bool rgbGetState(uint8_t led);
void rgbLedColorApply();
// Weak hook invoked by the LED refresh timer task before publishing the
// next frame. Boards that drive system LEDs (e.g. the 6POS position
// indicator) implement this to update their LEDs in the timer task
// context, keeping rgb_leds.cpp's back buffer single-writer.
void rgbLedOnUpdate();
radio/src/boards/jumper-h750/board.cpp
+2 -10
View File
@@ -23,7 +23,6 @@
#include "stm32_gpio.h"
#include "stm32_i2c_driver.h"
#include "stm32_hal.h"
#include "stm32_ws2812.h"
#include "stm32_spi.h"
#include "flash_driver.h"
@@ -59,17 +58,10 @@
// common ADC driver
extern const etx_hal_adc_driver_t _adc_driver;
// RGB LED timer
extern const stm32_pulse_timer_t _led_timer;
static void led_strip_off()
{
for (uint8_t i = 0; i < LED_STRIP_LENGTH; i++) {
ws2812_set_color(i, 0, 0, 0);
}
ws2812_update(&_led_timer);
}
rgbLedClearAll();
}
void INTERNAL_MODULE_ON()
{
radio/src/boards/rm-h750/board.cpp
+1 -8
View File
@@ -23,7 +23,6 @@
#include "stm32_gpio.h"
#include "stm32_i2c_driver.h"
#include "stm32_hal.h"
#include "stm32_ws2812.h"
#include "stm32_spi.h"
#include "flash_driver.h"
@@ -69,9 +68,6 @@
// common ADC driver
extern const etx_hal_adc_driver_t _adc_driver;
// RGB LED timer
extern const stm32_pulse_timer_t _led_timer;
static const etx_imu_t _imu_candidates[] = {
#if defined(IMU_ICM4207C)
{ &imu_icm42607_driver, IMU_I2C_BUS, ICM426xx_I2C_BASE_ADDR },
@@ -90,10 +86,7 @@ static void gyroInit()
static void led_strip_off()
{
for (uint8_t i = 0; i < LED_STRIP_LENGTH; i++) {
ws2812_set_color(i, 0, 0, 0);
}
ws2812_update(&_led_timer);
rgbLedClearAll();
}
void INTERNAL_MODULE_ON()
radio/src/targets/common/arm/stm32/stm32_ws2812.cpp
+13 -7
View File
@@ -221,32 +221,38 @@ void ws2812_init(const stm32_pulse_timer_t* timer, uint8_t* strip_colors,
_init_timer(timer);
}
void ws2812_set_color(uint8_t led, uint8_t r, uint8_t g, uint8_t b)
void ws2812_set_color_in_buf(uint8_t* buf, uint8_t led,
uint8_t r, uint8_t g, uint8_t b)
{
if (led >= _led_strip_len) return;
uint8_t* pixel = &_led_colors[led * WS2812_BYTES_PER_LED];
uint8_t* pixel = &buf[led * WS2812_BYTES_PER_LED];
pixel[_r_offset] = r;
pixel[_g_offset] = g;
pixel[_b_offset] = b;
}
uint32_t ws2812_get_color(uint8_t led)
uint32_t ws2812_get_color_in_buf(const uint8_t* buf, uint8_t led)
{
if (led >= _led_strip_len) return 0;
uint8_t* pixel = &_led_colors[led * WS2812_BYTES_PER_LED];
return (pixel[1] << 16) + (pixel[0] << 8) + pixel[2];
const uint8_t* pixel = &buf[led * WS2812_BYTES_PER_LED];
return (pixel[1] << 16) + (pixel[0] << 8) + pixel[2];
}
bool ws2812_get_state(uint8_t led)
bool ws2812_get_state_in_buf(const uint8_t* buf, uint8_t led)
{
if (led >= _led_strip_len) return false;
uint8_t* pixel = &_led_colors[led * WS2812_BYTES_PER_LED];
const uint8_t* pixel = &buf[led * WS2812_BYTES_PER_LED];
return pixel[0] || pixel[1] || pixel[2];
}
bool ws2812_is_busy(const stm32_pulse_timer_t* tim)
{
return LL_DMA_IsEnabledStream(tim->DMAx, tim->DMA_Stream);
}
void ws2812_update(const stm32_pulse_timer_t* tim)
{
WS2812_DBG_HIGH;
radio/src/targets/common/arm/stm32/stm32_ws2812.h
+10 -3
View File
@@ -38,8 +38,15 @@
void ws2812_init(const stm32_pulse_timer_t* timer, uint8_t* strip_colors,
uint8_t strip_len, uint8_t type);
void ws2812_update(const stm32_pulse_timer_t* timer);
bool ws2812_get_state(uint8_t led);
void ws2812_dma_isr(const stm32_pulse_timer_t* timer);
void ws2812_set_color(uint8_t led, uint8_t r, uint8_t g, uint8_t b);
uint32_t ws2812_get_color(uint8_t led);
// Returns true if a DMA transfer is currently in progress.
bool ws2812_is_busy(const stm32_pulse_timer_t* timer);
// Buffer-parameterised setter/getter. The caller owns the buffer; ws2812
// applies the strip's RGB byte ordering through the offsets configured at
// init time.
void ws2812_set_color_in_buf(uint8_t* buf, uint8_t led,
uint8_t r, uint8_t g, uint8_t b);
uint32_t ws2812_get_color_in_buf(const uint8_t* buf, uint8_t led);
bool ws2812_get_state_in_buf(const uint8_t* buf, uint8_t led);
radio/src/targets/horus/board.cpp
-40
View File
@@ -23,7 +23,6 @@
#include "stm32_hal_ll.h"
#include "stm32_gpio.h"
#include "stm32_spi.h"
#include "stm32_ws2812.h"
#include "vs1053b.h"
#include "hal/adc_driver.h"
@@ -115,45 +114,6 @@ void audioInit()
}
#endif
#if defined(SIXPOS_SWITCH_INDEX)
uint8_t lastADCState = 0;
uint8_t sixPosState = 0;
bool dirty = true;
uint16_t getSixPosAnalogValue(uint16_t adcValue)
{
uint8_t currentADCState = 0;
if (adcValue > 3800)
currentADCState = 6;
else if (adcValue > 3100)
currentADCState = 5;
else if (adcValue > 2300)
currentADCState = 4;
else if (adcValue > 1500)
currentADCState = 3;
else if (adcValue > 1000)
currentADCState = 2;
else if (adcValue > 400)
currentADCState = 1;
if (lastADCState != currentADCState) {
lastADCState = currentADCState;
} else if (lastADCState != 0 && lastADCState - 1 != sixPosState) {
sixPosState = lastADCState - 1;
dirty = true;
}
if (dirty) {
for (uint8_t i = 0; i < 6; i++) {
if (i == sixPosState) {
ws2812_set_color(i, SIXPOS_LED_RED, SIXPOS_LED_GREEN, SIXPOS_LED_BLUE);
} else {
ws2812_set_color(i, 0, 0, 0);
}
}
rgbLedColorApply();
}
return (4096/5)*(sixPosState);
}
#endif
#if defined(HAS_IMU)
#include "drivers/lsm6ds.h"
#include "stm32_i2c_driver.h"
radio/src/targets/horus/board.h
+4 -4
View File
@@ -185,10 +185,6 @@ uint32_t pwrPressedDuration();
void usbChargerInit();
bool usbChargerLed();
#if defined(RADIO_V16)
uint16_t getSixPosAnalogValue(uint16_t adcValue);
#endif
// Led driver
void ledInit();
void ledOff();
@@ -246,6 +242,10 @@ bool isBacklightEnabled();
}
#endif
#if defined(RADIO_V16)
uint16_t getSixPosAnalogValue(uint16_t adcValue);
#endif
// Audio driver
void audioInit();
radio/src/targets/pa01/battery_driver.cpp
+5 -7
View File
@@ -27,7 +27,6 @@
#include "edgetx.h"
#include "mainwindow.h"
#include "static.h"
#include "stm32_ws2812.h"
#include "LvglWrapper.h"
#define __BATTERY_DRIVER_C__
@@ -603,8 +602,8 @@ void setLedGroupColor(uint8_t index, uint8_t color, uint8_t brightness) {
scaled_b = brightness;
}
ws2812_set_color(rgbMapping[index], scaled_r, scaled_g, scaled_b);
ws2812_set_color(rgbMapping[index] + 1, scaled_r, scaled_g, scaled_b);
rgbSetLedColor(rgbMapping[index], scaled_r, scaled_g, scaled_b);
rgbSetLedColor(rgbMapping[index] + 1, scaled_r, scaled_g, scaled_b);
}
uint8_t ledBreathBright(float angle, uint8_t maxBright) {
@@ -655,11 +654,10 @@ void ledLoop(void) {
ledBreathUpdate(led_info.led_state, led_info.led_color, led_info.led_group);
}
extern const stm32_pulse_timer_t _led_timer;
static uint8_t _led_charge_colors[WS2812_BYTES_PER_LED * LED_STRIP_LENGTH];
void rgbChargeInit(void) {
ws2812_init(&_led_timer, _led_charge_colors, LED_STRIP_LENGTH, WS2812_GRB);
rgbLedClearAll();
// Bring up the WS2812 strip without starting the periodic refresh timer
// (the FreeRTOS scheduler isn't running yet during the charging UI).
rgbLedHwInit();
}
constexpr uint16_t vbatLedTable[] = {650, 720, 760, 800, 823 };
radio/src/targets/pa01/led_driver.cpp
+15 -11
View File
@@ -22,7 +22,6 @@
#include "hal/gpio.h"
#include "hal/rgbleds.h"
#include "stm32_gpio.h"
#include "stm32_ws2812.h"
#include "boards/generic_stm32/rgb_leds.h"
#include "board.h"
@@ -36,32 +35,37 @@ uint8_t ledMapping[] = {4, 6, 0, 2};
void fsLedRGB(uint8_t index, uint32_t color)
{
ws2812_set_color(ledMapping[index], GET_RED(color),
rgbSetLedColor(ledMapping[index], GET_RED(color),
GET_GREEN(color),GET_BLUE(color));
ws2812_set_color(ledMapping[index]+1, GET_RED(color),
rgbSetLedColor(ledMapping[index]+1, GET_RED(color),
GET_GREEN(color),GET_BLUE(color));
rgbLedColorApply();
}
void ledOff()
{
ws2812_set_color(8, 0, 0, 0);
ws2812_set_color(9, 0, 0, 0);
rgbSetLedColor(8, 0, 0, 0);
rgbSetLedColor(9, 0, 0, 0);
rgbLedColorApply();
}
void ledRed()
{
ws2812_set_color(8, 20, 0, 0);
ws2812_set_color(9, 20, 0, 0);
rgbSetLedColor(8, 20, 0, 0);
rgbSetLedColor(9, 20, 0, 0);
rgbLedColorApply();
}
void ledGreen()
{
ws2812_set_color(8, 0, 20, 0);
ws2812_set_color(9, 0, 20, 0);
rgbSetLedColor(8, 0, 20, 0);
rgbSetLedColor(9, 0, 20, 0);
rgbLedColorApply();
}
void ledBlue()
{
ws2812_set_color(8, 0, 0, 20);
ws2812_set_color(9, 0, 0, 20);
rgbSetLedColor(8, 0, 0, 20);
rgbSetLedColor(9, 0, 0, 20);
rgbLedColorApply();
}
radio/src/targets/pl18/board.cpp
+1 -8
View File
@@ -21,7 +21,6 @@
#include "stm32_adc.h"
#include "stm32_gpio.h"
#include "stm32_ws2812.h"
#include "stm32_spi.h"
#include "hal/adc_driver.h"
@@ -139,15 +138,9 @@ void delay_self(int count)
}
#if defined(LED_STRIP_GPIO)
// Common LED driver
extern const stm32_pulse_timer_t _led_timer;
void ledStripOff()
{
for (uint8_t i = 0; i < LED_STRIP_LENGTH; i++) {
ws2812_set_color(i, 0, 0, 0);
}
ws2812_update(&_led_timer);
rgbLedClearAll();
}
#endif
radio/src/targets/st16/led_driver.cpp
+1
View File
@@ -35,6 +35,7 @@ void fsLedRGB(uint8_t index, uint32_t color)
index = (index * 2) + CFS_LED_STRIP_START;
rgbSetLedColor(index, GET_RED(color), GET_GREEN(color),GET_BLUE(color));
rgbSetLedColor(index+1, GET_RED(color), GET_GREEN(color),GET_BLUE(color));
rgbLedColorApply();
}
uint32_t fsGetLedRGB(uint8_t index)
radio/src/targets/taranis/board.cpp
-39
View File
@@ -21,7 +21,6 @@
#include "stm32_hal_ll.h"
#include "stm32_gpio.h"
#include "stm32_ws2812.h"
#include "hal/adc_driver.h"
#include "hal/trainer_driver.h"
@@ -72,44 +71,6 @@ HardwareOptions hardwareOptions;
#include "storage/storage.h"
#endif
#if defined(SIXPOS_SWITCH_INDEX)
uint8_t lastADCState = 0;
uint8_t sixPosState = 0;
bool dirty = true;
uint16_t getSixPosAnalogValue(uint16_t adcValue)
{
uint8_t currentADCState = 0;
if (adcValue > 3800)
currentADCState = 6;
else if (adcValue > 3100)
currentADCState = 5;
else if (adcValue > 2300)
currentADCState = 4;
else if (adcValue > 1500)
currentADCState = 3;
else if (adcValue > 1000)
currentADCState = 2;
else if (adcValue > 400)
currentADCState = 1;
if (lastADCState != currentADCState) {
lastADCState = currentADCState;
} else if (lastADCState != 0 && lastADCState - 1 != sixPosState) {
sixPosState = lastADCState - 1;
dirty = true;
}
if (dirty) {
for (uint8_t i = 0; i < 6; i++) {
if (i == sixPosState)
ws2812_set_color(i, SIXPOS_LED_RED, SIXPOS_LED_GREEN, SIXPOS_LED_BLUE);
else
ws2812_set_color(i, 0, 0, 0);
}
rgbLedColorApply();
}
return (4096/5)*(sixPosState);
}
#endif
#if defined(SALED_PWR_GPIO)
void SALEDpwrInit()
{