From 95e47eb92652efcc25a07166397bbe35fd5ff975 Mon Sep 17 00:00:00 2001 From: Tom Fifield Date: Sun, 20 Oct 2024 12:38:59 +1100 Subject: [PATCH 1/8] Optimise GPS Baud Rate cycle Previously, our baud rate cycled through one list twice. There were some rarer baudrates in there, so this code separates out those into a dedicated list that is only run through if detection fails for common bauds. We also only run through each baud rate once. --- src/gps/GPS.cpp | 56 +++++++++++++++++++++++++++++++------------------ src/gps/GPS.h | 5 +++-- 2 files changed, 39 insertions(+), 22 deletions(-) diff --git a/src/gps/GPS.cpp b/src/gps/GPS.cpp index 3a9516c842..7d000a70fc 100644 --- a/src/gps/GPS.cpp +++ b/src/gps/GPS.cpp @@ -420,21 +420,34 @@ bool GPS::setup() if (tx_gpio && gnssModel == GNSS_MODEL_UNKNOWN) { // if GPS_BAUDRATE is specified in variant (i.e. not 9600), skip to the specified rate. - if (speedSelect == 0 && GPS_BAUDRATE != serialSpeeds[speedSelect]) { + if (probeTries == 0 && GPS_BAUDRATE != 9600) { speedSelect = std::find(serialSpeeds, std::end(serialSpeeds), GPS_BAUDRATE) - serialSpeeds; + if (speedSelect == 0) { + speedSelect = std::find(rareSerialSpeeds, std::end(rareSerialSpeeds), GPS_BAUDRATE) - rareSerialSpeeds; + probeTries = 1; + } } - - LOG_DEBUG("Probing for GPS at %d", serialSpeeds[speedSelect]); - gnssModel = probe(serialSpeeds[speedSelect]); - if (gnssModel == GNSS_MODEL_UNKNOWN) { - if (++speedSelect == sizeof(serialSpeeds) / sizeof(int)) { - speedSelect = 0; - if (--probeTries == 0) { + if (probeTries == 0) { + LOG_DEBUG("Probing for GPS at %d", serialSpeeds[speedSelect]); + gnssModel = probe(serialSpeeds[speedSelect]); + if (gnssModel == GNSS_MODEL_UNKNOWN) { + if (++speedSelect == sizeof(serialSpeeds) / sizeof(int)) { + speedSelect = 0; + ++probeTries; + } + } + } + // Rare Serial Speeds + if (probeTries == 1) { + LOG_DEBUG("Probing for GPS at %d", rareSerialSpeeds[speedSelect]); + gnssModel = probe(rareSerialSpeeds[speedSelect]); + if (gnssModel == GNSS_MODEL_UNKNOWN) { + if (++speedSelect == sizeof(rareSerialSpeeds) / sizeof(int)) { LOG_WARN("Giving up on GPS probe and setting to 9600."); return true; } + return false; } - return false; } } else { gnssModel = GNSS_MODEL_UNKNOWN; @@ -662,7 +675,8 @@ bool GPS::setup() SEND_UBX_PACKET(0x06, 0x8A, _message_VALSET_DISABLE_SBAS_BBR, "disable SBAS M10 GPS BBR", 300); delay(750); // will cause a receiver restart so wait a bit - // Done with initialization, Now enable wanted NMEA messages in BBR layer so they will survive a periodic sleep. + // Done with initialization, Now enable wanted NMEA messages in BBR layer so they will survive a periodic + // sleep. SEND_UBX_PACKET(0x06, 0x8A, _message_VALSET_ENABLE_NMEA_BBR, "enable messages for M10 GPS BBR", 300); delay(750); // Next enable wanted NMEA messages in RAM layer @@ -924,10 +938,10 @@ void GPS::down() #endif if (softsleepSupported) { - // How long does gps_update_interval need to be, for GPS_HARDSLEEP to become more efficient than GPS_SOFTSLEEP? - // Heuristic equation. A compromise manually fitted to power observations from U-blox NEO-6M and M10050 - // https://www.desmos.com/calculator/6gvjghoumr - // This is not particularly accurate, but probably an impromevement over a single, fixed threshold + // How long does gps_update_interval need to be, for GPS_HARDSLEEP to become more efficient than + // GPS_SOFTSLEEP? Heuristic equation. A compromise manually fitted to power observations from U-blox NEO-6M + // and M10050 https://www.desmos.com/calculator/6gvjghoumr This is not particularly accurate, but probably an + // impromevement over a single, fixed threshold uint32_t hardsleepThreshold = (2750 * pow(predictedSearchDuration / 1000, 1.22)); LOG_DEBUG("gps_update_interval >= %us needed to justify hardsleep", hardsleepThreshold / 1000); @@ -1279,10 +1293,12 @@ GPS *GPS::createGps() if (!GPS_EN_ACTIVE) { // Need to invert the pin before hardware new GpioNotTransformer( - virtPin, p); // We just leave this created object on the heap so it can stay watching virtPin and driving en_gpio + virtPin, + p); // We just leave this created object on the heap so it can stay watching virtPin and driving en_gpio } else { new GpioUnaryTransformer( - virtPin, p); // We just leave this created object on the heap so it can stay watching virtPin and driving en_gpio + virtPin, + p); // We just leave this created object on the heap so it can stay watching virtPin and driving en_gpio } } @@ -1390,8 +1406,8 @@ bool GPS::factoryReset() _serial_gps->write("$PMTK104*37\r\n"); // No PMTK_ACK for this command. delay(100); - // send the UBLOX Factory Reset Command regardless of detect state, something is very wrong, just assume it's UBLOX. - // Factory Reset + // send the UBLOX Factory Reset Command regardless of detect state, something is very wrong, just assume it's + // UBLOX. Factory Reset byte _message_reset[] = {0xB5, 0x62, 0x06, 0x09, 0x0D, 0x00, 0xFF, 0xFB, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xFF, 0xFF, 0x00, 0x00, 0x17, 0x2B, 0x7E}; _serial_gps->write(_message_reset, sizeof(_message_reset)); @@ -1430,8 +1446,8 @@ bool GPS::lookForTime() auto d = reader.date; if (ti.isValid() && d.isValid()) { // Note: we don't check for updated, because we'll only be called if needed /* Convert to unix time -The Unix epoch (or Unix time or POSIX time or Unix timestamp) is the number of seconds that have elapsed since January 1, 1970 -(midnight UTC/GMT), not counting leap seconds (in ISO 8601: 1970-01-01T00:00:00Z). +The Unix epoch (or Unix time or POSIX time or Unix timestamp) is the number of seconds that have elapsed since January 1, +1970 (midnight UTC/GMT), not counting leap seconds (in ISO 8601: 1970-01-01T00:00:00Z). */ struct tm t; t.tm_sec = ti.second() + round(ti.age() / 1000); diff --git a/src/gps/GPS.h b/src/gps/GPS.h index 8b1982cf74..55fc649f3a 100644 --- a/src/gps/GPS.h +++ b/src/gps/GPS.h @@ -75,13 +75,14 @@ class GPS : private concurrency::OSThread uint8_t fixType = 0; // fix type from GPGSA #endif private: - const int serialSpeeds[6] = {9600, 115200, 38400, 4800, 57600, 9600}; + const int serialSpeeds[6] = {9600, 115200, 38400}; + const int rareSerialSpeeds[6] = {4800, 57600, 9600}; uint32_t lastWakeStartMsec = 0, lastSleepStartMsec = 0, lastFixStartMsec = 0; uint32_t rx_gpio = 0; uint32_t tx_gpio = 0; int speedSelect = 0; - int probeTries = 2; + int probeTries = 0; /** * hasValidLocation - indicates that the position variables contain a complete From 3df4fa65f2c5a1128d6fb13715be0ba4a91f1bfb Mon Sep 17 00:00:00 2001 From: Tom Fifield Date: Mon, 21 Oct 2024 19:39:37 +1100 Subject: [PATCH 2/8] Fix first time around bug Would have always reset GPS baudrate every time. --- src/gps/GPS.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gps/GPS.cpp b/src/gps/GPS.cpp index 7d000a70fc..be18546c44 100644 --- a/src/gps/GPS.cpp +++ b/src/gps/GPS.cpp @@ -420,7 +420,7 @@ bool GPS::setup() if (tx_gpio && gnssModel == GNSS_MODEL_UNKNOWN) { // if GPS_BAUDRATE is specified in variant (i.e. not 9600), skip to the specified rate. - if (probeTries == 0 && GPS_BAUDRATE != 9600) { + if (probeTries == 0 && speedSelect == 0 && GPS_BAUDRATE != serialSpeeds[speedSelect]) { speedSelect = std::find(serialSpeeds, std::end(serialSpeeds), GPS_BAUDRATE) - serialSpeeds; if (speedSelect == 0) { speedSelect = std::find(rareSerialSpeeds, std::end(rareSerialSpeeds), GPS_BAUDRATE) - rareSerialSpeeds; From 3b265f5392232bab448c1956d761fc922144f62c Mon Sep 17 00:00:00 2001 From: Tom Fifield Date: Sat, 2 Nov 2024 10:16:07 +0800 Subject: [PATCH 3/8] Add support for fixing GPS_BAUDRATE If GPS_BAUDRATE is set in variant.h, the deployer knows something we don't about the GPS. Used especially when the GPS is soldered to a board in a commercial product :) If we see that, we don't try other baud rates at all. --- src/configuration.h | 3 +++ src/gps/GPS.cpp | 59 +++++++++++++++++++++++++++------------------ src/gps/GPS.h | 5 ++-- 3 files changed, 40 insertions(+), 27 deletions(-) diff --git a/src/configuration.h b/src/configuration.h index cb2326470c..33f11bd766 100644 --- a/src/configuration.h +++ b/src/configuration.h @@ -207,6 +207,9 @@ along with this program. If not, see . #ifndef GPS_BAUDRATE #define GPS_BAUDRATE 9600 +#define GPS_BAUDRATE_FIXED 0 +#else +#define GPS_BAUDRATE_FIXED 1 #endif /* Step #2: follow with defines common to the architecture; diff --git a/src/gps/GPS.cpp b/src/gps/GPS.cpp index 2302e200de..49e41edd99 100644 --- a/src/gps/GPS.cpp +++ b/src/gps/GPS.cpp @@ -413,6 +413,13 @@ int GPS::getACK(uint8_t *buffer, uint16_t size, uint8_t requestedClass, uint8_t return 0; } +/** + * @brief Setup the GPS based on the model detected. + * We detect the GPS by cyling through a set of baud rates, first common then rare. + * For each baud rate, we run GPS::Probe to send commands and match the responses + * to known GPS responses. + * @retval Whether setup reached the end of its potential to configure the GPS. + */ bool GPS::setup() { @@ -420,35 +427,39 @@ bool GPS::setup() int msglen = 0; if (tx_gpio && gnssModel == GNSS_MODEL_UNKNOWN) { - // if GPS_BAUDRATE is specified in variant (i.e. not 9600), skip to the specified rate. - if (probeTries == 0 && speedSelect == 0 && GPS_BAUDRATE != serialSpeeds[speedSelect]) { - speedSelect = std::find(serialSpeeds, std::end(serialSpeeds), GPS_BAUDRATE) - serialSpeeds; - if (speedSelect == 0) { - speedSelect = std::find(rareSerialSpeeds, std::end(rareSerialSpeeds), GPS_BAUDRATE) - rareSerialSpeeds; - probeTries = 1; + // if GPS_BAUDRATE is specified in variant, skip to the specified rate. + if (GPS_BAUDRATE_FIXED) { + gnssModel = probe(GPS_BAUDRATE); + if (gnssModel == GNSS_MODEL_UNKNOWN && probeTries == 1) { + LOG_WARN("GPS probe failed, setting to %d", GPS_BAUDRATE); + return true; + } else { + ++probeTries; + return false; } - } - if (probeTries == 0) { - LOG_DEBUG("Probing for GPS at %d", serialSpeeds[speedSelect]); - gnssModel = probe(serialSpeeds[speedSelect]); - if (gnssModel == GNSS_MODEL_UNKNOWN) { - if (++speedSelect == sizeof(serialSpeeds) / sizeof(int)) { - speedSelect = 0; - ++probeTries; + } else { + if (probeTries == 0) { + LOG_DEBUG("Probing for GPS at %d", serialSpeeds[speedSelect]); + gnssModel = probe(serialSpeeds[speedSelect]); + if (gnssModel == GNSS_MODEL_UNKNOWN) { + if (++speedSelect == sizeof(serialSpeeds) / sizeof(int)) { + speedSelect = 0; + ++probeTries; + } } } - } - // Rare Serial Speeds - if (probeTries == 1) { - LOG_DEBUG("Probing for GPS at %d", rareSerialSpeeds[speedSelect]); - gnssModel = probe(rareSerialSpeeds[speedSelect]); - if (gnssModel == GNSS_MODEL_UNKNOWN) { - if (++speedSelect == sizeof(rareSerialSpeeds) / sizeof(int)) { - LOG_WARN("Giving up on GPS probe and setting to %d", GPS_BAUDRATE); - return true; + // Rare Serial Speeds + if (probeTries == 1) { + LOG_DEBUG("Probing for GPS at %d", rareSerialSpeeds[speedSelect]); + gnssModel = probe(rareSerialSpeeds[speedSelect]); + if (gnssModel == GNSS_MODEL_UNKNOWN) { + if (++speedSelect == sizeof(rareSerialSpeeds) / sizeof(int)) { + LOG_WARN("Giving up on GPS probe and setting to %d", GPS_BAUDRATE); + return true; + } } - return false; } + return false; } } else { gnssModel = GNSS_MODEL_UNKNOWN; diff --git a/src/gps/GPS.h b/src/gps/GPS.h index 808c4008b7..f1abcf4b2f 100644 --- a/src/gps/GPS.h +++ b/src/gps/GPS.h @@ -76,9 +76,8 @@ class GPS : private concurrency::OSThread uint8_t fixType = 0; // fix type from GPGSA #endif private: - - const int serialSpeeds[6] = {9600, 115200, 38400}; - const int rareSerialSpeeds[6] = {4800, 57600, GPS_BAUDRATE}; + const int serialSpeeds[3] = {9600, 115200, 38400}; + const int rareSerialSpeeds[3] = {4800, 57600, GPS_BAUDRATE}; uint32_t lastWakeStartMsec = 0, lastSleepStartMsec = 0, lastFixStartMsec = 0; uint32_t rx_gpio = 0; From 2954095f3da6cea06bf4a519a904ca74b86b78d8 Mon Sep 17 00:00:00 2001 From: Tom Fifield Date: Sat, 2 Nov 2024 11:25:46 +0800 Subject: [PATCH 4/8] Don't print blank lines in GPS_DEBUG. --- src/gps/GPS.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/gps/GPS.cpp b/src/gps/GPS.cpp index 49e41edd99..e6d93ded0c 100644 --- a/src/gps/GPS.cpp +++ b/src/gps/GPS.cpp @@ -1705,7 +1705,9 @@ bool GPS::whileActive() } } #ifdef GPS_DEBUG - LOG_DEBUG(debugmsg.c_str()); + if (debugmsg != "") { + LOG_DEBUG(debugmsg.c_str()); + } #endif return isValid; } From 1d804f5a01af11b637acbfdcba763d6478855284 Mon Sep 17 00:00:00 2001 From: Tom Fifield Date: Sat, 2 Nov 2024 11:42:53 +0800 Subject: [PATCH 5/8] Try GPS_BAUDRATE first, not only. --- src/gps/GPS.cpp | 48 ++++++++++++++++++------------------------------ src/gps/GPS.h | 5 +++++ 2 files changed, 23 insertions(+), 30 deletions(-) diff --git a/src/gps/GPS.cpp b/src/gps/GPS.cpp index e6d93ded0c..6039fe9239 100644 --- a/src/gps/GPS.cpp +++ b/src/gps/GPS.cpp @@ -426,41 +426,29 @@ bool GPS::setup() if (!didSerialInit) { int msglen = 0; if (tx_gpio && gnssModel == GNSS_MODEL_UNKNOWN) { - - // if GPS_BAUDRATE is specified in variant, skip to the specified rate. - if (GPS_BAUDRATE_FIXED) { - gnssModel = probe(GPS_BAUDRATE); - if (gnssModel == GNSS_MODEL_UNKNOWN && probeTries == 1) { - LOG_WARN("GPS probe failed, setting to %d", GPS_BAUDRATE); - return true; - } else { - ++probeTries; - return false; - } - } else { - if (probeTries == 0) { - LOG_DEBUG("Probing for GPS at %d", serialSpeeds[speedSelect]); - gnssModel = probe(serialSpeeds[speedSelect]); - if (gnssModel == GNSS_MODEL_UNKNOWN) { - if (++speedSelect == sizeof(serialSpeeds) / sizeof(int)) { - speedSelect = 0; - ++probeTries; - } + if (probeTries < 2) { + LOG_DEBUG("Probing for GPS at %d", serialSpeeds[speedSelect]); + gnssModel = probe(serialSpeeds[speedSelect]); + if (gnssModel == GNSS_MODEL_UNKNOWN) { + if (++speedSelect == sizeof(serialSpeeds) / sizeof(int)) { + speedSelect = 0; + ++probeTries; } } - // Rare Serial Speeds - if (probeTries == 1) { - LOG_DEBUG("Probing for GPS at %d", rareSerialSpeeds[speedSelect]); - gnssModel = probe(rareSerialSpeeds[speedSelect]); - if (gnssModel == GNSS_MODEL_UNKNOWN) { - if (++speedSelect == sizeof(rareSerialSpeeds) / sizeof(int)) { - LOG_WARN("Giving up on GPS probe and setting to %d", GPS_BAUDRATE); - return true; - } + } + // Rare Serial Speeds + if (probeTries == 2) { + LOG_DEBUG("Probing for GPS at %d", rareSerialSpeeds[speedSelect]); + gnssModel = probe(rareSerialSpeeds[speedSelect]); + if (gnssModel == GNSS_MODEL_UNKNOWN) { + if (++speedSelect == sizeof(rareSerialSpeeds) / sizeof(int)) { + LOG_WARN("Giving up on GPS probe and setting to %d", GPS_BAUDRATE); + return true; } } - return false; } + return false; + } else { gnssModel = GNSS_MODEL_UNKNOWN; } diff --git a/src/gps/GPS.h b/src/gps/GPS.h index f1abcf4b2f..b95d0f76ba 100644 --- a/src/gps/GPS.h +++ b/src/gps/GPS.h @@ -76,7 +76,12 @@ class GPS : private concurrency::OSThread uint8_t fixType = 0; // fix type from GPGSA #endif private: +#if GPS_BAUDRATE_FIXED + // if GPS_BAUDRATE is specified in variant, try that first. + const int serialSpeeds[4] = {GPS_BAUDRATE, 9600, 115200, 38400}; +#else const int serialSpeeds[3] = {9600, 115200, 38400}; +#endif const int rareSerialSpeeds[3] = {4800, 57600, GPS_BAUDRATE}; uint32_t lastWakeStartMsec = 0, lastSleepStartMsec = 0, lastFixStartMsec = 0; From 065f9a5199f3606ba75de9c101729237260c0e1d Mon Sep 17 00:00:00 2001 From: Tom Fifield Date: Sat, 2 Nov 2024 12:25:36 +0800 Subject: [PATCH 6/8] Fix spelling mistakes in comments --- src/gps/GPS.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gps/GPS.cpp b/src/gps/GPS.cpp index 6039fe9239..60d9e8b24b 100644 --- a/src/gps/GPS.cpp +++ b/src/gps/GPS.cpp @@ -415,7 +415,7 @@ int GPS::getACK(uint8_t *buffer, uint16_t size, uint8_t requestedClass, uint8_t /** * @brief Setup the GPS based on the model detected. - * We detect the GPS by cyling through a set of baud rates, first common then rare. + * We detect the GPS by cycling through a set of baud rates, first common then rare. * For each baud rate, we run GPS::Probe to send commands and match the responses * to known GPS responses. * @retval Whether setup reached the end of its potential to configure the GPS. @@ -953,7 +953,7 @@ void GPS::down() // How long does gps_update_interval need to be, for GPS_HARDSLEEP to become more efficient than // GPS_SOFTSLEEP? Heuristic equation. A compromise manually fitted to power observations from U-blox NEO-6M // and M10050 https://www.desmos.com/calculator/6gvjghoumr This is not particularly accurate, but probably an - // impromevement over a single, fixed threshold + // improvement over a single, fixed threshold uint32_t hardsleepThreshold = (2750 * pow(predictedSearchDuration / 1000, 1.22)); LOG_DEBUG("gps_update_interval >= %us needed to justify hardsleep", hardsleepThreshold / 1000); From c41e76b8e629f4c6b7425ad5972b20bf729e6ddf Mon Sep 17 00:00:00 2001 From: Tom Fifield Date: Sat, 2 Nov 2024 19:19:19 +0800 Subject: [PATCH 7/8] Only use GPS_BAUDRATE if specified in variant.h --- src/gps/GPS.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gps/GPS.h b/src/gps/GPS.h index b95d0f76ba..483cb802d2 100644 --- a/src/gps/GPS.h +++ b/src/gps/GPS.h @@ -77,8 +77,8 @@ class GPS : private concurrency::OSThread #endif private: #if GPS_BAUDRATE_FIXED - // if GPS_BAUDRATE is specified in variant, try that first. - const int serialSpeeds[4] = {GPS_BAUDRATE, 9600, 115200, 38400}; + // if GPS_BAUDRATE is specified in variant, only try that. + const int serialSpeeds[1] = {GPS_BAUDRATE}; #else const int serialSpeeds[3] = {9600, 115200, 38400}; #endif From ffb4bff9a29c421fdc6b4f21aa209c98edf8c3fa Mon Sep 17 00:00:00 2001 From: Tom Fifield Date: Sat, 2 Nov 2024 19:50:47 +0800 Subject: [PATCH 8/8] Modify RareSerial Speeds based on FIXED or not. --- src/gps/GPS.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/gps/GPS.h b/src/gps/GPS.h index 483cb802d2..cd61c5444c 100644 --- a/src/gps/GPS.h +++ b/src/gps/GPS.h @@ -79,10 +79,11 @@ class GPS : private concurrency::OSThread #if GPS_BAUDRATE_FIXED // if GPS_BAUDRATE is specified in variant, only try that. const int serialSpeeds[1] = {GPS_BAUDRATE}; + const int rareSerialSpeeds[1] = {GPS_BAUDRATE}; #else const int serialSpeeds[3] = {9600, 115200, 38400}; -#endif const int rareSerialSpeeds[3] = {4800, 57600, GPS_BAUDRATE}; +#endif uint32_t lastWakeStartMsec = 0, lastSleepStartMsec = 0, lastFixStartMsec = 0; uint32_t rx_gpio = 0;