From 5a36db26c94c0bc97f77c47aa470c3b65aec4a7d Mon Sep 17 00:00:00 2001 From: Dominic Clifton Date: Sat, 6 Sep 2014 00:28:48 +0100 Subject: [PATCH] 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 | 30 ++++++++----- src/main/rx/rx.h | 2 +- src/test/unit/rc_controls_unittest.cc | 64 ++++++++++++++++++++++++++- 3 files changed, 83 insertions(+), 13 deletions(-) diff --git a/src/main/io/rc_controls.c b/src/main/io/rc_controls.c index c3d070f4c0..8ac1ff1572 100644 --- a/src/main/io/rc_controls.c +++ b/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; } diff --git a/src/main/rx/rx.h b/src/main/rx/rx.h index b8c5584c00..15eebffd26 100644 --- a/src/main/rx/rx.h +++ b/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 diff --git a/src/test/unit/rc_controls_unittest.cc b/src/test/unit/rc_controls_unittest.cc index 789c6a60fe..17af3a76eb 100644 --- a/src/test/unit/rc_controls_unittest.cc +++ b/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) {}