From 3aff3183f8ef9612ecc461ec6077178c346dfce1 Mon Sep 17 00:00:00 2001 From: mmmorks Date: Mon, 7 Sep 2026 14:21:52 -0700 Subject: [PATCH] CLI: add `gps interval` to set and persist the GPS read interval NodePrefs::gps_interval has been persisted in the prefs blob (and the "gps.int" key) since it was added, but nothing outside companion_radio's CMD_SET_CUSTOM_VAR path could write it: CommonCLI's get/set commands are a hand-written chain that never reaches the ConfigSerializer tree, and simple_repeater's applyGpsPrefs() pushed only "gps" to the sensor manager and never the interval. The stored value was loaded, saved, and ignored, leaving EnvironmentSensorManager on its 1 s default -- which on a node with a fix prints two lat/lon debug lines per second. Add "gps interval [seconds]" in the shape of "gps advert": the bare form reports, the argument form applies and persists, capped at 24 hours, with zero keeping the firmware default of 1 s at both ends. The argument must be all digits, since _atoi() reads a typo like "abc" as 0 and would silently reset the pref while answering "ok". The prefix match requires a delimiter after "interval": memcmp(command, "gps interval", 12) matches "gps intervalX" too, which would then read its argument from past the 'X' -- an empty string, so 0 -- instead of falling through to the generic "gps" handler and being rejected. Trailing spaces read as the bare query. Teach applyGpsPrefs() to re-apply a non-zero stored interval at boot in every firmware that compiles the command -- simple_repeater, simple_room_server and simple_sensor -- matching what companion_radio already does. Without that the setting is accepted and persisted but silently lost on the next boot, which is exactly what the command is for. Out-of-range and overlong arguments are rejected rather than clamped: _atoi() accumulates into a uint32_t with no overflow check, so "gps interval 4294967300" would otherwise wrap to 4 and be stored as a 4-second interval while answering "ok". gps_interval stays settable-but-not-enumerated in EnvironmentSensorManager, and the comment there records why: the enumeration calls also build RESP_CODE_CUSTOM_VARS's wire payload on every companion build, so listing it would change what every embedded companion node sends. It is also deliberately not gated on gps_detected, because every example's applyGpsPrefs() calls it at boot regardless. Documented in docs/cli_commands.md. --- docs/cli_commands.md | 14 ++++++ examples/simple_repeater/MyMesh.h | 5 ++ examples/simple_room_server/MyMesh.h | 5 ++ examples/simple_sensor/SensorMesh.h | 5 ++ src/helpers/CommonCLI.cpp | 50 +++++++++++++++++++ .../sensors/EnvironmentSensorManager.cpp | 13 +++++ 6 files changed, 92 insertions(+) diff --git a/docs/cli_commands.md b/docs/cli_commands.md index 8772b929fe..d668694ad3 100644 --- a/docs/cli_commands.md +++ b/docs/cli_commands.md @@ -1010,6 +1010,20 @@ region save --- +#### View or change the GPS read interval +**Usage:** +- `gps interval` +- `gps interval ` + +**Parameters:** +- `seconds`: seconds between location reads, `0` to `86400`. `0` restores the firmware default (1 s) + +**Default:** `0` + +**Note:** The bare form reports the stored value. The setting is persisted and re-applied at boot. A non-numeric or out-of-range argument is rejected with an error rather than clamped. + +--- + #### Sync this node's clock with GPS time **Usage:** - `gps sync` diff --git a/examples/simple_repeater/MyMesh.h b/examples/simple_repeater/MyMesh.h index cac6c4a281..1548b9c698 100644 --- a/examples/simple_repeater/MyMesh.h +++ b/examples/simple_repeater/MyMesh.h @@ -160,6 +160,11 @@ class MyMesh : public mesh::Mesh, public CommonCLICallbacks { #if ENV_INCLUDE_GPS == 1 void applyGpsPrefs() { sensors.setSettingValue("gps", _prefs.gps_enabled?"1":"0"); + if (_prefs.gps_interval > 0) { // 0 = leave the firmware default (1 s) + char interval_str[12]; // max: 86400 seconds, 5 digits + null + sprintf(interval_str, "%u", (unsigned) _prefs.gps_interval); + sensors.setSettingValue("gps_interval", interval_str); + } } #endif diff --git a/examples/simple_room_server/MyMesh.h b/examples/simple_room_server/MyMesh.h index 5cf949c6bd..fcea730ff8 100644 --- a/examples/simple_room_server/MyMesh.h +++ b/examples/simple_room_server/MyMesh.h @@ -169,6 +169,11 @@ class MyMesh : public mesh::Mesh, public CommonCLICallbacks { #if ENV_INCLUDE_GPS == 1 void applyGpsPrefs() { sensors.setSettingValue("gps", _prefs.gps_enabled?"1":"0"); + if (_prefs.gps_interval > 0) { // 0 = leave the firmware default (1 s) + char interval_str[12]; // max: 86400 seconds, 5 digits + null + sprintf(interval_str, "%u", (unsigned) _prefs.gps_interval); + sensors.setSettingValue("gps_interval", interval_str); + } } #endif diff --git a/examples/simple_sensor/SensorMesh.h b/examples/simple_sensor/SensorMesh.h index b5e96d5cc7..187a82acf3 100644 --- a/examples/simple_sensor/SensorMesh.h +++ b/examples/simple_sensor/SensorMesh.h @@ -162,6 +162,11 @@ class SensorMesh : public mesh::Mesh, public CommonCLICallbacks { #if ENV_INCLUDE_GPS == 1 void applyGpsPrefs() { sensors.setSettingValue("gps", _prefs.gps_enabled?"1":"0"); + if (_prefs.gps_interval > 0) { // 0 = leave the firmware default (1 s) + char interval_str[12]; // max: 86400 seconds, 5 digits + null + sprintf(interval_str, "%u", (unsigned) _prefs.gps_interval); + sensors.setSettingValue("gps_interval", interval_str); + } } #endif }; diff --git a/src/helpers/CommonCLI.cpp b/src/helpers/CommonCLI.cpp index 4930e81e9a..31d4ea2503 100644 --- a/src/helpers/CommonCLI.cpp +++ b/src/helpers/CommonCLI.cpp @@ -19,6 +19,17 @@ static uint32_t _atoi(const char* sp) { return n; } +// _atoi() returns 0 for garbage input, same as for a real "0" -- callers that +// need to tell "you typed 0" from "you typed a typo" check the string first. +static bool isAllDigits(const char* s) { + if (*s == 0) return false; + while (*s) { + if (*s < '0' || *s > '9') return false; + s++; + } + return true; +} + static bool isValidName(const char *n) { while (*n) { if (*n == '[' || *n == ']' || *n == '\\' || *n == ':' || *n == ',' || *n == '?' || *n == '*') return false; @@ -390,6 +401,45 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, char* command, char* re } else { strcpy(reply, "error"); } + } else if (memcmp(command, "gps interval", 12) == 0 + && (command[12] == 0 || command[12] == ' ')) { + // Seconds between location reads. 0 means "use the firmware default" + // (1 s), matching how applyGpsPrefs() interprets a zero pref. + // + // The prefix match needs the delimiter check: without it "gps intervalX" + // matches here and then parses command[13] -- past the 'X' -- as the + // argument, so a mistyped command silently sets the interval to 0 instead + // of falling through to the generic "gps" handler and being rejected. + char* arg = &command[12]; + while (*arg == ' ') arg++; + char* arg_end = arg + strlen(arg); // a trailing space must not fail the digit test + while (arg_end > arg && arg_end[-1] == ' ') arg_end--; + *arg_end = 0; + if (*arg == 0) { // bare `gps interval` (or only spaces): report + sprintf(reply, "> %u", (unsigned) _prefs->gps_interval); + } else if (!isAllDigits(arg)) { + // _atoi() would silently read a typo like "abc" as 0, resetting the + // pref to the firmware default instead of reporting the mistake. + strcpy(reply, "Error: interval must be a number of seconds"); + } else if (arg_end - arg > 5 || _atoi(arg) > 86400) { + // Reject on length before parsing: _atoi() accumulates into a uint32_t + // with no overflow check, so a long enough run of digits wraps back + // into range and would be accepted ("4294967300" lands on 4). The cap + // is five digits, so no valid value is lost. Out-of-range values are + // reported rather than clamped, matching the advert.interval handlers. + strcpy(reply, "Error: interval range is 0-86400 seconds"); + } else { + char secs_str[12]; + uint32_t secs = _atoi(arg); + sprintf(secs_str, "%u", (unsigned) secs); + if (_sensors->setSettingValue("gps_interval", secs_str)) { + _prefs->gps_interval = secs; + savePrefs(); + strcpy(reply, "ok"); + } else { + strcpy(reply, "gps interval not found"); + } + } } else if (memcmp(command, "gps", 3) == 0) { LocationProvider * l = _sensors->getLocationProvider(); if (l != NULL) { diff --git a/src/helpers/sensors/EnvironmentSensorManager.cpp b/src/helpers/sensors/EnvironmentSensorManager.cpp index 6f4607751c..574f870a38 100644 --- a/src/helpers/sensors/EnvironmentSensorManager.cpp +++ b/src/helpers/sensors/EnvironmentSensorManager.cpp @@ -737,6 +737,19 @@ bool EnvironmentSensorManager::setSettingValue(const char* name, const char* val } return true; } + // Deliberately NOT enumerated by getNumSettings()/getSettingName()/ + // getSettingValue() above, unlike "gps": those three also drive + // CMD_GET_CUSTOM_VARS's wire payload in every companion_radio build + // (examples/companion_radio/MyMesh.cpp, ui-tiny/ui-new UITask.cpp), so + // adding an entry there changes an embedded companion-app payload on every + // target that compiles ENV_INCLUDE_GPS. "gps_interval" stays + // settable-but-not-listed, same as before. + // + // Also deliberately NOT gated on gps_detected, unlike "gps": every + // example's applyGpsPrefs() calls this at boot regardless of whether GPS + // hardware was found (and CMD_SET_CUSTOM_VAR / CommonCLI's `gps interval` + // rely on that), so gating it would break persisting the interval pref on + // boards where no GPS was detected at boot. if (strcmp(name, "gps_interval") == 0) { uint32_t interval_seconds = atoi(value); gps_update_interval_sec = interval_seconds > 0 ? interval_seconds : 1;