From a0604dd1a5286fef7094c2d016faade12e7583b4 Mon Sep 17 00:00:00 2001 From: Tony Cabello Date: Sat, 1 Dec 2018 08:54:12 +0100 Subject: [PATCH 1/2] Reordered Max Altitude and Total Distance in stats screen --- src/main/io/osd.c | 41 +++++++++++++++++------------------ src/test/unit/osd_unittest.cc | 8 +++---- 2 files changed, 24 insertions(+), 25 deletions(-) diff --git a/src/main/io/osd.c b/src/main/io/osd.c index 357b826a5a..2eee3fdc0c 100644 --- a/src/main/io/osd.c +++ b/src/main/io/osd.c @@ -1787,15 +1787,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 @@ -1833,11 +1846,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); @@ -1876,14 +1884,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(); @@ -1895,7 +1895,6 @@ static void osdShowStats(uint16_t endBatteryVoltage) } } #endif - } static void osdShowArmed(void) 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); } /* From 5c322405222174a6d12dd4c75f5f1c5a8282552a Mon Sep 17 00:00:00 2001 From: Tony Cabello Date: Tue, 4 Dec 2018 16:36:41 +0100 Subject: [PATCH 2/2] Updated comment regarding stats display order --- src/main/io/osd.c | 11 +++++------ src/main/io/osd.h | 14 +++++++------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/main/io/osd.c b/src/main/io/osd.c index 2eee3fdc0c..5e74a2c986 100644 --- a/src/main/io/osd.c +++ b/src/main/io/osd.c @@ -1750,12 +1750,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) { diff --git a/src/main/io/osd.h b/src/main/io/osd.h index 84a282d81a..bfc30cf18b 100644 --- a/src/main/io/osd.h +++ b/src/main/io/osd.h @@ -129,14 +129,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,