Cleanup vbat time rollover. Cleanup whitespace. Code formatting.

Rename VBAT_DETECT to VBATT_PRESENT_THRESHOLD_MV.

Add two tests that show the two timing patterns that are in use in the
codebase.

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 ((int32_t)(currentTime - vbatLastServiced) >= VBATINTERVAL) {
-        if ((currentTime - vbatLastServiced) >= VBATINTERVAL) {
+            vbatLastServiced = currentTime;
-           vbatLastServiced = currentTime;
             updateBattery();
         }
     }
 
     if (feature(FEATURE_CURRENT_METER)) {
-        /* currentTime will rollover @ 70 minutes */
+        int32_t ibatTimeSinceLastServiced = (int32_t) (currentTime - ibatLastServiced);
-        ibatTimeSinceLastServiced = (currentTime - ibatLastServiced);
+
         if (ibatTimeSinceLastServiced >= IBATINTERVAL) {
             ibatLastServiced = currentTime;
             updateCurrentMeter((ibatTimeSinceLastServiced / 1000), &masterConfig.rxConfig, masterConfig.flight3DConfig.deadband3d_throttle);

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 */
+    /* 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_NOTPRESENT && vbat <= VBATT_DETECT)
+    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);
+                beeper(BEEPER_BAT_CRIT_LOW);
-            }
+            } else if (vbat > (batteryWarningVoltage + VBATT_HYSTERESIS)){
-            else if(vbat > (batteryWarningVoltage + VBATT_HYSTERESIS)){
                 batteryState = BATTERY_OK;
-            }
+            } else {
-            else{
+                beeper(BEEPER_BAT_LOW);
-                 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 {
-            else{
+                beeper(BEEPER_BAT_CRIT_LOW);
-                 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;

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;

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( );
+        updateBattery( );
-	batteryState_e batteryState = getBatteryState();
+        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