From beb4cd62b209a1d33b81eeb092243fe3f9ebedbf Mon Sep 17 00:00:00 2001 From: Andy Piper Date: Wed, 26 Mar 2025 17:35:19 +0000 Subject: [PATCH] AP_HAL_ChibiOS: fix dshot cancel race with waiter --- libraries/AP_HAL_ChibiOS/RCOutput.cpp | 7 ++++--- libraries/AP_HAL_ChibiOS/RCOutput_bdshot.cpp | 9 +++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/libraries/AP_HAL_ChibiOS/RCOutput.cpp b/libraries/AP_HAL_ChibiOS/RCOutput.cpp index 00bd4f2339f..9da57b3d220 100644 --- a/libraries/AP_HAL_ChibiOS/RCOutput.cpp +++ b/libraries/AP_HAL_ChibiOS/RCOutput.cpp @@ -347,7 +347,7 @@ void RCOutput::dshot_collect_dma_locks(rcout_timer_t cycle_start_us, rcout_timer if (!mask) { dma_cancel(group); } - group.dshot_waiter = nullptr; + osalDbgAssert(group.dshot_waiter == nullptr, "Dshot waiter was not reset"); #ifdef HAL_WITH_BIDIR_DSHOT // if using input capture DMA then clean up if (group.bdshot.enabled) { @@ -1847,12 +1847,12 @@ __RAMFUNC__ void RCOutput::dma_unlock(virtual_timer_t* vt, void *p) { chSysLockFromISR(); pwm_group *group = (pwm_group *)p; - group->dshot_state = DshotState::IDLE; if (group->dshot_waiter != nullptr) { // tell the waiting process we've done the DMA. Note that - // dshot_waiter can be null if we have cancelled the send + // dshot_waiter can be null if we have just cancelled the send chEvtSignalI(group->dshot_waiter, group->dshot_event_mask); + group->dshot_waiter = nullptr; } chSysUnlockFromISR(); } @@ -1911,6 +1911,7 @@ void RCOutput::dma_cancel(pwm_group& group) chEvtGetAndClearEventsI(group.dshot_event_mask | DSHOT_CASCADE); group.dshot_state = DshotState::IDLE; + group.dshot_waiter = nullptr; chSysUnlock(); } diff --git a/libraries/AP_HAL_ChibiOS/RCOutput_bdshot.cpp b/libraries/AP_HAL_ChibiOS/RCOutput_bdshot.cpp index 801512a687d..ad6c66d7f8a 100644 --- a/libraries/AP_HAL_ChibiOS/RCOutput_bdshot.cpp +++ b/libraries/AP_HAL_ChibiOS/RCOutput_bdshot.cpp @@ -496,6 +496,13 @@ __RAMFUNC__ void RCOutput::bdshot_finish_dshot_gcr_transaction(virtual_timer_t* #ifdef HAL_GPIO_LINE_GPIO56 TOGGLE_PIN_DEBUG(56); #endif + osalDbgAssert(group->dshot_waiter, "No dshot waiter to signal"); + + if (group->dshot_waiter == nullptr) { // transaction was cancelled, leave everything alone + chSysUnlockFromISR(); + return; + } + uint8_t curr_telem_chan = group->bdshot.curr_telem_chan; // the DMA buffer is either the regular outbound one because we are sharing UP and CH @@ -542,6 +549,8 @@ __RAMFUNC__ void RCOutput::bdshot_finish_dshot_gcr_transaction(virtual_timer_t* // tell the waiting process we've done the DMA chEvtSignalI(group->dshot_waiter, group->dshot_event_mask); + group->dshot_waiter = nullptr; + #ifdef HAL_GPIO_LINE_GPIO56 TOGGLE_PIN_DEBUG(56); #endif