Cleanup PWM rx loss detection.

src/main/rx/rx.c

@@ -68,7 +68,6 @@ uint16_t rssi = 0;                  // range: [0;1023]
 static bool rxDataReceived = false;
 static bool rxSignalReceived = false;
 static bool shouldCheckPulse = true;
-static bool rxPwmFlightChannelsAreGood = false;
 
 static uint32_t rxUpdateAt = 0;
 static uint32_t needRxSignalBefore = 0;
@@ -94,24 +93,26 @@ void useRxConfig(rxConfig_t *rxConfigToUse)
 
 #define REQUIRED_CHANNEL_MASK 0x0F // first 4 channels
 
-// pulse duration is in micro seconds (usec)
+static uint8_t validFlightChannelMask;
-STATIC_UNIT_TESTED void rxCheckPulse(uint8_t channel, uint16_t pulseDuration)
-{
-    static uint8_t goodChannelMask = 0;
 
-    if (channel < 4 &&
+STATIC_UNIT_TESTED void rxResetFlightChannelStatus(void) {
+    validFlightChannelMask = 0;
+}
+
+STATIC_UNIT_TESTED bool rxHaveValidFlightChannels(void)
+{
+    return (validFlightChannelMask == REQUIRED_CHANNEL_MASK);
+}
+
+// pulse duration is in micro seconds (usec)
+STATIC_UNIT_TESTED void rxUpdateFlightChannelStatus(uint8_t channel, uint16_t pulseDuration)
+{
+    if (channel < NON_AUX_CHANNEL_COUNT &&
         pulseDuration >= rxConfig->rx_min_usec &&
         pulseDuration <= rxConfig->rx_max_usec
     ) {
         // if signal is valid - mark channel as OK
-        goodChannelMask |= (1 << channel);
+        validFlightChannelMask |= (1 << channel);
-    }
-
-    if (goodChannelMask == REQUIRED_CHANNEL_MASK) {
-        goodChannelMask = 0;
-        failsafeOnValidDataReceived();
-        rxSignalReceived = true;
-        rxPwmFlightChannelsAreGood = true;
     }
 }
 
@@ -262,7 +263,7 @@ void updateRx(uint32_t currentTime)
     }
 
     if ((feature(FEATURE_RX_SERIAL | FEATURE_RX_MSP) && rxDataReceived)
-         || feature(FEATURE_RX_PARALLEL_PWM)) {
+          || feature(FEATURE_RX_PARALLEL_PWM)) {
         needRxSignalBefore = currentTime + DELAY_10_HZ;
     }
 
@@ -334,7 +335,6 @@ static uint16_t getRxfailValue(uint8_t channel)
 
         case RX_FAILSAFE_MODE_SET:
             return RXFAIL_STEP_TO_CHANNEL_VALUE(channelFailsafeConfiguration->step);
-
     }
 
 }
@@ -347,7 +347,7 @@ static void processRxChannels(void)
         return; // rcData will have already been updated by MSP_SET_RAW_RC
     }
 
-    rxPwmFlightChannelsAreGood = false;
+    rxResetFlightChannelStatus();
 
     for (chan = 0; chan < rxRuntimeConfig.channelCount; chan++) {
 
@@ -361,11 +361,8 @@ static void processRxChannels(void)
         // sample the channel
         uint16_t sample = rcReadRawFunc(&rxRuntimeConfig, rawChannel);
 
-        if (shouldCheckPulse) {
+        rxUpdateFlightChannelStatus(chan, sample);
-            rxCheckPulse(chan, sample);
-        }
 
-        // validate the range and check if rx signal is received
         if (sample < rxConfig->rx_min_usec || sample > rxConfig->rx_max_usec || !rxSignalReceived) {
             sample = getRxfailValue(chan);
         }
@@ -377,9 +374,16 @@ static void processRxChannels(void)
         }
     }
 
-    // Using PARALLEL PWM and one of the 4 control channels is out of range:
+    if (rxHaveValidFlightChannels()) {
-    // Probably one of the cables came loose, all channels are set to rxfail values
+        if (shouldCheckPulse) {
-    if ((rxPwmFlightChannelsAreGood == false) && feature(FEATURE_RX_PARALLEL_PWM)) {
+            failsafeOnValidDataReceived();
+            rxSignalReceived = true;
+        }
+    } else {
+        if (feature(FEATURE_RX_PARALLEL_PWM)) {
+            rxSignalReceived = false;
+        }
+
         for (chan = 0; chan < rxRuntimeConfig.channelCount; chan++) {
             rcData[chan] = getRxfailValue(chan);
         }

src/test/unit/rx_rx_unittest.cc

@@ -26,35 +26,23 @@ extern "C" {
     #include "rx/rx.h"
 
     void rxInit(rxConfig_t *rxConfig);
-    void rxCheckPulse(uint8_t channel, uint16_t pulseDuration);
+    void rxResetFlightChannelStatus(void);
+    bool rxHaveValidFlightChannels(void);
+    void rxUpdateFlightChannelStatus(uint8_t channel, uint16_t pulseDuration);
 }
 
 #include "unittest_macros.h"
 #include "gtest/gtest.h"
 
-enum {
-    COUNTER_FAILSAFE_ON_VALID_DATA_RECEIVED = 0,
-};
-#define CALL_COUNT_ITEM_COUNT 1
-
-static int callCounts[CALL_COUNT_ITEM_COUNT];
-
-#define CALL_COUNTER(item) (callCounts[item])
-
-void resetCallCounters(void) {
-    memset(&callCounts, 0, sizeof(callCounts));
-}
-
 typedef struct testData_s {
     bool isPPMDataBeingReceived;
 } testData_t;
 
 static testData_t testData;
 
-TEST(RxTest, TestFailsafeInformedOfValidData)
+TEST(RxTest, TestValidFlightChannels)
 {
     // given
-    resetCallCounters();
     memset(&testData, 0, sizeof(testData));
 
     // and
@@ -67,15 +55,17 @@ TEST(RxTest, TestFailsafeInformedOfValidData)
     // when
     rxInit(&rxConfig);
 
+    rxResetFlightChannelStatus();
+
     for (uint8_t channelIndex = 0; channelIndex < MAX_SUPPORTED_RC_CHANNEL_COUNT; channelIndex++) {
-        rxCheckPulse(channelIndex, 1500);
+        rxUpdateFlightChannelStatus(channelIndex, 1500);
     }
 
     // then
-    EXPECT_EQ(1, CALL_COUNTER(COUNTER_FAILSAFE_ON_VALID_DATA_RECEIVED));
+    EXPECT_TRUE(rxHaveValidFlightChannels());
 }
 
-TEST(RxTest, TestFailsafeNotInformedOfValidDataWhenStickChannelsAreBad)
+TEST(RxTest, TestInvalidFlightChannels)
 {
     // given
     memset(&testData, 0, sizeof(testData));
@@ -99,36 +89,36 @@ TEST(RxTest, TestFailsafeNotInformedOfValidDataWhenStickChannelsAreBad)
     for (uint8_t stickChannelIndex = 0; stickChannelIndex < STICK_CHANNEL_COUNT; stickChannelIndex++) {
 
         // given
-        resetCallCounters();
+        rxResetFlightChannelStatus();
 
-        for (uint8_t channelIndex = 0; channelIndex < STICK_CHANNEL_COUNT; channelIndex++) {
+        for (uint8_t otherStickChannelIndex = 0; otherStickChannelIndex < STICK_CHANNEL_COUNT; otherStickChannelIndex++) {
-            channelPulses[stickChannelIndex] = rxConfig.rx_min_usec;
+            channelPulses[otherStickChannelIndex] = rxConfig.rx_min_usec;
         }
         channelPulses[stickChannelIndex] = rxConfig.rx_min_usec - 1;
 
         // when
         for (uint8_t channelIndex = 0; channelIndex < MAX_SUPPORTED_RC_CHANNEL_COUNT; channelIndex++) {
-            rxCheckPulse(channelIndex, channelPulses[channelIndex]);
+            rxUpdateFlightChannelStatus(channelIndex, channelPulses[channelIndex]);
         }
 
         // then
-        EXPECT_EQ(0, CALL_COUNTER(COUNTER_FAILSAFE_ON_VALID_DATA_RECEIVED));
+        EXPECT_FALSE(rxHaveValidFlightChannels());
 
         // given
-        resetCallCounters();
+        rxResetFlightChannelStatus();
 
-        for (uint8_t channelIndex = 0; channelIndex < STICK_CHANNEL_COUNT; channelIndex++) {
+        for (uint8_t otherStickChannelIndex = 0; otherStickChannelIndex < STICK_CHANNEL_COUNT; otherStickChannelIndex++) {
-            channelPulses[stickChannelIndex] = rxConfig.rx_max_usec;
+            channelPulses[otherStickChannelIndex] = rxConfig.rx_max_usec;
         }
         channelPulses[stickChannelIndex] = rxConfig.rx_max_usec + 1;
 
         // when
         for (uint8_t channelIndex = 0; channelIndex < MAX_SUPPORTED_RC_CHANNEL_COUNT; channelIndex++) {
-            rxCheckPulse(channelIndex, channelPulses[channelIndex]);
+            rxUpdateFlightChannelStatus(channelIndex, channelPulses[channelIndex]);
         }
 
         // then
-        EXPECT_EQ(0, CALL_COUNTER(COUNTER_FAILSAFE_ON_VALID_DATA_RECEIVED));
+        EXPECT_FALSE(rxHaveValidFlightChannels());
     }
 }
 
@@ -137,9 +127,7 @@ TEST(RxTest, TestFailsafeNotInformedOfValidDataWhenStickChannelsAreBad)
 
 extern "C" {
     void failsafeOnRxCycleStarted() {}
-    void failsafeOnValidDataReceived() {
+    void failsafeOnValidDataReceived() {}
-        callCounts[COUNTER_FAILSAFE_ON_VALID_DATA_RECEIVED]++;
-    }
 
     bool feature(uint32_t mask) {
         UNUSED(mask);