diff --git a/src/main/io/osd.c b/src/main/io/osd.c index 45c859b909..571b91cb27 100644 --- a/src/main/io/osd.c +++ b/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. -// 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. If you change the -// display order you *must* update the osd_stats_e enumeration to match. Additionally the -// 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. +// 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. static void osdShowStats(uint16_t endBatteryVoltage) { @@ -1791,15 +1790,28 @@ static void osdShowStats(uint16_t endBatteryVoltage) osdDisplayStatisticLabel(top++, osdTimerSourceNames[OSD_TIMER_SRC(osdConfig()->timers[OSD_TIMER_2])], buff); } -#ifdef USE_GPS - if (osdStatGetState(OSD_STAT_MAX_SPEED) && featureIsEnabled(FEATURE_GPS)) { - itoa(stats.max_speed, buff, 10); - osdDisplayStatisticLabel(top++, "MAX SPEED", buff); + if (osdStatGetState(OSD_STAT_MAX_ALTITUDE)) { + osdFormatAltitudeString(buff, stats.max_altitude); + osdDisplayStatisticLabel(top++, "MAX ALTITUDE", buff); } - if (osdStatGetState(OSD_STAT_MAX_DISTANCE) && featureIsEnabled(FEATURE_GPS)) { - tfp_sprintf(buff, "%d%c", osdGetMetersToSelectedUnit(stats.max_distance), osdGetMetersToSelectedUnitSymbol()); - osdDisplayStatisticLabel(top++, "MAX DISTANCE", buff); +#ifdef USE_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)) { + 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 @@ -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) diff --git a/src/main/io/osd.h b/src/main/io/osd.h index 68a4e4cfc9..22794bdd98 100644 --- a/src/main/io/osd.h +++ b/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 -// 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 +// If the stats enumeration is 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, diff --git a/src/test/unit/osd_unittest.cc b/src/test/unit/osd_unittest.cc index 23948be217..a9dd93da5a 100644 --- a/src/test/unit/osd_unittest.cc +++ b/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); } /*