diff --git a/src/main/mw.c b/src/main/mw.c index 766ebb247f..5a376f8f23 100644 --- a/src/main/mw.c +++ b/src/main/mw.c @@ -178,7 +178,6 @@ void annexCode(void) static uint32_t vbatLastServiced = 0; static uint32_t ibatLastServiced = 0; - uint32_t ibatTimeSinceLastServiced; // PITCH & ROLL only dynamic PID adjustment, depending on throttle value if (rcData[THROTTLE] < currentControlRateProfile->tpa_breakpoint) { prop2 = 100; @@ -249,16 +248,15 @@ void annexCode(void) } if (feature(FEATURE_VBAT)) { - /* currentTime will rollover @ 70 minutes */ - if ((currentTime - vbatLastServiced) >= VBATINTERVAL) { - vbatLastServiced = currentTime; + if ((int32_t)(currentTime - vbatLastServiced) >= VBATINTERVAL) { + vbatLastServiced = currentTime; updateBattery(); } } if (feature(FEATURE_CURRENT_METER)) { - /* currentTime will rollover @ 70 minutes */ - ibatTimeSinceLastServiced = (currentTime - ibatLastServiced); + int32_t ibatTimeSinceLastServiced = (int32_t) (currentTime - ibatLastServiced); + if (ibatTimeSinceLastServiced >= IBATINTERVAL) { ibatLastServiced = currentTime; updateCurrentMeter((ibatTimeSinceLastServiced / 1000), &masterConfig.rxConfig, masterConfig.flight3DConfig.deadband3d_throttle); diff --git a/src/main/sensors/battery.c b/src/main/sensors/battery.c index dc83519b2f..0b8c5c3406 100644 --- a/src/main/sensors/battery.c +++ b/src/main/sensors/battery.c @@ -34,7 +34,7 @@ #include "flight/lowpass.h" #include "io/beeper.h" -#define VBATT_DETECT 10 +#define VBATT_PRESENT_THRESHOLD_MV 10 #define VBATT_LPF_FREQ 10 // Battery monitoring stuff @@ -81,7 +81,7 @@ void updateBattery(void) updateBatteryVoltage(); /* battery has just been connected*/ - if(batteryState == BATTERY_NOTPRESENT && vbat > VBATT_DETECT) + if (batteryState == BATTERY_NOT_PRESENT && vbat > VBATT_PRESENT_THRESHOLD_MV) { /* Actual battery state is calculated below, this is really BATTERY_PRESENT */ batteryState = BATTERY_OK; @@ -93,16 +93,18 @@ void updateBattery(void) updateBatteryVoltage(); unsigned cells = (batteryAdcToVoltage(vbatLatestADC) / batteryConfig->vbatmaxcellvoltage) + 1; - if(cells > 8) // something is wrong, we expect 8 cells maximum (and autodetection will be problematic at 6+ cells) + if (cells > 8) { + // something is wrong, we expect 8 cells maximum (and autodetection will be problematic at 6+ cells) cells = 8; + } batteryCellCount = cells; batteryWarningVoltage = batteryCellCount * batteryConfig->vbatwarningcellvoltage; batteryCriticalVoltage = batteryCellCount * batteryConfig->vbatmincellvoltage; } - /* battery has been disconnected - can take a while for filter cap to disharge so we use a threshold of VBATT_DETECT */ - else if(batteryState != BATTERY_NOTPRESENT && vbat <= VBATT_DETECT) + /* battery has been disconnected - can take a while for filter cap to disharge so we use a threshold of VBATT_PRESENT_THRESHOLD_MV */ + else if (batteryState != BATTERY_NOT_PRESENT && vbat <= VBATT_PRESENT_THRESHOLD_MV) { - batteryState = BATTERY_NOTPRESENT; + batteryState = BATTERY_NOT_PRESENT; batteryCellCount = 0; batteryWarningVoltage = 0; batteryCriticalVoltage = 0; @@ -111,33 +113,30 @@ void updateBattery(void) switch(batteryState) { case BATTERY_OK: - if(vbat <= (batteryWarningVoltage - VBATT_HYSTERESIS)){ + if (vbat <= (batteryWarningVoltage - VBATT_HYSTERESIS)) { batteryState = BATTERY_WARNING; beeper(BEEPER_BAT_LOW); } break; case BATTERY_WARNING: - if(vbat <= (batteryCriticalVoltage - VBATT_HYSTERESIS)){ + if (vbat <= (batteryCriticalVoltage - VBATT_HYSTERESIS)) { batteryState = BATTERY_CRITICAL; - beeper(BEEPER_BAT_CRIT_LOW); - } - else if(vbat > (batteryWarningVoltage + VBATT_HYSTERESIS)){ + beeper(BEEPER_BAT_CRIT_LOW); + } else if (vbat > (batteryWarningVoltage + VBATT_HYSTERESIS)){ batteryState = BATTERY_OK; - } - else{ - beeper(BEEPER_BAT_LOW); + } else { + beeper(BEEPER_BAT_LOW); } break; case BATTERY_CRITICAL: - if(vbat > (batteryCriticalVoltage + VBATT_HYSTERESIS)){ + if (vbat > (batteryCriticalVoltage + VBATT_HYSTERESIS)){ batteryState = BATTERY_WARNING; beeper(BEEPER_BAT_LOW); - } - else{ - beeper(BEEPER_BAT_CRIT_LOW); + } else { + beeper(BEEPER_BAT_CRIT_LOW); } break; - case BATTERY_NOTPRESENT: + case BATTERY_NOT_PRESENT: break; } } @@ -157,7 +156,7 @@ const char * getBatteryStateString(void) void batteryInit(batteryConfig_t *initialBatteryConfig) { batteryConfig = initialBatteryConfig; - batteryState = BATTERY_NOTPRESENT; + batteryState = BATTERY_NOT_PRESENT; batteryCellCount = 1; batteryWarningVoltage = 0; batteryCriticalVoltage = 0; @@ -189,7 +188,7 @@ void updateCurrentMeter(int32_t lastUpdateAt, rxConfig_t *rxConfig, uint16_t dea break; case CURRENT_SENSOR_VIRTUAL: amperage = (int32_t)batteryConfig->currentMeterOffset; - if(ARMING_FLAG(ARMED)) { + if (ARMING_FLAG(ARMED)) { throttleStatus_e throttleStatus = calculateThrottleStatus(rxConfig, deadband3d_throttle); if (throttleStatus == THROTTLE_LOW && feature(FEATURE_MOTOR_STOP)) throttleOffset = 0; diff --git a/src/main/sensors/battery.h b/src/main/sensors/battery.h index 8214c7203b..197ebe99cb 100644 --- a/src/main/sensors/battery.h +++ b/src/main/sensors/battery.h @@ -49,7 +49,7 @@ typedef enum { BATTERY_OK = 0, BATTERY_WARNING, BATTERY_CRITICAL, - BATTERY_NOTPRESENT + BATTERY_NOT_PRESENT } batteryState_e; extern uint16_t vbat; diff --git a/src/test/unit/battery_unittest.cc b/src/test/unit/battery_unittest.cc index 388ea6e2a1..948e76ec30 100644 --- a/src/test/unit/battery_unittest.cc +++ b/src/test/unit/battery_unittest.cc @@ -116,17 +116,17 @@ TEST(BatteryTest, BatteryState) batteryAdcToBatteryStateExpectation_t batteryAdcToBatteryStateExpectations[] = { {1420, 126, BATTERY_OK, ELEVEN_TO_ONE_VOLTAGE_DIVIDER}, - /* fall down to battery warning level */ + /* fall down to battery warning level */ {1185, 105, BATTERY_OK, ELEVEN_TO_ONE_VOLTAGE_DIVIDER}, {1175, 104, BATTERY_WARNING, ELEVEN_TO_ONE_VOLTAGE_DIVIDER}, - /* creep back up to battery ok */ + /* creep back up to battery ok */ {1185, 105, BATTERY_WARNING, ELEVEN_TO_ONE_VOLTAGE_DIVIDER}, {1195, 106, BATTERY_WARNING, ELEVEN_TO_ONE_VOLTAGE_DIVIDER}, {1207, 107, BATTERY_OK, ELEVEN_TO_ONE_VOLTAGE_DIVIDER}, - /* fall down to battery critical level */ + /* fall down to battery critical level */ {1175, 104, BATTERY_WARNING, ELEVEN_TO_ONE_VOLTAGE_DIVIDER}, {1108, 98, BATTERY_CRITICAL, ELEVEN_TO_ONE_VOLTAGE_DIVIDER}, - /* creep back up to battery warning */ + /* creep back up to battery warning */ {1115, 99, BATTERY_CRITICAL, ELEVEN_TO_ONE_VOLTAGE_DIVIDER}, {1130, 100, BATTERY_CRITICAL, ELEVEN_TO_ONE_VOLTAGE_DIVIDER}, {1145, 101, BATTERY_WARNING, ELEVEN_TO_ONE_VOLTAGE_DIVIDER}, @@ -139,8 +139,8 @@ TEST(BatteryTest, BatteryState) batteryAdcToBatteryStateExpectation_t *batteryAdcToBatteryStateExpectation = &batteryAdcToBatteryStateExpectations[index]; batteryConfig.vbatscale = batteryAdcToBatteryStateExpectation->scale; currentADCReading = batteryAdcToBatteryStateExpectation->adcReading; - updateBattery( ); - batteryState_e batteryState = getBatteryState(); + updateBattery( ); + batteryState_e batteryState = getBatteryState(); EXPECT_EQ(batteryAdcToBatteryStateExpectation->expectedBatteryState, batteryState); } } @@ -182,11 +182,83 @@ TEST(BatteryTest, CellCount) batteryAdcToCellCountExpectation_t *batteryAdcToCellCountExpectation = &batteryAdcToCellCountExpectations[index]; batteryConfig.vbatscale = batteryAdcToCellCountExpectation->scale; currentADCReading = batteryAdcToCellCountExpectation->adcReading; - updateBattery( ); + updateBattery( ); EXPECT_EQ(batteryAdcToCellCountExpectation->cellCount, batteryCellCount); } } +/** + * These next two tests do not test any production code (!) but serves as an example of how to use a signed variable for timing purposes. + * + * The 'signed diff timing' pattern is followed in a few places in the production codebase. + */ +TEST(BatteryTest, RollOverPattern1) +{ + uint16_t now = 0; + uint16_t servicedAt = 0; + uint16_t serviceInterval = 5000; + int serviceCount = 0; + int rolloverCount = 0; + + while(rolloverCount < 3) { + + int16_t diff = (now - servicedAt); + if (diff >= serviceInterval) { + + if (now < servicedAt) { + rolloverCount++; + } + + servicedAt = now; +#if 1 + printf("servicedAt: %d, diff: %d\n", servicedAt, diff); +#endif + serviceCount++; + + EXPECT_GT(diff, 0); + EXPECT_GE(diff, serviceInterval); // service interval must have passed + EXPECT_LT(diff, serviceInterval + 95 + 10); // but never more than the service interval + ticks + jitter + } + + now += 95 + (rand() % 10); // simulate 100 ticks +/- 5 ticks of jitter, this can rollover + } + EXPECT_GT(serviceCount, 0); +} + +TEST(BatteryTest, RollOverPattern2) +{ + uint16_t now = 0; + uint16_t serviceAt = 0; + uint16_t serviceInterval = 5000; + int serviceCount = 0; + int rolloverCount = 0; + + while(rolloverCount < 3) { + + int16_t diff = (now - serviceAt); + if (diff >= 0) { + + if (now < serviceAt) { + rolloverCount++; + } + + serviceAt = now + serviceInterval; // this can rollover +#if 1 + printf("servicedAt: %d, nextServiceAt: %d, diff: %d\n", now, serviceAt, diff); +#endif + + serviceCount++; + + EXPECT_GE(diff, 0); + EXPECT_LE(diff, serviceInterval); + EXPECT_LT(diff, 95 + 10); // never more than the ticks + jitter + } + + now += 95 + (rand() % 10); // simulate 100 ticks +/- 5 ticks of jitter, this can rollover + } + EXPECT_GT(serviceCount, 0); +} + // STUBS