From e488035be480534bbae89919925dd877761d714d 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 simple_repeater's applyGpsPrefs() to re-apply a non-zero stored interval at boot, matching what companion_radio already does. simple_room_server and simple_sensor have the same gap in their own applyGpsPrefs() and are left alone here. 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. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01EXSCjgNEbJfHwLjD2WSHW4 --- docs/cli_commands.md | 14 +++++++ examples/simple_repeater/MyMesh.h | 5 +++ src/helpers/CommonCLI.cpp | 41 +++++++++++++++++++ .../sensors/EnvironmentSensorManager.cpp | 13 ++++++ 4 files changed, 73 insertions(+) diff --git a/docs/cli_commands.md b/docs/cli_commands.md index 8772b929fe..592b1e806a 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. + +--- + #### 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/src/helpers/CommonCLI.cpp b/src/helpers/CommonCLI.cpp index 4930e81e9a..bad6bc7226 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,36 @@ 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. + const char* arg = &command[12]; + while (*arg == ' ') arg++; + if (*arg == 0) { // bare `gps interval` (or only trailing 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 { + char secs_str[12]; + uint32_t secs = _atoi(arg); + if (secs > 86400) secs = 86400; // cap at 24 hours + 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;