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 <noreply@anthropic.com> Signed-off-by: Andrew Yong <me@ndoo.sg> Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
This commit is contained in:
co-authored by
GitHub
Ben Meadors
parent
269b974e96
commit
dd509a8ecf
+38
-5
@@ -36,19 +36,52 @@ GeoCoord::GeoCoord(double lat, double lon, int32_t alt) : _altitude(alt)
|
|||||||
GeoCoord::setCoords();
|
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()
|
void GeoCoord::setCoords()
|
||||||
{
|
{
|
||||||
double lat = _latitude * 1e-7;
|
double lat = _latitude * 1e-7;
|
||||||
double lon = _longitude * 1e-7;
|
double lon = _longitude * 1e-7;
|
||||||
GeoCoord::latLongToDMS(lat, lon, _dms);
|
GeoCoord::latLongToDMS(lat, lon, _dms);
|
||||||
GeoCoord::latLongToUTM(lat, lon, _utm);
|
_utmValid = false;
|
||||||
GeoCoord::latLongToMGRS(lat, lon, _mgrs);
|
_mgrsValid = false;
|
||||||
GeoCoord::latLongToOSGR(lat, lon, _osgr);
|
_osgrValid = false;
|
||||||
GeoCoord::latLongToOLC(lat, lon, _olc);
|
_olcValid = false;
|
||||||
_dirty = 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)
|
void GeoCoord::updateCoords(int32_t lat, int32_t lon, int32_t alt)
|
||||||
{
|
{
|
||||||
// If marked dirty or new coordinates
|
// If marked dirty or new coordinates
|
||||||
|
|||||||
+93
-23
@@ -65,14 +65,24 @@ class GeoCoord
|
|||||||
int32_t _altitude = 0;
|
int32_t _altitude = 0;
|
||||||
|
|
||||||
DMS _dms = {};
|
DMS _dms = {};
|
||||||
UTM _utm = {};
|
// Computed lazily on first access via ensure*() below; mutable so const getters can populate
|
||||||
MGRS _mgrs = {};
|
// them on demand.
|
||||||
OSGR _osgr = {};
|
mutable UTM _utm = {};
|
||||||
OLC _olc = {};
|
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;
|
bool _dirty = true;
|
||||||
|
|
||||||
void setCoords();
|
void setCoords();
|
||||||
|
void ensureUTM() const;
|
||||||
|
void ensureMGRS() const;
|
||||||
|
void ensureOSGR() const;
|
||||||
|
void ensureOLC() const;
|
||||||
|
|
||||||
public:
|
public:
|
||||||
GeoCoord();
|
GeoCoord();
|
||||||
@@ -123,26 +133,86 @@ class GeoCoord
|
|||||||
uint32_t getDMSLonSec() const { return _dms.lonSec; }
|
uint32_t getDMSLonSec() const { return _dms.lonSec; }
|
||||||
char getDMSLonCP() const { return _dms.lonCP; }
|
char getDMSLonCP() const { return _dms.lonCP; }
|
||||||
|
|
||||||
// UTM getters
|
// UTM getters (computed on first access - see ensureUTM())
|
||||||
uint8_t getUTMZone() const { return _utm.zone; }
|
uint8_t getUTMZone() const
|
||||||
char getUTMBand() const { return _utm.band; }
|
{
|
||||||
uint32_t getUTMEasting() const { return _utm.easting; }
|
ensureUTM();
|
||||||
uint32_t getUTMNorthing() const { return _utm.northing; }
|
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
|
// MGRS getters (computed on first access - see ensureMGRS())
|
||||||
uint8_t getMGRSZone() const { return _mgrs.zone; }
|
uint8_t getMGRSZone() const
|
||||||
char getMGRSBand() const { return _mgrs.band; }
|
{
|
||||||
char getMGRSEast100k() const { return _mgrs.east100k; }
|
ensureMGRS();
|
||||||
char getMGRSNorth100k() const { return _mgrs.north100k; }
|
return _mgrs.zone;
|
||||||
uint32_t getMGRSEasting() const { return _mgrs.easting; }
|
}
|
||||||
uint32_t getMGRSNorthing() const { return _mgrs.northing; }
|
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
|
// OSGR getters (computed on first access - see ensureOSGR())
|
||||||
char getOSGRE100k() const { return _osgr.e100k; }
|
char getOSGRE100k() const
|
||||||
char getOSGRN100k() const { return _osgr.n100k; }
|
{
|
||||||
uint32_t getOSGREasting() const { return _osgr.easting; }
|
ensureOSGR();
|
||||||
uint32_t getOSGRNorthing() const { return _osgr.northing; }
|
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
|
// OLC getter (computed on first access - see ensureOLC())
|
||||||
void getOLCCode(char *code) { strncpy(code, _olc.code, OLC_CODE_LEN + 1); } // +1 for null termination
|
void getOLCCode(char *code)
|
||||||
|
{
|
||||||
|
ensureOLC();
|
||||||
|
strncpy(code, _olc.code, OLC_CODE_LEN + 1); // +1 for null termination
|
||||||
|
}
|
||||||
};
|
};
|
||||||
@@ -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
|
// 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
|
// (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.
|
// 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()
|
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
|
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]);
|
const size_t n = sizeof(vals) / sizeof(vals[0]);
|
||||||
for (size_t i = 0; i < n; i++)
|
for (size_t i = 0; i < n; i++)
|
||||||
for (size_t j = 0; j < n; j++) {
|
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.
|
// Surviving every extreme pair (no ASan fault) means the index clamps hold.
|
||||||
TEST_ASSERT_EQUAL_INT32(vals[i], g.getLatitude());
|
TEST_ASSERT_EQUAL_INT32(vals[i], g.getLatitude());
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user