From dd029bc59170dcf1e86dcb076b21ab28db4101c1 Mon Sep 17 00:00:00 2001 From: Raphael Coeffic Date: Tue, 14 Oct 2025 20:42:41 +1000 Subject: [PATCH] fix(radio): race condition on pulse buffer (#6634) --- radio/src/hal/module_driver.h | 4 ++++ radio/src/hal/module_port.cpp | 16 ++++++++++++++++ radio/src/hal/module_port.h | 3 +++ radio/src/hal/serial_driver.h | 3 ++- radio/src/hal/timer_driver.h | 2 ++ radio/src/pulses/afhds3.cpp | 10 ++++++++++ radio/src/pulses/afhds3_transport.h | 3 +++ radio/src/pulses/crossfire.cpp | 1 + radio/src/pulses/dsm2.cpp | 2 ++ radio/src/pulses/ghost.cpp | 1 + radio/src/pulses/multi.cpp | 1 + radio/src/pulses/ppm.cpp | 1 + radio/src/pulses/pulses.cpp | 3 +++ radio/src/pulses/pxx1.cpp | 1 + radio/src/pulses/pxx2.cpp | 2 +- radio/src/pulses/sbus.cpp | 1 + .../common/arm/stm32/stm32_pulse_driver.cpp | 7 +++++++ .../common/arm/stm32/stm32_pulse_driver.h | 1 + .../common/arm/stm32/stm32_serial_driver.cpp | 2 +- .../common/arm/stm32/stm32_softserial_driver.cpp | 9 ++++++++- radio/src/targets/simu/module_drivers.cpp | 3 ++- radio/src/targets/simu/simpgmspace.cpp | 3 ++- 22 files changed, 73 insertions(+), 6 deletions(-) diff --git a/radio/src/hal/module_driver.h b/radio/src/hal/module_driver.h index 5095c169a..d0fc30d58 100644 --- a/radio/src/hal/module_driver.h +++ b/radio/src/hal/module_driver.h @@ -22,6 +22,7 @@ #pragma once #include +#include enum ChannelsProtocols { PROTOCOL_CHANNELS_UNINITIALIZED, @@ -62,6 +63,9 @@ struct etx_proto_driver_t { // Some module settings may have been modified void (*onConfigChange)(void* ctx); + // Previous Frame already sent? + bool (*txCompleted)(void* ctx); + // Module state mode has been modified // void (*onModeChange)(void* ctx, uint8_t old_mode, uint8_t new_mode); }; diff --git a/radio/src/hal/module_port.cpp b/radio/src/hal/module_port.cpp index 81ce63cef..96f43e5de 100644 --- a/radio/src/hal/module_port.cpp +++ b/radio/src/hal/module_port.cpp @@ -297,6 +297,22 @@ uint8_t modulePortGetModule(etx_module_state_t* st) return st - _module_states; } +bool modulePortSerialTxCompleted(void* ctx) +{ + auto st = (etx_module_state_t*)ctx; + auto drv = modulePortGetSerialDrv(st->tx); + auto drv_ctx = modulePortGetCtx(st->tx); + return drv->txCompleted(drv_ctx); +} + +bool modulePortTimerTxCompleted(void* ctx) +{ + auto st = (etx_module_state_t*)ctx; + auto drv = modulePortGetTimerDrv(st->tx); + auto drv_ctx = modulePortGetCtx(st->tx); + return drv->txCompleted(drv_ctx); +} + bool modulePortIsPortUsedByModule(uint8_t module, uint8_t port) { auto mod_st = modulePortGetState(module); diff --git a/radio/src/hal/module_port.h b/radio/src/hal/module_port.h index 8a5dafe34..123a20423 100644 --- a/radio/src/hal/module_port.h +++ b/radio/src/hal/module_port.h @@ -162,6 +162,9 @@ inline void* modulePortGetCtx(const etx_module_driver_t& d) { return d.ctx; } +bool modulePortSerialTxCompleted(void* ctx); +bool modulePortTimerTxCompleted(void* ctx); + bool modulePortIsPortUsedByModule(uint8_t module, uint8_t port); bool modulePortIsPortUsed(uint8_t port); diff --git a/radio/src/hal/serial_driver.h b/radio/src/hal/serial_driver.h index c91b09b74..9ca3510c6 100644 --- a/radio/src/hal/serial_driver.h +++ b/radio/src/hal/serial_driver.h @@ -22,6 +22,7 @@ #pragma once #include +#include enum SerialEncoding { @@ -79,7 +80,7 @@ typedef struct { void (*sendBuffer)(void* ctx, const uint8_t* data, uint32_t size); // Is TX phase completed - uint8_t (*txCompleted)(void* ctx); + bool (*txCompleted)(void* ctx); // Wait for last send operation to complete void (*waitForTxCompleted)(void* ctx); diff --git a/radio/src/hal/timer_driver.h b/radio/src/hal/timer_driver.h index 98d85f7fa..4e3c0c2ef 100644 --- a/radio/src/hal/timer_driver.h +++ b/radio/src/hal/timer_driver.h @@ -22,6 +22,7 @@ #pragma once #include +#include typedef struct { @@ -33,4 +34,5 @@ typedef struct { void* (*init)(void* hw_def, const etx_timer_config_t* cfg); void (*deinit)(void* ctx); void (*send)(void* ctx, const etx_timer_config_t* cfg, const void* pulses, uint16_t length); + bool (*txCompleted)(void* ctx); } etx_timer_driver_t; diff --git a/radio/src/pulses/afhds3.cpp b/radio/src/pulses/afhds3.cpp index 6b866081f..223599dea 100644 --- a/radio/src/pulses/afhds3.cpp +++ b/radio/src/pulses/afhds3.cpp @@ -262,6 +262,8 @@ class ProtoState void applyConfigFromModel(); + bool fifoFull() { return trsp.fifoFull(); } + protected: void resetConfig(uint8_t version); @@ -1159,6 +1161,13 @@ static void sendPulses(void* ctx, uint8_t* buffer, int16_t* channels, p_state->sendFrame(); } +static bool txCompleted(void* ctx) +{ + auto mod_st = (etx_module_state_t*)ctx; + auto p_state = (ProtoState*)mod_st->user_data; + return !p_state->fifoFull(); +} + etx_proto_driver_t ProtoDriver = { .protocol = PROTOCOL_CHANNELS_AFHDS3, .init = initModule, @@ -1167,6 +1176,7 @@ etx_proto_driver_t ProtoDriver = { .processData = processTelemetryData, .processFrame = nullptr, .onConfigChange = nullptr, + .txCompleted = txCompleted, }; } // namespace afhds3 diff --git a/radio/src/pulses/afhds3_transport.h b/radio/src/pulses/afhds3_transport.h index e562f13b8..59e4fe77e 100644 --- a/radio/src/pulses/afhds3_transport.h +++ b/radio/src/pulses/afhds3_transport.h @@ -129,6 +129,7 @@ struct CommandFifo { } inline bool isEmpty() const { return (getIndex == setIndex); } + inline bool isFull() const { return nextIndex(setIndex) == getIndex; } inline void skip() { getIndex = nextIndex(getIndex); } @@ -204,6 +205,8 @@ class Transport void sendBuffer(); + bool fifoFull() { return fifo.isFull(); } + /** * Process retransmissions * error: transport is in error state diff --git a/radio/src/pulses/crossfire.cpp b/radio/src/pulses/crossfire.cpp index 0e4c30e26..18fc7f612 100644 --- a/radio/src/pulses/crossfire.cpp +++ b/radio/src/pulses/crossfire.cpp @@ -452,4 +452,5 @@ const etx_proto_driver_t CrossfireDriver = { .processData = nullptr, .processFrame = crossfireProcessFrame, .onConfigChange = nullptr, + .txCompleted = modulePortSerialTxCompleted, }; diff --git a/radio/src/pulses/dsm2.cpp b/radio/src/pulses/dsm2.cpp index 3510388dc..df6806758 100644 --- a/radio/src/pulses/dsm2.cpp +++ b/radio/src/pulses/dsm2.cpp @@ -298,6 +298,7 @@ const etx_proto_driver_t DSM2Driver = { .processData = nullptr, .processFrame = nullptr, .onConfigChange = dsm2ConfigChange, + .txCompleted = modulePortSerialTxCompleted, }; const etx_proto_driver_t DSMPDriver = { @@ -308,4 +309,5 @@ const etx_proto_driver_t DSMPDriver = { .processData = dsmpProcessData, .processFrame = nullptr, .onConfigChange = nullptr, + .txCompleted = modulePortSerialTxCompleted, }; diff --git a/radio/src/pulses/ghost.cpp b/radio/src/pulses/ghost.cpp index 9dd3fc656..9dc8997ee 100644 --- a/radio/src/pulses/ghost.cpp +++ b/radio/src/pulses/ghost.cpp @@ -255,4 +255,5 @@ const etx_proto_driver_t GhostDriver = { .processData = ghostProcessData, .processFrame = nullptr, .onConfigChange = nullptr, + .txCompleted = modulePortSerialTxCompleted, }; diff --git a/radio/src/pulses/multi.cpp b/radio/src/pulses/multi.cpp index 055750949..f5ed79b9e 100644 --- a/radio/src/pulses/multi.cpp +++ b/radio/src/pulses/multi.cpp @@ -288,6 +288,7 @@ const etx_proto_driver_t MultiDriver = { .processData = multiProcessData, .processFrame = nullptr, .onConfigChange = nullptr, + .txCompleted = modulePortSerialTxCompleted, }; static void sendChannels(uint8_t*& p_buf, uint8_t module) diff --git a/radio/src/pulses/ppm.cpp b/radio/src/pulses/ppm.cpp index c4882ffdf..6c240b47d 100644 --- a/radio/src/pulses/ppm.cpp +++ b/radio/src/pulses/ppm.cpp @@ -244,4 +244,5 @@ const etx_proto_driver_t PpmDriver = { .processData = ppmProcessTelemetryData, .processFrame = nullptr, .onConfigChange = ppmOnConfigChange, + .txCompleted = modulePortTimerTxCompleted, }; diff --git a/radio/src/pulses/pulses.cpp b/radio/src/pulses/pulses.cpp index 5106c1000..24aa1eb64 100644 --- a/radio/src/pulses/pulses.cpp +++ b/radio/src/pulses/pulses.cpp @@ -551,6 +551,9 @@ void pulsesSendNextFrame(uint8_t module) state.settings_updated = 0; } + // if previous frame not completed, skip this one + if (!drv->txCompleted(ctx)) return; + uint8_t channelStart = g_model.moduleData[module].channelsStart; int16_t* channels = &channelOutputs[channelStart]; uint8_t nChannels = 16; // TODO: MAX_CHANNELS - channelsStart diff --git a/radio/src/pulses/pxx1.cpp b/radio/src/pulses/pxx1.cpp index 86c8d7670..bea052506 100644 --- a/radio/src/pulses/pxx1.cpp +++ b/radio/src/pulses/pxx1.cpp @@ -416,4 +416,5 @@ const etx_proto_driver_t Pxx1Driver = { .processData = pxx1ProcessData, .processFrame = nullptr, .onConfigChange = nullptr, + .txCompleted = modulePortSerialTxCompleted, }; diff --git a/radio/src/pulses/pxx2.cpp b/radio/src/pulses/pxx2.cpp index cada78f0e..08d99c8ac 100644 --- a/radio/src/pulses/pxx2.cpp +++ b/radio/src/pulses/pxx2.cpp @@ -833,7 +833,6 @@ static void pxx2ProcessData(void* ctx, uint8_t data, uint8_t* buffer, uint8_t* l } #include "hal/module_driver.h" -// #include "extmodule_serial_driver.h" const etx_proto_driver_t Pxx2Driver = { .protocol = PROTOCOL_CHANNELS_PXX2, @@ -843,4 +842,5 @@ const etx_proto_driver_t Pxx2Driver = { .processData = pxx2ProcessData, .processFrame = nullptr, .onConfigChange = nullptr, + .txCompleted = modulePortSerialTxCompleted, }; diff --git a/radio/src/pulses/sbus.cpp b/radio/src/pulses/sbus.cpp index f325c4f3b..734f6eee9 100644 --- a/radio/src/pulses/sbus.cpp +++ b/radio/src/pulses/sbus.cpp @@ -217,4 +217,5 @@ const etx_proto_driver_t SBusDriver = { .processData = sbusProcessTelemetryData, .processFrame = nullptr, .onConfigChange = sbusOnConfigChange, + .txCompleted = modulePortSerialTxCompleted, }; diff --git a/radio/src/targets/common/arm/stm32/stm32_pulse_driver.cpp b/radio/src/targets/common/arm/stm32/stm32_pulse_driver.cpp index f5b4c743c..08e27381e 100644 --- a/radio/src/targets/common/arm/stm32/stm32_pulse_driver.cpp +++ b/radio/src/targets/common/arm/stm32/stm32_pulse_driver.cpp @@ -274,6 +274,13 @@ static void set_oc_mode(const stm32_pulse_timer_t* tim, uint32_t ocmode) LL_TIM_OC_SetMode(tim->TIMx, channel, ocmode); } +bool stm32_pulse_is_completed(const stm32_pulse_timer_t* tim) +{ + uint32_t channel = _get_base_channel(tim->TIM_Channel); + return !LL_TIM_IsEnabledCounter(tim->TIMx) || + LL_TIM_OC_GetMode(tim->TIMx, channel) == LL_TIM_OCMODE_FORCED_INACTIVE; +} + void stm32_pulse_wait_for_completed(const stm32_pulse_timer_t* tim) { uint32_t channel = _get_base_channel(tim->TIM_Channel); diff --git a/radio/src/targets/common/arm/stm32/stm32_pulse_driver.h b/radio/src/targets/common/arm/stm32/stm32_pulse_driver.h index 4cc32e37c..8f2732989 100644 --- a/radio/src/targets/common/arm/stm32/stm32_pulse_driver.h +++ b/radio/src/targets/common/arm/stm32/stm32_pulse_driver.h @@ -69,6 +69,7 @@ void stm32_pulse_set_cmp_val(const stm32_pulse_timer_t* tim, uint32_t cmp_val); void stm32_pulse_start(const stm32_pulse_timer_t* tim); void stm32_pulse_stop(const stm32_pulse_timer_t* tim); +bool stm32_pulse_is_completed(const stm32_pulse_timer_t* tim); void stm32_pulse_wait_for_completed(const stm32_pulse_timer_t* tim); // return true if it could be disabled without interrupting a pulse train, false otherwise diff --git a/radio/src/targets/common/arm/stm32/stm32_serial_driver.cpp b/radio/src/targets/common/arm/stm32/stm32_serial_driver.cpp index d38575340..a8ad16796 100644 --- a/radio/src/targets/common/arm/stm32/stm32_serial_driver.cpp +++ b/radio/src/targets/common/arm/stm32/stm32_serial_driver.cpp @@ -383,7 +383,7 @@ static void stm32_serial_send_buffer(void* ctx, const uint8_t* data, uint32_t si } } -static uint8_t stm32_serial_tx_completed(void* ctx) +static bool stm32_serial_tx_completed(void* ctx) { auto st = (stm32_serial_state*)ctx; if (!st) return 1; diff --git a/radio/src/targets/common/arm/stm32/stm32_softserial_driver.cpp b/radio/src/targets/common/arm/stm32/stm32_softserial_driver.cpp index 3f5f3d188..dfd02ef54 100644 --- a/radio/src/targets/common/arm/stm32/stm32_softserial_driver.cpp +++ b/radio/src/targets/common/arm/stm32/stm32_softserial_driver.cpp @@ -494,6 +494,13 @@ static void stm32_softserial_tx_send_buffer(void* ctx, const uint8_t* data, uint stm32_pulse_start_dma_req(timer, pulses, length, ocmode, cmp_val); } +static bool stm32_softserial_tx_completed(void* ctx) +{ + auto port = (const stm32_softserial_tx_port*)ctx; + auto timer = port->tim; + return stm32_pulse_is_completed(timer); +} + void stm32_softserial_set_polarity(void* ctx, uint8_t polarity) { auto port = (const stm32_softserial_tx_port*)ctx; @@ -507,7 +514,7 @@ const etx_serial_driver_t STM32SoftSerialTxDriver = { .deinit = stm32_softserial_tx_deinit, .sendByte = stm32_softserial_tx_send_byte, .sendBuffer = stm32_softserial_tx_send_buffer, - .txCompleted = nullptr, + .txCompleted = stm32_softserial_tx_completed, .waitForTxCompleted = stm32_softserial_tx_wait, .enableRx = nullptr, // TODO: combine with EXTI / Timer implementation? (S.PORT INV RX) .getByte = nullptr, diff --git a/radio/src/targets/simu/module_drivers.cpp b/radio/src/targets/simu/module_drivers.cpp index d990f1410..f75add52b 100644 --- a/radio/src/targets/simu/module_drivers.cpp +++ b/radio/src/targets/simu/module_drivers.cpp @@ -86,6 +86,7 @@ static void deinit(void* ctx) static void sendByte(void*, uint8_t) {} static void sendBuffer(void*, const uint8_t*, uint32_t) {} +static bool txCompleted(void*) { return true; } static void waitForTxCompleted(void*) {} static int getByte(void*,uint8_t*) { return -1; } @@ -94,7 +95,7 @@ const etx_serial_driver_t _fakeSerialDriver = { .deinit = deinit, .sendByte = sendByte, .sendBuffer = sendBuffer, - .txCompleted = nullptr, + .txCompleted = txCompleted, .waitForTxCompleted = waitForTxCompleted, .enableRx = nullptr, .getByte = getByte, diff --git a/radio/src/targets/simu/simpgmspace.cpp b/radio/src/targets/simu/simpgmspace.cpp index 22b49c7a7..1a1477d19 100644 --- a/radio/src/targets/simu/simpgmspace.cpp +++ b/radio/src/targets/simu/simpgmspace.cpp @@ -381,6 +381,7 @@ static void* null_drv_init(void* hw_def, const etx_serial_init* dev) { return nu static void null_drv_deinit(void* ctx) { } static void null_drv_send_byte(void* ctx, uint8_t b) { } static void null_drv_send_buffer(void* ctx, const uint8_t* b, uint32_t l) { } +static bool null_drv_tx_completed(void* ctx) { return true; } static int null_drv_get_byte(void* ctx, uint8_t* b) { return 0; } static void null_drv_set_baudrate(void* ctx, uint32_t baudrate) { } @@ -389,7 +390,7 @@ const etx_serial_driver_t null_drv = { .deinit = null_drv_deinit, .sendByte = null_drv_send_byte, .sendBuffer = null_drv_send_buffer, - .txCompleted = nullptr, + .txCompleted = null_drv_tx_completed, .waitForTxCompleted = nullptr, .enableRx = nullptr, .getByte = null_drv_get_byte,