From 655965d7a6bddd16ebb9a658e302a243ca2bdccd Mon Sep 17 00:00:00 2001
From: Raphael Coeffic <raphaelcoeffic@users.noreply.github.com>
Date: Wed, 12 Oct 2022 04:45:10 +0200
Subject: [PATCH] fix: prevent endless loop with SBUS trainer on model change
 (#2515)

* fix: prevent generic module de-init if protocol is NONE

Fixes #1898

De-initialising the external module while SBUS trainer is used on external module RX leads to the following sequence:
- `extmoduleStop()` is called, thus shutting down the timer DMA
- on certain targets, the timer DMA stream is the same as the external module RX DMA stream
- when this DMA stream is used by the SBUS trainer input, this leads to DMAFifo to return random bytes continuously
- thus driving `processSbusInput()` into an endless loop, which is run in the mixer task
- as the mixer task has the highest priority, it runs forever
- after some time the watchdog timer expires, and causes a reboot

* DMAFifo: check if the stream is enabled

This provides some hardening to avoid the conditions described in previous commit.
---
 radio/src/dmafifo.h         |  2 +-
 radio/src/pulses/pulses.cpp | 13 ++++++++-----
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/radio/src/dmafifo.h b/radio/src/dmafifo.h
index 837dfd2da86..bf19fb935e0 100644
--- a/radio/src/dmafifo.h
+++ b/radio/src/dmafifo.h
@@ -54,7 +54,7 @@ class DMAFifo
 #if defined(SIMU)
       return true;
 #endif
-      return (ridx == N - stream->NDTR);
+      return !(stream->CR & DMA_SxCR_EN) || (ridx == N - stream->NDTR);
     }
 
     bool pop(uint8_t & element)
diff --git a/radio/src/pulses/pulses.cpp b/radio/src/pulses/pulses.cpp
index 1e6dddc5902..d6dc3b8d797 100755
--- a/radio/src/pulses/pulses.cpp
+++ b/radio/src/pulses/pulses.cpp
@@ -458,7 +458,9 @@ bool setupPulsesInternalModule(uint8_t protocol)
 
 void stopPulsesInternalModule()
 {
-  if (moduleState[INTERNAL_MODULE].protocol != PROTOCOL_CHANNELS_UNINITIALIZED) {
+  auto& proto = moduleState[INTERNAL_MODULE].protocol;
+  if (proto != PROTOCOL_CHANNELS_UNINITIALIZED &&
+      proto != PROTOCOL_CHANNELS_NONE) {
     if (internalModuleDriver) {
       internalModuleDriver->deinit(internalModuleContext);
       internalModuleDriver = nullptr;
@@ -467,7 +469,7 @@ void stopPulsesInternalModule()
       mixerSchedulerSetPeriod(INTERNAL_MODULE, 0);
       intmoduleStop();
     }
-    moduleState[INTERNAL_MODULE].protocol = PROTOCOL_CHANNELS_NONE;
+    proto = PROTOCOL_CHANNELS_NONE;
   }
 }
 
@@ -766,8 +768,9 @@ void extmoduleSendNextFrame()
 
 void stopPulsesExternalModule()
 {
-  if (moduleState[EXTERNAL_MODULE].protocol !=
-      PROTOCOL_CHANNELS_UNINITIALIZED) {
+  auto& proto = moduleState[EXTERNAL_MODULE].protocol;
+  if (proto != PROTOCOL_CHANNELS_UNINITIALIZED &&
+      proto != PROTOCOL_CHANNELS_NONE) {
     if (externalModuleDriver) {
       externalModuleDriver->deinit(externalModuleContext);
       externalModuleDriver = nullptr;
@@ -776,7 +779,7 @@ void stopPulsesExternalModule()
       mixerSchedulerSetPeriod(EXTERNAL_MODULE, 0);
       extmoduleStop();
     }
-    moduleState[EXTERNAL_MODULE].protocol = PROTOCOL_CHANNELS_NONE;
+    proto = PROTOCOL_CHANNELS_NONE;
   }
 }