From 7d745ec40802d8b0f1fcc18b0de4cd9c4f23816a Mon Sep 17 00:00:00 2001 From: Tom Fifield Date: Mon, 1 Sep 2025 08:34:54 +1000 Subject: [PATCH 1/5] Hold for >20s after GPS lock GPS chips are designed to stay locked for a while to download some data and save it. This data is important for speeding up future locks, and making them higher quality. Our present configuration could make every lock perform similar to first lock. This patch sets a hold of between 20s and 10% of the lock search time after lock is acquired. This should allow the GPS to finish its work before we turn it off. Fixes https://github.com/meshtastic/firmware/issues/7466 --- src/gps/GPS.cpp | 13 ++++++++++--- src/gps/GPS.h | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/gps/GPS.cpp b/src/gps/GPS.cpp index ae74f0fe2..ef37df196 100644 --- a/src/gps/GPS.cpp +++ b/src/gps/GPS.cpp @@ -1126,6 +1126,10 @@ int32_t GPS::runOnce() LOG_DEBUG("hasValidLocation RISING EDGE"); hasValidLocation = true; shouldPublish = true; + lastFixStartMsec = millis(); + // Calculate hold duration: 10% of search time or 20s, whichever is smaller + uint32_t tenPercent = scheduling.elapsedSearchMs() / 10; + fixHoldEnds = lastFixStartMsec + ((tenPercent < 20000) ? tenPercent : 20000); } bool tooLong = scheduling.searchedTooLong(); @@ -1135,7 +1139,7 @@ int32_t GPS::runOnce() // Once we get a location we no longer desperately want an update if ((gotLoc && gotTime) || tooLong) { - if (tooLong) { + if (tooLong && !gotLoc) { // we didn't get a location during this ack window, therefore declare loss of lock if (hasValidLocation) { LOG_DEBUG("hasValidLocation FALLING EDGE"); @@ -1143,8 +1147,11 @@ int32_t GPS::runOnce() p = meshtastic_Position_init_default; hasValidLocation = false; } - - down(); + if (millis() > fixHoldEnds) { + down(); + } else { + LOG_DEBUG("Holding for GPS data download: %d ms", fixHoldEnds - millis()); + } shouldPublish = true; // publish our update for this just finished acquisition window } diff --git a/src/gps/GPS.h b/src/gps/GPS.h index 9be57017f..3dcb17611 100644 --- a/src/gps/GPS.h +++ b/src/gps/GPS.h @@ -159,7 +159,7 @@ class GPS : private concurrency::OSThread uint8_t fixType = 0; // fix type from GPGSA #endif - uint32_t lastWakeStartMsec = 0, lastSleepStartMsec = 0, lastFixStartMsec = 0; + uint32_t lastFixStartMsec = 0, fixHoldEnds = 0; uint32_t rx_gpio = 0; uint32_t tx_gpio = 0; From 94ce40ce581c0a4afce116df3c3c7c33400ff65f Mon Sep 17 00:00:00 2001 From: Tom Fifield Date: Mon, 1 Sep 2025 10:31:41 +1000 Subject: [PATCH 2/5] Remove T1000E-specific GPS holds The new code does the same thing, for all devices. --- src/gps/GPS.cpp | 53 ++------------------- variants/nrf52840/tracker-t1000-e/variant.h | 3 +- variants/nrf52840/wio-t1000-s/variant.h | 3 +- 3 files changed, 6 insertions(+), 53 deletions(-) diff --git a/src/gps/GPS.cpp b/src/gps/GPS.cpp index ef37df196..a8dc7ea5d 100644 --- a/src/gps/GPS.cpp +++ b/src/gps/GPS.cpp @@ -843,9 +843,6 @@ void GPS::setPowerState(GPSPowerState newState, uint32_t sleepTime) setPowerPMU(true); // Power (PMU): on writePinStandby(false); // Standby (pin): awake (not standby) setPowerUBLOX(true); // Standby (UBLOX): awake -#ifdef GNSS_AIROHA - lastFixStartMsec = 0; -#endif break; case GPS_SOFTSLEEP: @@ -863,9 +860,7 @@ void GPS::setPowerState(GPSPowerState newState, uint32_t sleepTime) writePinStandby(true); // Standby (pin): asleep (not awake) setPowerUBLOX(false, sleepTime); // Standby (UBLOX): asleep, timed #ifdef GNSS_AIROHA - if (config.position.gps_update_interval * 1000 >= GPS_FIX_HOLD_TIME * 2) { - digitalWrite(PIN_GPS_EN, LOW); - } + digitalWrite(PIN_GPS_EN, LOW); #endif break; @@ -877,9 +872,7 @@ void GPS::setPowerState(GPSPowerState newState, uint32_t sleepTime) writePinStandby(true); // Standby (pin): asleep setPowerUBLOX(false, 0); // Standby (UBLOX): asleep, indefinitely #ifdef GNSS_AIROHA - if (config.position.gps_update_interval * 1000 >= GPS_FIX_HOLD_TIME * 2) { - digitalWrite(PIN_GPS_EN, LOW); - } + digitalWrite(PIN_GPS_EN, LOW); #endif break; } @@ -1126,10 +1119,9 @@ int32_t GPS::runOnce() LOG_DEBUG("hasValidLocation RISING EDGE"); hasValidLocation = true; shouldPublish = true; + // Hold for 20secs after getting a lock to download ephemeris etc lastFixStartMsec = millis(); - // Calculate hold duration: 10% of search time or 20s, whichever is smaller - uint32_t tenPercent = scheduling.elapsedSearchMs() / 10; - fixHoldEnds = lastFixStartMsec + ((tenPercent < 20000) ? tenPercent : 20000); + fixHoldEnds = lastFixStartMsec + 20000; } bool tooLong = scheduling.searchedTooLong(); @@ -1515,24 +1507,6 @@ static int32_t toDegInt(RawDegrees d) */ bool GPS::lookForTime() { - -#ifdef GNSS_AIROHA - uint8_t fix = reader.fixQuality(); - if (fix >= 1 && fix <= 5) { - if (lastFixStartMsec > 0) { - if (Throttle::isWithinTimespanMs(lastFixStartMsec, GPS_FIX_HOLD_TIME)) { - return false; - } else { - clearBuffer(); - } - } else { - lastFixStartMsec = millis(); - return false; - } - } else { - return false; - } -#endif auto ti = reader.time; auto d = reader.date; if (ti.isValid() && d.isValid()) { // Note: we don't check for updated, because we'll only be called if needed @@ -1570,25 +1544,6 @@ The Unix epoch (or Unix time or POSIX time or Unix timestamp) is the number of s */ bool GPS::lookForLocation() { -#ifdef GNSS_AIROHA - if ((config.position.gps_update_interval * 1000) >= (GPS_FIX_HOLD_TIME * 2)) { - uint8_t fix = reader.fixQuality(); - if (fix >= 1 && fix <= 5) { - if (lastFixStartMsec > 0) { - if (Throttle::isWithinTimespanMs(lastFixStartMsec, GPS_FIX_HOLD_TIME)) { - return false; - } else { - clearBuffer(); - } - } else { - lastFixStartMsec = millis(); - return false; - } - } else { - return false; - } - } -#endif // By default, TinyGPS++ does not parse GPGSA lines, which give us // the 2D/3D fixType (see NMEAGPS.h) // At a minimum, use the fixQuality indicator in GPGGA (FIXME?) diff --git a/variants/nrf52840/tracker-t1000-e/variant.h b/variants/nrf52840/tracker-t1000-e/variant.h index 81b4ef3fb..403552ec0 100644 --- a/variants/nrf52840/tracker-t1000-e/variant.h +++ b/variants/nrf52840/tracker-t1000-e/variant.h @@ -124,8 +124,7 @@ extern "C" { #define GPS_RTC_INT (0 + 15) // P0.15, normal is LOW, wake by HIGH #define GPS_RESETB_OUT (32 + 14) // P1.14, always input pull_up -#define GPS_FIX_HOLD_TIME 15000 // ms -#define BATTERY_PIN 2 // P0.02/AIN0, BAT_ADC +#define BATTERY_PIN 2 // P0.02/AIN0, BAT_ADC #define BATTERY_IMMUTABLE #define ADC_MULTIPLIER (2.0F) // P0.04/AIN2 is VCC_ADC, P0.05/AIN3 is CHARGER_DET, P1.03 is CHARGE_STA, P1.04 is CHARGE_DONE diff --git a/variants/nrf52840/wio-t1000-s/variant.h b/variants/nrf52840/wio-t1000-s/variant.h index eb6a34d6c..02f8a20b2 100644 --- a/variants/nrf52840/wio-t1000-s/variant.h +++ b/variants/nrf52840/wio-t1000-s/variant.h @@ -123,7 +123,6 @@ extern "C" { #define GPS_RESETB_OUT (32 + 14) // P1.14, awlays input pull_up // #define GPS_THREAD_INTERVAL 50 -#define GPS_FIX_HOLD_TIME 15000 // ms #define BATTERY_PIN 2 // #define ADC_CHANNEL ADC1_GPIO2_CHANNEL @@ -157,4 +156,4 @@ extern "C" { * Arduino objects - C++ only *----------------------------------------------------------------------------*/ -#endif // _VARIANT_WIO_SDK_WM1110_ \ No newline at end of file +#endif // _VARIANT_WIO_SDK_WM1110_ From 0b104fd0306932dd5deb00a4d57c041789afb6b0 Mon Sep 17 00:00:00 2001 From: Tom Fifield Date: Mon, 1 Sep 2025 11:52:30 +1000 Subject: [PATCH 3/5] Fix publishing settings --- src/gps/GPS.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/gps/GPS.cpp b/src/gps/GPS.cpp index a8dc7ea5d..9fab136d2 100644 --- a/src/gps/GPS.cpp +++ b/src/gps/GPS.cpp @@ -1055,6 +1055,8 @@ void GPS::down() } // If update interval long enough (or softsleep unsupported): hardsleep instead setPowerState(GPS_HARDSLEEP, sleepTime); + // Reset the fix quality to 0, since we're off. + fixQual = 0; } } @@ -1114,6 +1116,7 @@ int32_t GPS::runOnce() shouldPublish = true; } + bool prev_fixQual = fixQual; bool gotLoc = lookForLocation(); if (gotLoc && !hasValidLocation) { // declare that we have location ASAP LOG_DEBUG("hasValidLocation RISING EDGE"); @@ -1124,6 +1127,14 @@ int32_t GPS::runOnce() fixHoldEnds = lastFixStartMsec + 20000; } + if (gotLoc && prev_fixQual == 0) { // we've moved from no lock to lock + LOG_DEBUG("Probably just got a lock after turning back on."); + // Hold for 20secs after getting a lock to download ephemeris etc + lastFixStartMsec = millis(); + fixHoldEnds = lastFixStartMsec + 20000; + shouldPublish = true; // Publish immediately, since next publish is at end of hold + } + bool tooLong = scheduling.searchedTooLong(); if (tooLong) LOG_WARN("Couldn't publish a valid location: didn't get a GPS lock in time"); @@ -1140,11 +1151,12 @@ int32_t GPS::runOnce() hasValidLocation = false; } if (millis() > fixHoldEnds) { + shouldPublish = true; // publish our update at the end of the lock hold + publishUpdate(); down(); } else { - LOG_DEBUG("Holding for GPS data download: %d ms", fixHoldEnds - millis()); + LOG_DEBUG("Holding for GPS data download: %d ms (numSats=%d)", fixHoldEnds - millis(), p.sats_in_view); } - shouldPublish = true; // publish our update for this just finished acquisition window } // If state has changed do a publish From 0713a08686068e777ec19b0173c9e06f9c58f732 Mon Sep 17 00:00:00 2001 From: Tom Fifield Date: Mon, 1 Sep 2025 12:04:13 +1000 Subject: [PATCH 4/5] Cleanups, removing unused variables. --- src/gps/GPS.cpp | 10 +++------- src/gps/GPS.h | 2 +- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/gps/GPS.cpp b/src/gps/GPS.cpp index 9fab136d2..ab09d7e05 100644 --- a/src/gps/GPS.cpp +++ b/src/gps/GPS.cpp @@ -1123,15 +1123,11 @@ int32_t GPS::runOnce() hasValidLocation = true; shouldPublish = true; // Hold for 20secs after getting a lock to download ephemeris etc - lastFixStartMsec = millis(); - fixHoldEnds = lastFixStartMsec + 20000; + fixHoldEnds = millis() + 20000; } - if (gotLoc && prev_fixQual == 0) { // we've moved from no lock to lock - LOG_DEBUG("Probably just got a lock after turning back on."); - // Hold for 20secs after getting a lock to download ephemeris etc - lastFixStartMsec = millis(); - fixHoldEnds = lastFixStartMsec + 20000; + if (gotLoc && prev_fixQual == 0) { // just got a lock after turning back on. + fixHoldEnds = millis() + 20000; shouldPublish = true; // Publish immediately, since next publish is at end of hold } diff --git a/src/gps/GPS.h b/src/gps/GPS.h index 3dcb17611..177cfe74b 100644 --- a/src/gps/GPS.h +++ b/src/gps/GPS.h @@ -159,7 +159,7 @@ class GPS : private concurrency::OSThread uint8_t fixType = 0; // fix type from GPGSA #endif - uint32_t lastFixStartMsec = 0, fixHoldEnds = 0; + uint32_t fixHoldEnds = 0; uint32_t rx_gpio = 0; uint32_t tx_gpio = 0; From 4135202a58fa42f7b6d024453579cf4166151ec5 Mon Sep 17 00:00:00 2001 From: Tom Fifield Date: Tue, 2 Sep 2025 08:05:45 +1000 Subject: [PATCH 5/5] ifdef log line with GPS_DEBUG --- src/gps/GPS.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/gps/GPS.cpp b/src/gps/GPS.cpp index ab09d7e05..20c2dab43 100644 --- a/src/gps/GPS.cpp +++ b/src/gps/GPS.cpp @@ -1137,7 +1137,6 @@ int32_t GPS::runOnce() // Once we get a location we no longer desperately want an update if ((gotLoc && gotTime) || tooLong) { - if (tooLong && !gotLoc) { // we didn't get a location during this ack window, therefore declare loss of lock if (hasValidLocation) { @@ -1150,8 +1149,10 @@ int32_t GPS::runOnce() shouldPublish = true; // publish our update at the end of the lock hold publishUpdate(); down(); +#ifdef GPS_DEBUG } else { LOG_DEBUG("Holding for GPS data download: %d ms (numSats=%d)", fixHoldEnds - millis(), p.sats_in_view); +#endif } }