Merge pull request #7161 from TonyBlit/gps_total_distance_stats_pos

Reordered Max Altitude and Total Distance in stats screen

src/main/io/osd.c

@@ -1754,12 +1754,11 @@ static bool isSomeStatEnabled(void)
 }
 
 // *** IMPORTANT ***
-// The order of the OSD stats as displayed on-screen must match the osd_stats_e enumeration.
+// The stats display order was previously required to match the enumeration definition so it matched
-// This is because the fields are presented in the configurator in the order of the enumeration
+// the order shown in the configurator. However, to allow reordering this screen without breaking the
-// and we want the configuration order to match the on-screen display order.  If you change the
+// compatibility, this requirement has been relaxed to a best effort approach. Reordering the elements
-// display order you *must* update the osd_stats_e enumeration to match. Additionally the
+// on the stats screen will have to be more beneficial than the hassle of not matching exactly to the
-// changes to the stats display order *must* be implemented in the configurator otherwise the
+// configurator list.
-// stats selections will not be populated correctly and the settings will become corrupted.
 
 static void osdShowStats(uint16_t endBatteryVoltage)
 {
@@ -1791,16 +1790,29 @@ static void osdShowStats(uint16_t endBatteryVoltage)
         osdDisplayStatisticLabel(top++, osdTimerSourceNames[OSD_TIMER_SRC(osdConfig()->timers[OSD_TIMER_2])], buff);
     }
 
+    if (osdStatGetState(OSD_STAT_MAX_ALTITUDE)) {
+        osdFormatAltitudeString(buff, stats.max_altitude);
+        osdDisplayStatisticLabel(top++, "MAX ALTITUDE", buff);
+    }
+
 #ifdef USE_GPS
-    if (osdStatGetState(OSD_STAT_MAX_SPEED) && featureIsEnabled(FEATURE_GPS)) {
+    if (featureIsEnabled(FEATURE_GPS)) {
+        if (osdStatGetState(OSD_STAT_MAX_SPEED)) {
             itoa(stats.max_speed, buff, 10);    
             osdDisplayStatisticLabel(top++, "MAX SPEED", buff);
         }
 
-    if (osdStatGetState(OSD_STAT_MAX_DISTANCE) && featureIsEnabled(FEATURE_GPS)) {
+        if (osdStatGetState(OSD_STAT_MAX_DISTANCE)) {
             tfp_sprintf(buff, "%d%c", osdGetMetersToSelectedUnit(stats.max_distance), osdGetMetersToSelectedUnitSymbol());
             osdDisplayStatisticLabel(top++, "MAX DISTANCE", buff);
         }
+
+        if (osdStatGetState(OSD_STAT_FLIGHT_DISTANCE)) {
+            const uint32_t distanceFlown = GPS_distanceFlownInCm / 100;
+            tfp_sprintf(buff, "%d%c", osdGetMetersToSelectedUnit(distanceFlown), osdGetMetersToSelectedUnitSymbol());
+            osdDisplayStatisticLabel(top++, "FLIGHT DISTANCE", buff);
+        }
+    }
 #endif
 
     if (osdStatGetState(OSD_STAT_MIN_BATTERY)) {
@@ -1837,11 +1849,6 @@ static void osdShowStats(uint16_t endBatteryVoltage)
         }
     }
 
-    if (osdStatGetState(OSD_STAT_MAX_ALTITUDE)) {
-        osdFormatAltitudeString(buff, stats.max_altitude);
-        osdDisplayStatisticLabel(top++, "MAX ALTITUDE", buff);
-    }
-
 #ifdef USE_BLACKBOX
     if (osdStatGetState(OSD_STAT_BLACKBOX) && blackboxConfig()->device && blackboxConfig()->device != BLACKBOX_DEVICE_SERIAL) {
         osdGetBlackboxStatusString(buff);
@@ -1880,14 +1887,6 @@ static void osdShowStats(uint16_t endBatteryVoltage)
     }
 #endif
 
-#ifdef USE_GPS
-    if (osdStatGetState(OSD_STAT_FLIGHT_DISTANCE) && featureIsEnabled(FEATURE_GPS)) {
-        const uint32_t distanceFlown = GPS_distanceFlownInCm / 100;
-        tfp_sprintf(buff, "%d%c", osdGetMetersToSelectedUnit(distanceFlown), osdGetMetersToSelectedUnitSymbol());
-        osdDisplayStatisticLabel(top++, "FLIGHT DISTANCE", buff);
-    }
-#endif
-
 #if defined(USE_GYRO_DATA_ANALYSE)
     if (osdStatGetState(OSD_STAT_MAX_FFT) && featureIsEnabled(FEATURE_DYNAMIC_FILTER)) {
         int value = getMaxFFT();
@@ -1899,7 +1898,6 @@ static void osdShowStats(uint16_t endBatteryVoltage)
         }
     }
 #endif
-
 }
 
 static void osdShowArmed(void)

src/main/io/osd.h

@@ -125,14 +125,14 @@ typedef enum {
 } osd_items_e;
 
 // *** IMPORTANT ***
-// The order of the OSD stats enumeration *must* match the order they're displayed on-screen
+// If the stats enumeration is reordered then the PR version must be incremented. Otherwise there
-// This is because the fields are presented in the configurator in the order of the enumeration
-// and we want the configuration order to match the on-screen display order.
-// Changes to the stats display order *must* be implemented in the configurator otherwise the
-// stats selections will not be populated correctly and the settings will become corrupted.
-//
-// Also - if the stats are reordered then the PR version must be incremented. Otherwise there
 // is no indication that the stored config must be reset and the bitmapped values will be incorrect.
+//
+// The stats display order was previously required to match the enumeration definition so it matched
+// the order shown in the configurator. However, to allow reordering this screen without breaking the
+// compatibility, this requirement has been relaxed to a best effort approach. Reordering the elements
+// on the stats screen will have to be more beneficial than the hassle of not matching exactly to the
+// configurator list.
 typedef enum {
     OSD_STAT_RTC_DATE_TIME,
     OSD_STAT_TIMER_1,

src/test/unit/osd_unittest.cc

@@ -394,13 +394,13 @@ TEST(OsdTest, TestStatsImperial)
     displayPortTestBufferSubstring(2, row++, "2017-11-19 10:12:");
     displayPortTestBufferSubstring(2, row++, "TOTAL ARM         : 00:05.00");
     displayPortTestBufferSubstring(2, row++, "LAST ARM          : 00:03");
+    displayPortTestBufferSubstring(2, row++, "MAX ALTITUDE      :    6.5%c", SYM_FT);
     displayPortTestBufferSubstring(2, row++, "MAX SPEED         : 17");
     displayPortTestBufferSubstring(2, row++, "MAX DISTANCE      : 328%c", SYM_FT);
+    displayPortTestBufferSubstring(2, row++, "FLIGHT DISTANCE   : 656%c", SYM_FT);
     displayPortTestBufferSubstring(2, row++, "MIN BATTERY       : 14.70%c", SYM_VOLT);
     displayPortTestBufferSubstring(2, row++, "END BATTERY       : 15.20%c", SYM_VOLT);
     displayPortTestBufferSubstring(2, row++, "MIN RSSI          : 25%%");
-    displayPortTestBufferSubstring(2, row++, "MAX ALTITUDE      :    6.5%c", SYM_FT);
-    displayPortTestBufferSubstring(2, row++, "FLIGHT DISTANCE   : 656%c", SYM_FT);
 }
 
 /*
@@ -447,13 +447,13 @@ TEST(OsdTest, TestStatsMetric)
     displayPortTestBufferSubstring(2, row++, "2017-11-19 10:12:");
     displayPortTestBufferSubstring(2, row++, "TOTAL ARM         : 00:07.50");
     displayPortTestBufferSubstring(2, row++, "LAST ARM          : 00:02");
+    displayPortTestBufferSubstring(2, row++, "MAX ALTITUDE      :    2.0%c", SYM_M);
     displayPortTestBufferSubstring(2, row++, "MAX SPEED         : 28");
     displayPortTestBufferSubstring(2, row++, "MAX DISTANCE      : 100%c", SYM_M);
+    displayPortTestBufferSubstring(2, row++, "FLIGHT DISTANCE   : 100%c", SYM_M);
     displayPortTestBufferSubstring(2, row++, "MIN BATTERY       : 14.70%c", SYM_VOLT);
     displayPortTestBufferSubstring(2, row++, "END BATTERY       : 15.20%c", SYM_VOLT);
     displayPortTestBufferSubstring(2, row++, "MIN RSSI          : 25%%");
-    displayPortTestBufferSubstring(2, row++, "MAX ALTITUDE      :    2.0%c", SYM_M);
-    displayPortTestBufferSubstring(2, row++, "FLIGHT DISTANCE   : 100%c", SYM_M);
 }
 
 /*