Fix bug where aux configuration for channels 5-8 were ignored.

The bug was introduced by a recent refactor which was in error.

Fixed using TDD by first creating a failing unit test and then fixing
the code.

src/main/io/rc_controls.c

@@ -224,30 +224,38 @@ void processRcStickPositions(rxConfig_t *rxConfig, throttleStatus_e throttleStat
 
 void updateRcOptions(uint32_t *activate)
 {
-    // Check AUX switches
-
-    // auxState is a bitmask, 3 bits per channel. aux1 is first.
-    // lower 16 bits contain aux 1 to 4, upper 16 bits contain aux 5 to 8
+    // auxState is a bitmask, 3 bits per channel.
+    // lower 16 bits contain aux 4 to 1 (msb to lsb)
+    // upper 16 bits contain aux 8 to 5 (msb to lsb)
     //
     // the three bits are as follows:
     // bit 1 is SET when the stick is less than 1300
     // bit 2 is SET when the stick is between 1300 and 1700
     // bit 3 is SET when the stick is above 1700
+
     // if the value is 1300 or 1700 NONE of the three bits are set.
 
     int i;
     uint32_t auxState = 0;
+    uint8_t shift = 0;
+    int8_t chunkOffset = 0;
 
     for (i = 0; i < rxRuntimeConfig.auxChannelCount && i < MAX_AUX_STATE_CHANNELS; i++) {
-    	uint32_t temp = (rcData[AUX1 + i] < 1300) << (3 * i) |
-				(1300 < rcData[AUX1 + i] && rcData[AUX1 + i] < 1700) << (3 * i + 1) |
-				(rcData[AUX1 + i] > 1700) << (3 * i + 2);
+        if (i > 0 && i % 4 == 0) {
+            chunkOffset -= 4;
+            shift += 16;
+        }
 
-    	if (i >= 4 && i < 8) {
-    		temp <<= 16;
-    	}
-		auxState |= temp;
+        uint8_t bitIndex = 3 * (i + chunkOffset);
+
+        uint32_t temp =
+                (rcData[AUX1 + i] < 1300) << bitIndex |
+                (1300 < rcData[AUX1 + i] && rcData[AUX1 + i] < 1700) << (bitIndex + 1) |
+                (rcData[AUX1 + i] > 1700) << (bitIndex + 2);
+
+        auxState |= temp << shift;
     }
+
     for (i = 0; i < CHECKBOX_ITEM_COUNT; i++)
         rcOptions[i] = (auxState & activate[i]) > 0;
 }

src/main/rx/rx.h

@@ -20,7 +20,7 @@
 #define PWM_RANGE_ZERO 0 // FIXME should all usages of this be changed to use PWM_RANGE_MIN?
 #define PWM_RANGE_MIN 1000
 #define PWM_RANGE_MAX 2000
-#define PWM_RANGE_MIDDLE ((PWM_RANGE_MAX - PWM_RANGE_MIN) / 2)
+#define PWM_RANGE_MIDDLE (PWM_RANGE_MIN + ((PWM_RANGE_MAX - PWM_RANGE_MIN) / 2))
 
 #define DEFAULT_SERVO_MIN 1020
 #define DEFAULT_SERVO_MIDDLE 1500

src/test/unit/rc_controls_unittest.cc

@@ -47,7 +47,7 @@ TEST(RcControlsTest, updateRcOptionsWithAllInputsAtMidde)
     rxRuntimeConfig.auxChannelCount = MAX_SUPPORTED_RC_CHANNEL_COUNT - NON_AUX_CHANNEL_COUNT;
 
     // and
-    for (index = AUX1; index < MAX_SUPPORTED_RC_CHANNEL_COUNT - NON_AUX_CHANNEL_COUNT; index++) {
+    for (index = AUX1; index < MAX_SUPPORTED_RC_CHANNEL_COUNT; index++) {
         rcData[index] = PWM_RANGE_MIDDLE;
     }
 
@@ -66,6 +66,68 @@ TEST(RcControlsTest, updateRcOptionsWithAllInputsAtMidde)
     }
 }
 
+TEST(RcControlsTest, updateRcOptionsUsingValidAuxConfigurationAndRXValues)
+{
+    // given
+    uint32_t activate[CHECKBOX_ITEM_COUNT];
+    memset(&activate, 0, sizeof(activate));
+    activate[0] = 0b000000000100UL;
+    activate[1] = 0b000000010000UL;
+    activate[2] = 0b000001000000UL;
+    activate[3] = 0b111000000000UL;
+
+    activate[4] = 0b000000000001UL << 16;
+    activate[5] = 0b000000010000UL << 16;
+    activate[6] = 0b000100000000UL << 16;
+    activate[7] = 0b111000000000UL << 16;
+
+    uint8_t index;
+
+    for (index = 0; index < CHECKBOX_ITEM_COUNT; index++) {
+        rcOptions[index] = 0;
+    }
+
+    // and
+    memset(&rxRuntimeConfig, 0, sizeof(rxRuntimeConfig_t));
+    rxRuntimeConfig.auxChannelCount = MAX_SUPPORTED_RC_CHANNEL_COUNT - NON_AUX_CHANNEL_COUNT;
+
+    // and
+    for (index = AUX1; index < MAX_SUPPORTED_RC_CHANNEL_COUNT; index++) {
+        rcData[index] = PWM_RANGE_MIDDLE;
+    }
+
+    rcData[AUX1] = PWM_RANGE_MAX;
+    rcData[AUX2] = PWM_RANGE_MIDDLE;
+    rcData[AUX3] = PWM_RANGE_MIN;
+    rcData[AUX4] = PWM_RANGE_MAX;
+    rcData[AUX5] = PWM_RANGE_MIN;
+    rcData[AUX6] = PWM_RANGE_MIDDLE;
+    rcData[AUX7] = PWM_RANGE_MAX;
+    rcData[AUX8] = PWM_RANGE_MIN;
+
+
+    // and
+    uint32_t expectedRcOptions[CHECKBOX_ITEM_COUNT];
+    memset(&expectedRcOptions, 0, sizeof(expectedRcOptions));
+    expectedRcOptions[0] = 1;
+    expectedRcOptions[1] = 1;
+    expectedRcOptions[2] = 1;
+    expectedRcOptions[3] = 1;
+    expectedRcOptions[4] = 1;
+    expectedRcOptions[5] = 1;
+    expectedRcOptions[6] = 1;
+    expectedRcOptions[7] = 1;
+
+    // when
+    updateRcOptions(activate);
+
+    // then
+    for (index = 0; index < CHECKBOX_ITEM_COUNT; index++) {
+        printf("iteration: %d\n", index);
+        EXPECT_EQ(expectedRcOptions[index], rcOptions[index]);
+    }
+}
+
 void changeProfile(uint8_t profileIndex) {}
 void accSetCalibrationCycles(uint16_t) {}
 void gyroSetCalibrationCycles(uint16_t) {}