From dd509a8ecf24c964aad1d7d72ef1f01217bdb993 Mon Sep 17 00:00:00 2001 From: Andrew Yong Date: Tue, 14 Jul 2026 09:04:24 +0800 Subject: [PATCH] Compute GeoCoord's UTM/MGRS/OSGR/OLC lazily instead of eagerly (#10996) GeoCoord's constructor unconditionally computed all five coordinate representations (DMS, UTM, MGRS, OSGR, OLC) via setCoords(), even though most callers only ever read one. The UTM conversion alone pulls in a chain of libm trig functions (atan, __kernel_tan, __ieee754_acos, __ieee754_pow, __kernel_rem_pio2/__ieee754_rem_pio2) that then have to be linked in regardless of whether anything is ever displayed. The only screen consumer (UIRenderer.cpp's GPS coordinate display) already dispatches on a single configured format and reads exactly one representation per call - never more than one. Compute DMS eagerly (cheap, most commonly needed) and defer UTM/MGRS/OSGR/OLC to first access via their own getters, tracked with per-representation mutable dirty flags, so a caller that never touches a given representation never pulls in its conversion code or the trig functions it needs. On stm32wl, GeoCoord's heavy constructor is reached only through NMEAWPL.cpp's NMEA/CalTopo serial export (GPS-gated, so wio-e5 only), which only ever reads the DMS getters - so this recovers 8,288 bytes flash there (of the 10,172-byte ceiling if the whole feature were cut) with the feature fully intact and zero behavior change on any platform. Updated test_geocoord_extreme_coords_no_oob (the existing regression test for a historical out-of-bounds crash in the UTM/MGRS conversion on extreme lat/lon) to explicitly call each representation's getter, since it previously relied on the constructor eagerly triggering all four conversions - which this change intentionally defers. Verified: full native test suite passes (586/586 test cases, including this one under ASan), and rak4631 (nRF52, screen-equipped, where the UTM/MGRS/OSGR/OLC getters are actually reachable) builds successfully. Assisted-by: Claude Sonnet 5 Signed-off-by: Andrew Yong Co-authored-by: Ben Meadors --- src/gps/GeoCoord.cpp | 43 +++++++- src/gps/GeoCoord.h | 116 +++++++++++++++++---- test/test_position_precision/test_main.cpp | 11 +- 3 files changed, 140 insertions(+), 30 deletions(-) diff --git a/src/gps/GeoCoord.cpp b/src/gps/GeoCoord.cpp index f225ab61c..8324bc3c0 100644 --- a/src/gps/GeoCoord.cpp +++ b/src/gps/GeoCoord.cpp @@ -36,19 +36,52 @@ GeoCoord::GeoCoord(double lat, double lon, int32_t alt) : _altitude(alt) GeoCoord::setCoords(); } -// Initialize all the coordinate systems +// Initialize DMS eagerly (cheap, most commonly used); UTM/MGRS/OSGR/OLC are computed lazily on +// first access via their getters (see ensure*() below), since most callers never touch them. void GeoCoord::setCoords() { double lat = _latitude * 1e-7; double lon = _longitude * 1e-7; GeoCoord::latLongToDMS(lat, lon, _dms); - GeoCoord::latLongToUTM(lat, lon, _utm); - GeoCoord::latLongToMGRS(lat, lon, _mgrs); - GeoCoord::latLongToOSGR(lat, lon, _osgr); - GeoCoord::latLongToOLC(lat, lon, _olc); + _utmValid = false; + _mgrsValid = false; + _osgrValid = false; + _olcValid = false; _dirty = false; } +void GeoCoord::ensureUTM() const +{ + if (!_utmValid) { + GeoCoord::latLongToUTM(_latitude * 1e-7, _longitude * 1e-7, _utm); + _utmValid = true; + } +} + +void GeoCoord::ensureMGRS() const +{ + if (!_mgrsValid) { + GeoCoord::latLongToMGRS(_latitude * 1e-7, _longitude * 1e-7, _mgrs); + _mgrsValid = true; + } +} + +void GeoCoord::ensureOSGR() const +{ + if (!_osgrValid) { + GeoCoord::latLongToOSGR(_latitude * 1e-7, _longitude * 1e-7, _osgr); + _osgrValid = true; + } +} + +void GeoCoord::ensureOLC() const +{ + if (!_olcValid) { + GeoCoord::latLongToOLC(_latitude * 1e-7, _longitude * 1e-7, _olc); + _olcValid = true; + } +} + void GeoCoord::updateCoords(int32_t lat, int32_t lon, int32_t alt) { // If marked dirty or new coordinates diff --git a/src/gps/GeoCoord.h b/src/gps/GeoCoord.h index 658c177b3..3bb6606f2 100644 --- a/src/gps/GeoCoord.h +++ b/src/gps/GeoCoord.h @@ -65,14 +65,24 @@ class GeoCoord int32_t _altitude = 0; DMS _dms = {}; - UTM _utm = {}; - MGRS _mgrs = {}; - OSGR _osgr = {}; - OLC _olc = {}; + // Computed lazily on first access via ensure*() below; mutable so const getters can populate + // them on demand. + mutable UTM _utm = {}; + mutable MGRS _mgrs = {}; + mutable OSGR _osgr = {}; + mutable OLC _olc = {}; + mutable bool _utmValid = false; + mutable bool _mgrsValid = false; + mutable bool _osgrValid = false; + mutable bool _olcValid = false; bool _dirty = true; void setCoords(); + void ensureUTM() const; + void ensureMGRS() const; + void ensureOSGR() const; + void ensureOLC() const; public: GeoCoord(); @@ -123,26 +133,86 @@ class GeoCoord uint32_t getDMSLonSec() const { return _dms.lonSec; } char getDMSLonCP() const { return _dms.lonCP; } - // UTM getters - uint8_t getUTMZone() const { return _utm.zone; } - char getUTMBand() const { return _utm.band; } - uint32_t getUTMEasting() const { return _utm.easting; } - uint32_t getUTMNorthing() const { return _utm.northing; } + // UTM getters (computed on first access - see ensureUTM()) + uint8_t getUTMZone() const + { + ensureUTM(); + return _utm.zone; + } + char getUTMBand() const + { + ensureUTM(); + return _utm.band; + } + uint32_t getUTMEasting() const + { + ensureUTM(); + return _utm.easting; + } + uint32_t getUTMNorthing() const + { + ensureUTM(); + return _utm.northing; + } - // MGRS getters - uint8_t getMGRSZone() const { return _mgrs.zone; } - char getMGRSBand() const { return _mgrs.band; } - char getMGRSEast100k() const { return _mgrs.east100k; } - char getMGRSNorth100k() const { return _mgrs.north100k; } - uint32_t getMGRSEasting() const { return _mgrs.easting; } - uint32_t getMGRSNorthing() const { return _mgrs.northing; } + // MGRS getters (computed on first access - see ensureMGRS()) + uint8_t getMGRSZone() const + { + ensureMGRS(); + return _mgrs.zone; + } + char getMGRSBand() const + { + ensureMGRS(); + return _mgrs.band; + } + char getMGRSEast100k() const + { + ensureMGRS(); + return _mgrs.east100k; + } + char getMGRSNorth100k() const + { + ensureMGRS(); + return _mgrs.north100k; + } + uint32_t getMGRSEasting() const + { + ensureMGRS(); + return _mgrs.easting; + } + uint32_t getMGRSNorthing() const + { + ensureMGRS(); + return _mgrs.northing; + } - // OSGR getters - char getOSGRE100k() const { return _osgr.e100k; } - char getOSGRN100k() const { return _osgr.n100k; } - uint32_t getOSGREasting() const { return _osgr.easting; } - uint32_t getOSGRNorthing() const { return _osgr.northing; } + // OSGR getters (computed on first access - see ensureOSGR()) + char getOSGRE100k() const + { + ensureOSGR(); + return _osgr.e100k; + } + char getOSGRN100k() const + { + ensureOSGR(); + return _osgr.n100k; + } + uint32_t getOSGREasting() const + { + ensureOSGR(); + return _osgr.easting; + } + uint32_t getOSGRNorthing() const + { + ensureOSGR(); + return _osgr.northing; + } - // OLC getter - void getOLCCode(char *code) { strncpy(code, _olc.code, OLC_CODE_LEN + 1); } // +1 for null termination + // OLC getter (computed on first access - see ensureOLC()) + void getOLCCode(char *code) + { + ensureOLC(); + strncpy(code, _olc.code, OLC_CODE_LEN + 1); // +1 for null termination + } }; \ No newline at end of file diff --git a/test/test_position_precision/test_main.cpp b/test/test_position_precision/test_main.cpp index fbb937952..5f5569752 100644 --- a/test/test_position_precision/test_main.cpp +++ b/test/test_position_precision/test_main.cpp @@ -214,7 +214,8 @@ static void test_cryptoKeyIsPublic_invalidKeyIsNotPublic() // Pre-fix, latitude_i = INT32_MAX made latLongToUTM read latBands[36] on a 21-char string // (stack-buffer-overflow at GeoCoord.cpp:128, an AddressSanitizer abort); extreme longitude produced // a negative UTM zone feeding the MGRS letter tables. The fix clamps the zone/band/col/row indices. -// This exercises the fix under the coverage env's ASan. +// This exercises the fix under the coverage env's ASan. Each representation is computed lazily, +// so its getter must be called explicitly here to exercise its conversion path. static void test_geocoord_extreme_coords_no_oob() { const int32_t vals[] = {INT32_MIN, INT32_MAX, INT32_MIN + 1, INT32_MAX - 1, 0, 1, -1, 900000000, -900000000, // +/-90 deg @@ -223,7 +224,13 @@ static void test_geocoord_extreme_coords_no_oob() const size_t n = sizeof(vals) / sizeof(vals[0]); for (size_t i = 0; i < n; i++) for (size_t j = 0; j < n; j++) { - GeoCoord g(vals[i], vals[j], 0); // ctor -> setCoords() -> UTM/MGRS/OSGR/OLC + GeoCoord g(vals[i], vals[j], 0); // ctor -> setCoords() -> DMS only + // Force UTM/MGRS/OSGR/OLC computation (each lazy on first getter access). + g.getUTMZone(); + g.getMGRSZone(); + g.getOSGRE100k(); + char olcCode[OLC_CODE_LEN + 1]; + g.getOLCCode(olcCode); // Surviving every extreme pair (no ASan fault) means the index clamps hold. TEST_ASSERT_EQUAL_INT32(vals[i], g.getLatitude()); }