Merge pull request #3615 from DanNixon/osd_battery_current_formatting

Improve formatting of numerical OSD elements

src/main/io/osd.c

@@ -206,6 +206,16 @@ static int32_t osdGetMetersToSelectedUnit(int32_t meters)
     }
 }
 
+static void osdFormatAltitudeString(char * buff, int altitude, bool pad)
+{
+    const int alt = osdGetMetersToSelectedUnit(altitude);
+    int altitudeIntergerPart = abs(alt / 100);
+    if (alt < 0) {
+        altitudeIntergerPart *= -1;
+    }
+    tfp_sprintf(buff, pad ? "%4d.%01d%c" : "%d.%01d%c", altitudeIntergerPart, abs((alt % 100) / 10), osdGetMetersToSelectedUnitSymbol());
+}
+
 static void osdFormatPID(char * buff, const char * label, const pid8_t * pid)
 {
     tfp_sprintf(buff, "%s %3d %3d %3d", label, pid->P, pid->I, pid->D);
@@ -309,8 +319,7 @@ static void osdDrawSingleElement(uint8_t item)
             if (osdRssi >= 100)
                 osdRssi = 99;
 
-            buff[0] = SYM_RSSI;
+            tfp_sprintf(buff, "%c%d", SYM_RSSI, osdRssi);
-            tfp_sprintf(buff + 1, "%d", osdRssi);
             break;
         }
 
@@ -322,20 +331,17 @@ static void osdDrawSingleElement(uint8_t item)
     case OSD_CURRENT_DRAW:
         {
             const int32_t amperage = getAmperage();
-            buff[0] = SYM_AMP;
+            tfp_sprintf(buff, "%3d.%02d%c", abs(amperage) / 100, abs(amperage) % 100, SYM_AMP);
-            tfp_sprintf(buff + 1, "%d.%02d", abs(amperage) / 100, abs(amperage) % 100);
             break;
         }
 
     case OSD_MAH_DRAWN:
-        buff[0] = SYM_MAH;
+        tfp_sprintf(buff, "%4d%c", getMAhDrawn(), SYM_MAH);
-        tfp_sprintf(buff + 1, "%d", getMAhDrawn());
         break;
 
 #ifdef GPS
     case OSD_GPS_SATS:
-        buff[0] = 0x1f;
+        tfp_sprintf(buff, "%c%d", 0x1f, gpsSol.numSat);
-        tfp_sprintf(buff + 1, "%d", gpsSol.numSat);
         break;
 
     case OSD_GPS_SPEED:
@@ -410,8 +416,7 @@ static void osdDrawSingleElement(uint8_t item)
 
     case OSD_ALTITUDE:
         {
-            const int32_t alt = osdGetMetersToSelectedUnit(getEstimatedAltitude());
+            osdFormatAltitudeString(buff, getEstimatedAltitude(), true);
-            tfp_sprintf(buff, "%c%d.%01d%c", alt < 0 ? '-' : ' ', abs(alt / 100), abs((alt % 100) / 10), osdGetMetersToSelectedUnitSymbol());
             break;
         }
 
@@ -682,8 +687,7 @@ static void osdDrawSingleElement(uint8_t item)
         }
 #ifdef USE_ESC_SENSOR
     case OSD_ESC_TMP:
-        buff[0] = SYM_TEMP_C;
+        tfp_sprintf(buff, "%d%c", escData == NULL ? 0 : escData->temperature, SYM_TEMP_C);
-        tfp_sprintf(buff + 1, "%d", escData == NULL ? 0 : escData->temperature);
         break;
 
     case OSD_ESC_RPM:
@@ -1078,8 +1082,7 @@ static void osdShowStats(void)
     }
 
     if (osdConfig()->enabled_stats[OSD_STAT_MAX_ALTITUDE]) {
-        int32_t alt = osdGetMetersToSelectedUnit(stats.max_altitude);
+        osdFormatAltitudeString(buff, stats.max_altitude, false);
-        tfp_sprintf(buff, "%c%d.%01d%c", alt < 0 ? '-' : ' ', abs(alt / 100), abs((alt % 100) / 10), osdGetMetersToSelectedUnitSymbol());
         osdDisplayStatisticLabel(top++, "MAX ALTITUDE", buff);
     }
 

src/test/unit/osd_unittest.cc

@@ -70,6 +70,8 @@ extern "C" {
     batteryState_e simulationBatteryState;
     uint8_t simulationBatteryCellCount;
     uint16_t simulationBatteryVoltage;
+    uint32_t simulationBatteryAmperage;
+    uint32_t simulationMahDrawn;
     int32_t simulationAltitude;
     int32_t simulationVerticalSpeed;
 }
@@ -87,6 +89,8 @@ void setDefualtSimulationState()
     simulationBatteryState = BATTERY_OK;
     simulationBatteryCellCount = 4;
     simulationBatteryVoltage = 168;
+    simulationBatteryAmperage = 0;
+    simulationMahDrawn = 0;
     simulationAltitude = 0;
     simulationVerticalSpeed = 0;
 }
@@ -337,7 +341,7 @@ TEST(OsdTest, TestStatsImperial)
     displayPortTestBufferSubstring(2, row++, "MIN BATTERY       : 14.7%c", SYM_VOLT);
     displayPortTestBufferSubstring(2, row++, "END BATTERY       : 15.2%c", SYM_VOLT);
     displayPortTestBufferSubstring(2, row++, "MIN RSSI          : 25%%");
-    displayPortTestBufferSubstring(2, row++, "MAX ALTITUDE      :  6.5%c", SYM_FT);
+    displayPortTestBufferSubstring(2, row++, "MAX ALTITUDE      : 6.5%c", SYM_FT);
 }
 
 /*
@@ -387,7 +391,7 @@ TEST(OsdTest, TestStatsMetric)
     displayPortTestBufferSubstring(2, row++, "MIN BATTERY       : 14.7%c", SYM_VOLT);
     displayPortTestBufferSubstring(2, row++, "END BATTERY       : 15.2%c", SYM_VOLT);
     displayPortTestBufferSubstring(2, row++, "MIN RSSI          : 25%%");
-    displayPortTestBufferSubstring(2, row++, "MAX ALTITUDE      :  2.0%c", SYM_M);
+    displayPortTestBufferSubstring(2, row++, "MAX ALTITUDE      : 2.0%c", SYM_M);
 }
 
 /*
@@ -403,7 +407,7 @@ TEST(OsdTest, TestAlarms)
     // the following OSD elements are visible
     osdConfigMutable()->item_pos[OSD_RSSI_VALUE]        = OSD_POS(8, 1)  | VISIBLE_FLAG;
     osdConfigMutable()->item_pos[OSD_MAIN_BATT_VOLTAGE] = OSD_POS(12, 1) | VISIBLE_FLAG;
-    osdConfigMutable()->item_pos[OSD_ITEM_TIMER_1]      = OSD_POS(20, 1)  | VISIBLE_FLAG;
+    osdConfigMutable()->item_pos[OSD_ITEM_TIMER_1]      = OSD_POS(20, 1) | VISIBLE_FLAG;
     osdConfigMutable()->item_pos[OSD_ITEM_TIMER_2]      = OSD_POS(1, 1)  | VISIBLE_FLAG;
     osdConfigMutable()->item_pos[OSD_ALTITUDE]          = OSD_POS(23, 7) | VISIBLE_FLAG;
 
@@ -449,7 +453,7 @@ TEST(OsdTest, TestAlarms)
         displayPortTestBufferSubstring(12, 1, "%c16.8%c", SYM_BATT_FULL, SYM_VOLT);
         displayPortTestBufferSubstring(1,  1, "%c00:", SYM_FLY_M); // only test the minute part of the timer
         displayPortTestBufferSubstring(20, 1, "%c01:", SYM_ON_M); // only test the minute part of the timer
-        displayPortTestBufferSubstring(23, 7, " 0.0%c", SYM_M);
+        displayPortTestBufferSubstring(23, 7, "   0.0%c", SYM_M);
     }
 
     // when
@@ -518,6 +522,132 @@ TEST(OsdTest, TestElementRssi)
     displayPortTestBufferSubstring(8, 1, "%c50", SYM_RSSI);
 }
 
+/*
+ * Tests the instantaneous battery current OSD element.
+ */
+TEST(OsdTest, TestElementAmperage)
+{
+    // given
+    osdConfigMutable()->item_pos[OSD_CURRENT_DRAW] = OSD_POS(1, 12) | VISIBLE_FLAG;
+
+    // when
+    simulationBatteryAmperage = 0;
+    displayClearScreen(&testDisplayPort);
+    osdRefresh(simulationTime);
+
+    // then
+    displayPortTestBufferSubstring(1, 12, "  0.00%c", SYM_AMP);
+
+    // when
+    simulationBatteryAmperage = 2156;
+    displayClearScreen(&testDisplayPort);
+    osdRefresh(simulationTime);
+
+    // then
+    displayPortTestBufferSubstring(1, 12, " 21.56%c", SYM_AMP);
+
+    // when
+    simulationBatteryAmperage = 12345;
+    displayClearScreen(&testDisplayPort);
+    osdRefresh(simulationTime);
+
+    // then
+    displayPortTestBufferSubstring(1, 12, "123.45%c", SYM_AMP);
+}
+
+/*
+ * Tests the battery capacity drawn OSD element.
+ */
+TEST(OsdTest, TestElementMahDrawn)
+{
+    // given
+    osdConfigMutable()->item_pos[OSD_MAH_DRAWN] = OSD_POS(1, 11) | VISIBLE_FLAG;
+
+    // when
+    simulationMahDrawn = 0;
+    displayClearScreen(&testDisplayPort);
+    osdRefresh(simulationTime);
+
+    // then
+    displayPortTestBufferSubstring(1, 11, "   0%c", SYM_MAH);
+
+    // when
+    simulationMahDrawn = 4;
+    displayClearScreen(&testDisplayPort);
+    osdRefresh(simulationTime);
+
+    // then
+    displayPortTestBufferSubstring(1, 11, "   4%c", SYM_MAH);
+
+    // when
+    simulationMahDrawn = 15;
+    displayClearScreen(&testDisplayPort);
+    osdRefresh(simulationTime);
+
+    // then
+    displayPortTestBufferSubstring(1, 11, "  15%c", SYM_MAH);
+
+    // when
+    simulationMahDrawn = 246;
+    displayClearScreen(&testDisplayPort);
+    osdRefresh(simulationTime);
+
+    // then
+    displayPortTestBufferSubstring(1, 11, " 246%c", SYM_MAH);
+
+    // when
+    simulationMahDrawn = 1042;
+    displayClearScreen(&testDisplayPort);
+    osdRefresh(simulationTime);
+
+    // then
+    displayPortTestBufferSubstring(1, 11, "1042%c", SYM_MAH);
+}
+
+/*
+ * Tests the altitude OSD element.
+ */
+TEST(OsdTest, TestElementAltitude)
+{
+    // given
+    osdConfigMutable()->item_pos[OSD_ALTITUDE] = OSD_POS(23, 7) | VISIBLE_FLAG;
+
+    // and
+    osdConfigMutable()->units = OSD_UNIT_METRIC;
+
+    // when
+    simulationAltitude = 0;
+    displayClearScreen(&testDisplayPort);
+    osdRefresh(simulationTime);
+
+    // then
+    displayPortTestBufferSubstring(23, 7, "   0.0%c", SYM_M);
+
+    // when
+    simulationAltitude = 247;
+    displayClearScreen(&testDisplayPort);
+    osdRefresh(simulationTime);
+
+    // then
+    displayPortTestBufferSubstring(23, 7, "   2.4%c", SYM_M);
+
+    // when
+    simulationAltitude = 4247;
+    displayClearScreen(&testDisplayPort);
+    osdRefresh(simulationTime);
+
+    // then
+    displayPortTestBufferSubstring(23, 7, "  42.4%c", SYM_M);
+
+    // when
+    simulationAltitude = -247;
+    displayClearScreen(&testDisplayPort);
+    osdRefresh(simulationTime);
+
+    // then
+    displayPortTestBufferSubstring(23, 7, "  -2.4%c", SYM_M);
+}
+
 /*
  * Tests the time string formatting function with a series of precision settings and time values.
  */
@@ -615,11 +745,11 @@ extern "C" {
     }
 
     int32_t getAmperage() {
-        return 0;
+        return simulationBatteryAmperage;
     }
 
     int32_t getMAhDrawn() {
-        return 0;
+        return simulationMahDrawn;
     }
 
     int32_t getEstimatedAltitude() {