diff --git a/docs/Repeater_CLI_commands.md b/docs/Repeater_CLI_commands.md index 8bba877..f876349 100644 --- a/docs/Repeater_CLI_commands.md +++ b/docs/Repeater_CLI_commands.md @@ -293,6 +293,20 @@ All `set uplink.*` changes are saved immediately and only applied after reboot. Changes are persisted immediately unless noted. Some require a reboot. +**The literal `default` is accepted by every `set` that takes a number or an +on/off value**, and restores that setting to what a factory-fresh node boots +with — e.g. `set probe.interval default`, `set tx default`, `set cad.auto +default`. The values come from `initNodePrefs()` (`helpers/NodePrefs.h`), so the +keyword cannot drift from the real defaults. `set radio default` restores all +four radio parameters together, since they are one interop-critical set. + +> [!NOTE] +> Before 1.17.4 these setters used `atoi()`/`atof()`, which turn any +> non-numeric text — `default` included — into `0`. On the knobs where `0` is +> itself legal and means "off" (`probe.interval`, `cad.busycap`, `flood.max`, +> `rxduty`) that silently disabled the feature and still answered `OK`. +> Unparseable input is now rejected with an error instead. + | Command | Constraints | Description | |---------|-------------|-------------| | `set name ` | No `[ ] \ : , ? *` | Set node name | @@ -336,7 +350,7 @@ Changes are persisted immediately unless noted. Some require a reboot. | `set cad.offset ` | −8 to 12, default 0 | Operating detPeak offset from the chip family's base for the current SF, bandwidth and CAD symbol count (Semtech LoRa Basics Modem reference tables; SX126x ~18–34, LR11xx ~50–85, LR20xx its own symbol-indexed table). Negative = more sensitive LBT (catches weaker signals, risks false busy), positive = less sensitive. Wide range so dense hilltops / quiet valleys can settle far from base. The per-family absolute clamp in the driver (SX126x 12–48, LR11xx 40–100, LR20xx 48–90) is a firmware guardrail against a CAD that never/always fires, not a chip limit (`cadDetPeak` is a full `uint8_t`); the driver reports it so the controller narrows this range to match rather than exploring offsets that collapse onto one peak. Applied live; the auto staircase may move it later if `cad.auto` is on. | | `set probe.interval ` | 0 (off) or 10–255, default **15** | Seconds between periodic radio measurements. ONE reading serves both: the noise-floor RSSI sample (median of 8) and the CAD calibration probe, which consumes that same reading rather than measuring separately — so this is also the noise-floor sampling rate, and it sets how often an idle repeater wakes. Default 15 s → ~1–2 h CAD staircase response; the floor EMA warms up over 8 samples (~2 min) and its unguarded bypass runs every 16th (~4 min). Longer = fewer wakes, slower to track a changing RF environment. 0 disables CAD probing entirely (also freezes auto adaptation); the floor sampler then falls back to its build-time default. | | `set cad.busycap ` | 0 (off) or 10–90, default **25** | Airtime-protection cap: the max percentage of TX attempts the node will let CAD defer before the staircase backs off to a less sensitive detPeak — counting **real** traffic, not just false positives. On a congested hilltop most busy verdicts are distant traffic won on capture anyway, so deferring for all of it starves the node's own airtime. Self-targeting: a quiet node's busy rate never reaches the cap. Shown as `bc:` in `get cad.stats`. 0 disables the cap (pure knee-seeking). | -| `set cad.reset` | | Clear the accumulated per-level CAD probe statistics (RAM only; also cleared automatically on any radio parameter change). | +| `set cad.reset` | | Full CAD reset: clears the accumulated per-level probe statistics **and** returns the operating detPeak offset to the family base (`cad.offset` = 0), applying it to the radio live and persisting it. Use after a change to the base tables, or to undo a staircase that has walked somewhere unhelpful. Before 1.17.4 this cleared only the statistics, leaving the node to re-converge *from* the walked offset with no evidence for why it was there. | | `set extra.sf [sf] [sf]` | up to 3 SFs, `0`/`off` clears | **LR2021 only** (`Error: unsupported` elsewhere) — LoRa *side detectors*: demodulate up to three extra spreading factors concurrently with `sf`, on the same bandwidth, so one repeater can serve several SF communities. Which SF a packet arrived on is a chip-side readout, not a guess. Chip constraints, enforced in the driver and reported as `Error: unsupported or invalid extra SF config`: every extra SF must be **greater** than `sf`, all distinct, highest−lowest ≤ 4, and at BW ≥ 500 kHz at most 2 (only 1 when `sf` ≥ 10). **Receive only, and the bridge it creates is one-way.** TX always uses the single configured `sf`, and all detectors share one bandwidth, so this is multi-SF, not multi-channel. A node with `sf 7` + `extra.sf 8` hears SF8 traffic and *does* forward it — but the forward goes out at SF7, so traffic moves SF8 -> SF7 only and nothing comes back. An SF8 node's direct messages are delivered while its ACKs never arrive, so it retries to its limit every time; adverts and one-way flood traffic propagate fine. Because every extra SF must be **greater** than `sf`, the main SF is always the lowest in the set and TX always uses it — so the bridge direction is fixed at high-SF-in / low-SF-out and **cannot be reversed**. Two nodes back to back both point the same way; there is no configuration that carries SF7 -> SF8. Treat it as a collector for slower-SF stragglers, not as a link between two SF islands. Applied live and restored on every RX entry. **Interaction with CAD:** the chip's SF constraint for CAD is the inverse of the one for RX, so the driver switches side detectors off for each LBT CAD and back on when RX re-arms — two extra SPI commands per TX, no configuration required. Persisted; a set that no longer fits after an `sf`/`bw` change is refused at boot and logged. | | `set prv.key ` | **128-char hex** (64-byte expanded Ed25519 key) | Replace private key; derive new identity *(reboot to apply)*. The length must be exact — `fromHex` rejects anything else with `Error, bad key`. `get prv.key` returns the same 128-char form. Not USB-gated. | diff --git a/releasenotes/RELEASE_NOTES_1.17.4-zephcore.md b/releasenotes/RELEASE_NOTES_1.17.4-zephcore.md index f087dd4..4bf923d 100644 --- a/releasenotes/RELEASE_NOTES_1.17.4-zephcore.md +++ b/releasenotes/RELEASE_NOTES_1.17.4-zephcore.md @@ -107,6 +107,11 @@ unless you run a 250 or 500 kHz bandwidth, where the old value was up to ten cou > **Run `set cad.reset` after upgrading.** The tuning statistics your node collected are measured > against the old starting point and are not comparable to the new one. Clearing them lets the tuner > re-converge cleanly; left alone it blends two sets of readings. Everything else is automatic. +> +> `cad.reset` also returns the threshold itself to the starting point now — see below. Until this +> release it cleared only the statistics, which left a node re-tuning from wherever it had already +> walked to. On the LR1110 boards that is exactly the position you are trying to leave, so run this +> on the new firmware, not the old. > [!IMPORTANT] > This is a first release of the corrected tables. They are verified against Semtech's reference and @@ -136,6 +141,43 @@ anywhere. --- +## Two console commands did not do what they said + +**`set cad.reset` left the threshold where it was.** It cleared the tuning statistics and nothing +else, so a node that had spent weeks walking its listen-before-talk threshold away from the starting +point stayed exactly there — now with no measurements to explain why. That is the opposite of a +reset, and it matters most in this release, where the whole point of the command is to let an LR1110 +node leave the position the old reference table pushed it into. It now clears the statistics *and* +returns the threshold to the starting point, applying it to the radio immediately. + +**`set probe.interval default` switched probing off.** The console read the word `default` as the +number zero, and zero is a legal value for that setting meaning "stop probing" — so the node did +exactly that and answered `OK`. Any typo did the same. The same flaw sat in `set cad.busycap`, where +zero means "no airtime cap". + +Both are fixed, and the fix is general: **every `set` that takes a number or an on/off value now +accepts the word `default`** and restores that setting to what a factory-fresh node uses. `set radio +default` puts all four radio parameters back together. Input that is neither a number nor `default` +is now rejected with an error instead of being quietly read as zero. + +> [!NOTE] +> **Worth checking `get probe.interval` and `get cad.busycap` on nodes you have configured by hand.** +> If either reads `0` and you did not intend to switch it off, a mistyped value is the likely cause. +> `set probe.interval default` and `set cad.busycap default` now genuinely restore them. + +--- + +## Two settings did not behave the way they were documented + +**`set backoff.multiplier 0` did not survive a reboot.** Zero is the documented way +to switch reactive backoff off, and it worked until the node restarted — at which +point a migration meant for old settings files could not tell "never configured" +from "deliberately zero", and put it back to 0.2. Anyone who believed they had +disabled reactive backoff has in fact been running with it on the whole time. Zero +now means zero, and a genuinely absent setting still arrives as 0.2. + +--- + ## Also in this release Nothing here changes how a node behaves. diff --git a/zephcore/Kconfig b/zephcore/Kconfig index 98ffe53..45c6494 100644 --- a/zephcore/Kconfig +++ b/zephcore/Kconfig @@ -459,9 +459,10 @@ config ZEPHCORE_NOISE_FLOOR_INTERVAL_MS raising it there would change their RF tracking for no power gain, since their housekeeping timer wakes them at 5 s regardless. - Repeaters use 15 s, matching ZEPHCORE_CAD_PROBE_INTERVAL — the only other - recurring radio deadline — so the two do not interleave into separate - wakes any more often than they have to. + Repeaters use 15 s, matching the default probe.interval pref (set in + initNodePrefs(), helpers/NodePrefs.h) — the only other recurring radio + deadline — so the two do not interleave into separate wakes any more + often than they have to. On a repeater this is one of the two shortest recurring deadlines, so it bounds how long the SoC can sleep between wakes. Raising it stretches diff --git a/zephcore/app/RepeaterDataStore.cpp b/zephcore/app/RepeaterDataStore.cpp index 15c8d87..920f88c 100644 --- a/zephcore/app/RepeaterDataStore.cpp +++ b/zephcore/app/RepeaterDataStore.cpp @@ -284,10 +284,12 @@ bool RepeaterDataStore::loadPrefs(NodePrefs& prefs) { * unwritten byte both mean "on". */ prefs.leds_disabled = (leds_byte == LEDS_PREF_OFF) ? 1 : 0; - /* Migrate uninitialized backoff_multiplier (0.0 or NaN) to default */ - if (prefs.backoff_multiplier == 0.0f || prefs.backoff_multiplier != prefs.backoff_multiplier) { - prefs.backoff_multiplier = 0.2f; - } + /* The 0.0-or-NaN -> 0.2 coercion that used to live here is gone. It could + * not tell "field absent from an old file" from "the user set 0.0 to turn + * reactive backoff off", so the documented off switch never survived a + * reboot. Both cases are now handled properly: initNodePrefs() supplies + * 0.2 and a short-file fs_read() is a no-op that keeps it, while NaN and + * out-of-range values are caught by sanitizeNodePrefs(). */ LOG_INF("Loaded prefs from %s", path); LOG_DBG(" name='%s' freq=%.3f sf=%u bw=%.1f tx_pwr=%d", diff --git a/zephcore/helpers/CommonCLI.cpp b/zephcore/helpers/CommonCLI.cpp index 78b60dd..a5579fc 100644 --- a/zephcore/helpers/CommonCLI.cpp +++ b/zephcore/helpers/CommonCLI.cpp @@ -40,6 +40,79 @@ static uint32_t _atoi(const char* sp) { return n; } +/* ---- "default" keyword + strict numeric parsing for the `set` path ---- + * + * Bare atoi()/atof() fold every non-numeric string to 0, the word "default" + * included. On the knobs where 0 is itself legal and means "off" — + * probe.interval, cad.busycap, flood.max, rxduty — that silently switched the + * feature off and still answered OK. `set probe.interval default` disabling + * probing is the report that prompted this. + * + * Every `set` that has a default now takes the literal `default`, and rejects + * trailing garbage rather than turning it into a zero. */ + +/* The defaults are read out of initNodePrefs() itself rather than restated as + * constants here, so `set default` cannot drift from what a factory-fresh + * node actually boots with. File-scope statics (not a function-local one) to + * avoid emitting a __cxa_guard for the lazy init. */ +static NodePrefs s_cli_defaults; +static bool s_cli_defaults_ready = false; + +static const NodePrefs* cliDefaults() { + if (!s_cli_defaults_ready) { + initNodePrefs(&s_cli_defaults); + s_cli_defaults_ready = true; + } + return &s_cli_defaults; +} + +static const char* cliSkipSpace(const char* s) { + while (*s == ' ') s++; + return s; +} + +static bool cliIsDefault(const char* arg) { + const char* s = cliSkipSpace(arg); + if (strncmp(s, "default", 7) != 0) return false; + return *cliSkipSpace(s + 7) == '\0'; +} + +/* Integer argument, or "default". false = unparseable; the caller reports + * usage rather than acting on a silently-zeroed value. */ +static bool cliNum(const char* arg, long def_val, long* out) { + const char* s = cliSkipSpace(arg); + if (cliIsDefault(s)) { *out = def_val; return true; } + char* end = NULL; + long v = strtol(s, &end, 10); + if (end == s || *cliSkipSpace(end) != '\0') return false; + *out = v; + return true; +} + +/* Float argument, or "default". NaN is rejected here so range checks in the + * callers (every comparison against NaN is false) cannot pass it through. */ +static bool cliFloat(const char* arg, float def_val, float* out) { + const char* s = cliSkipSpace(arg); + if (cliIsDefault(s)) { *out = def_val; return true; } + char* end = NULL; + float v = strtof(s, &end); + if (end == s || *cliSkipSpace(end) != '\0') return false; + if (v != v) return false; + *out = v; + return true; +} + +/* on / off / 1 / 0 / default -> 1 or 0; -1 when unrecognised. Replaces the + * hand-rolled copy of this ladder in a dozen setters, each of which accepted a + * bare leading '0'/'1' and ignored whatever followed. */ +static int cliOnOff(const char* arg, int def_val) { + const char* s = cliSkipSpace(arg); + if (cliIsDefault(s)) return def_val; + if (strcmp(s, "on") == 0 || strcmp(s, "1") == 0) return 1; + if (strcmp(s, "off") == 0 || strcmp(s, "0") == 0) return 0; + return -1; +} + static bool isValidName(const char* n) { while (*n) { if (*n == '[' || *n == ']' || *n == '\\' || *n == ':' || @@ -503,8 +576,10 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch } else if (memcmp(command, "set ", 4) == 0) { const char* config = &command[4]; if (memcmp(config, "dutycycle ", 10) == 0) { - float dc = atof(&config[10]); - if (dc < 1 || dc > 100) { + float dc; + if (!cliFloat(&config[10], 100.0f / (cliDefaults()->airtime_factor + 1.0f), &dc)) { + strcpy(reply, "ERROR: dutycycle must be 1-100, or default"); + } else if (dc < 1 || dc > 100) { strcpy(reply, "ERROR: dutycycle must be 1-100"); } else { _prefs->airtime_factor = (100.0f / dc) - 1.0f; @@ -515,9 +590,14 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch snprintf(reply, CLI_REPLY_SIZE, "OK - %d.%d%%", a_int, a_frac); } } else if (memcmp(config, "af ", 3) == 0) { - _prefs->airtime_factor = atof(&config[3]); - savePrefs(); - strcpy(reply, "OK"); + float af; + if (!cliFloat(&config[3], cliDefaults()->airtime_factor, &af)) { + strcpy(reply, "Error: expected a number or default"); + } else { + _prefs->airtime_factor = af; + savePrefs(); + strcpy(reply, "OK"); + } } else if (memcmp(config, "int.thresh ", 11) == 0) { /* Companion runtime never reads this (getInterferenceThreshold is * only overridden in Repeater/RoomServer) — reject instead of a @@ -525,25 +605,22 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch if (strcmp(_callbacks->getRole(), "companion") == 0) { strcpy(reply, "Error: not supported on companion"); } else { - _prefs->interference_threshold = atoi(&config[11]); - savePrefs(); - strcpy(reply, "OK"); + long v; + if (!cliNum(&config[11], cliDefaults()->interference_threshold, &v)) { + strcpy(reply, "Error: expected a number or default"); + } else { + _prefs->interference_threshold = (uint8_t)v; + savePrefs(); + strcpy(reply, "OK"); + } } } else if (memcmp(config, "leds ", 5) == 0) { /* Master switch for every LED on the node: heartbeat, unread-message * and LoRa TX activity, plus the message and shutdown flashes. Not * the display backlight — that has its own UI brightness setting. */ - const char* val = &config[5]; - int on; - if (memcmp(val, "on", 2) == 0 || val[0] == '1') { - on = 1; - } else if (memcmp(val, "off", 3) == 0 || val[0] == '0') { - on = 0; - } else { - on = -1; - } + int on = cliOnOff(&config[5], cliDefaults()->leds_disabled ? 0 : 1); if (on < 0) { - strcpy(reply, "Error: must be on or off"); + strcpy(reply, "Error: must be on, off or default"); } else { _prefs->leds_disabled = on ? 0 : 1; zephcore_leds_set_disabled(_prefs->leds_disabled != 0); @@ -565,11 +642,13 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch mode = ZEPHCORE_BUZZER_ON; } else if (memcmp(val, "off", 3) == 0 || val[0] == '0') { mode = ZEPHCORE_BUZZER_OFF; + } else if (cliIsDefault(val)) { + mode = zephcore_buzzer_mode_from_prefs(cliDefaults()->buzzer_quiet); } else { mode = -1; } if (mode < 0) { - strcpy(reply, "Error: 0 (silent), 1 (sound+vib), 2 (vibrate) or 3 (sound)"); + strcpy(reply, "Error: 0 (silent), 1 (sound+vib), 2 (vibrate), 3 (sound) or default"); } else if ((mode == ZEPHCORE_BUZZER_VIBRATE || mode == ZEPHCORE_BUZZER_SOUND) && !zephcore_buzzer_has_vibrate()) { strcpy(reply, "Error: no vibration motor on this board - use 0 or 1"); @@ -591,17 +670,21 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch * existing node's prefs. */ strcpy(reply, "Removed - Automatic AGC reset is on"); } else if (memcmp(config, "cad.auto ", 9) == 0) { - if (memcmp(&config[9], "on", 2) == 0 || memcmp(&config[9], "off", 3) == 0) { - _prefs->cad_auto = (config[9] == 'o' && config[10] == 'n') ? 1 : 0; + int on = cliOnOff(&config[9], cliDefaults()->cad_auto); + if (on >= 0) { + _prefs->cad_auto = (uint8_t)on; _callbacks->applyCadPrefs(); savePrefs(); strcpy(reply, "OK"); } else { - strcpy(reply, "Error: must be on or off"); + strcpy(reply, "Error: must be on, off or default"); } } else if (memcmp(config, "cad.offset ", 11) == 0) { - int val = atoi(&config[11]); - if (val < CAD_OFFSET_MIN || val > CAD_OFFSET_MAX) { + long val; + if (!cliNum(&config[11], cliDefaults()->cad_offset, &val)) { + snprintf(reply, CLI_REPLY_SIZE, "Error: expected %d..%d or default", + CAD_OFFSET_MIN, CAD_OFFSET_MAX); + } else if (val < CAD_OFFSET_MIN || val > CAD_OFFSET_MAX) { snprintf(reply, CLI_REPLY_SIZE, "Error: offset range is %d..%d", CAD_OFFSET_MIN, CAD_OFFSET_MAX); } else { @@ -613,8 +696,10 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch /* Governs every periodic radio measurement, not just CAD — the * noise-floor sampler and the CAD probe share one reading. */ } else if (memcmp(config, "probe.interval ", 15) == 0) { - int val = atoi(&config[15]); - if (val != 0 && (val < 10 || val > 255)) { + long val; + if (!cliNum(&config[15], cliDefaults()->probe_interval, &val)) { + strcpy(reply, "Error: interval is 0 (probing off), 10-255 seconds, or default"); + } else if (val != 0 && (val < 10 || val > 255)) { strcpy(reply, "Error: interval is 0 (probing off) or 10-255 seconds"); } else { _prefs->probe_interval = (uint8_t)val; @@ -623,8 +708,10 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch strcpy(reply, "OK"); } } else if (memcmp(config, "cad.busycap ", 12) == 0) { - int val = atoi(&config[12]); - if (val != 0 && (val < 10 || val > 90)) { + long val; + if (!cliNum(&config[12], cliDefaults()->cad_busycap, &val)) { + strcpy(reply, "Error: busycap is 0 (off), 10-90 percent, or default"); + } else if (val != 0 && (val < 10 || val > 90)) { strcpy(reply, "Error: busycap is 0 (off) or 10-90 percent"); } else { _prefs->cad_busycap = (uint8_t)val; @@ -633,8 +720,20 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch strcpy(reply, "OK"); } } else if (memcmp(config, "cad.reset", 9) == 0) { + /* Clearing the probe statistics alone left the node re-converging + * FROM wherever the staircase had already walked detPeak, with no + * evidence left to justify sitting there — the opposite of a reset, + * and useless for the one job this command has (recovering after a + * base-table change). The operating offset lives in two places: + * _prefs->cad_offset, and _cad_offset inside the radio, which + * applyCadPrefs() reloads through setCadParams(). */ + _prefs->cad_offset = cliDefaults()->cad_offset; _callbacks->resetCadStats(); - strcpy(reply, "OK - CAD probe stats cleared"); + _callbacks->applyCadPrefs(); + savePrefs(); + snprintf(reply, CLI_REPLY_SIZE, + "OK - CAD probe stats cleared, detPeak offset reset to %d", + (int)_prefs->cad_offset); } else if (memcmp(config, "extra.sf ", 9) == 0) { /* LR2021 side detectors: up to 3 extra SFs received alongside * `sf`. "0" / "off" clears the set. The chip-side constraints @@ -662,32 +761,32 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch } } } else if (memcmp(config, "multi.acks ", 11) == 0) { - int val = atoi(&config[11]); - if (val == 0 || val == 1) { + long val; + if (cliNum(&config[11], cliDefaults()->multi_acks, &val) && + (val == 0 || val == 1)) { _prefs->multi_acks = (uint8_t)val; savePrefs(); strcpy(reply, "OK"); } else { - strcpy(reply, "Error: must be 0 or 1"); + strcpy(reply, "Error: must be 0, 1 or default"); } #ifdef CONFIG_ZEPHCORE_ROLE_ROOM_SERVER /* Room server only -- see the matching guard on the `get` side. */ } else if (memcmp(config, "allow.read.only ", 16) == 0) { - if (memcmp(&config[16], "on", 2) == 0) { - _prefs->allow_read_only = 1; - savePrefs(); - strcpy(reply, "OK"); - } else if (memcmp(&config[16], "off", 3) == 0) { - _prefs->allow_read_only = 0; + int on = cliOnOff(&config[16], cliDefaults()->allow_read_only); + if (on >= 0) { + _prefs->allow_read_only = (uint8_t)on; savePrefs(); strcpy(reply, "OK"); } else { - strcpy(reply, "Error: must be on or off"); + strcpy(reply, "Error: must be on, off or default"); } #endif } else if (memcmp(config, "flood.advert.interval ", 22) == 0) { - int hours = _atoi(&config[22]); - if ((hours > 0 && hours < 3) || (hours > 168)) { + long hours; + if (!cliNum(&config[22], cliDefaults()->flood_advert_interval, &hours)) { + strcpy(reply, "Error: expected 0, 3-168 hours, or default"); + } else if ((hours > 0 && hours < 3) || (hours > 168)) { strcpy(reply, "Error: interval range is 3-168 hours"); } else { _prefs->flood_advert_interval = (uint8_t)hours; @@ -696,8 +795,12 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch strcpy(reply, "OK"); } } else if (memcmp(config, "advert.interval ", 16) == 0) { - int mins = _atoi(&config[16]); - if ((mins > 0 && mins < MIN_LOCAL_ADVERT_INTERVAL) || (mins > 240)) { + long mins; + if (!cliNum(&config[16], (long)cliDefaults()->advert_interval * 2, &mins)) { + snprintf(reply, CLI_REPLY_SIZE, + "Error: expected 0, %d-240 minutes, or default", + MIN_LOCAL_ADVERT_INTERVAL); + } else if ((mins > 0 && mins < MIN_LOCAL_ADVERT_INTERVAL) || (mins > 240)) { snprintf(reply, CLI_REPLY_SIZE, "Error: interval range is %d-240 minutes", MIN_LOCAL_ADVERT_INTERVAL); } else { _prefs->advert_interval = (uint8_t)(mins / 2); @@ -745,10 +848,14 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch snprintf(tmp, sizeof(tmp), "%.*s", (int)(sizeof(tmp) - 1), &config[6]); const char* parts[4]; int num = mesh::Utils::parseTextParts(tmp, parts, 4); - float freq = num > 0 ? strtof(parts[0], nullptr) : 0.0f; - float bw = num > 1 ? strtof(parts[1], nullptr) : 0.0f; - uint8_t sf = num > 2 ? atoi(parts[2]) : 0; - uint8_t cr = num > 3 ? atoi(parts[3]) : 0; + /* "set radio default" restores all four together — they are one + * interop-critical set and resetting them piecemeal can leave a + * node on a combination no other node uses. */ + bool want_def = (num == 1 && cliIsDefault(parts[0])); + float freq = want_def ? cliDefaults()->freq : (num > 0 ? strtof(parts[0], nullptr) : 0.0f); + float bw = want_def ? cliDefaults()->bw : (num > 1 ? strtof(parts[1], nullptr) : 0.0f); + uint8_t sf = want_def ? cliDefaults()->sf : (num > 2 ? (uint8_t)atoi(parts[2]) : 0); + uint8_t cr = want_def ? cliDefaults()->cr : (num > 3 ? (uint8_t)atoi(parts[3]) : 0); if (freq >= 150.0f && freq <= 2500.0f && sf >= 5 && sf <= 12 && cr >= 5 && cr <= 8 && bw >= 7.0f && bw <= 500.0f) { /* Snapshot old params, then mutate _prefs and save so later @@ -768,53 +875,59 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch _callbacks->freezeRadioParams(old_freq, old_bw, old_sf, old_cr); strcpy(reply, "OK - reboot to apply"); } else { - strcpy(reply, "Error: freq 150-2500, bw 7-500, sf 5-12, cr 5-8"); + strcpy(reply, "Error: freq 150-2500, bw 7-500, sf 5-12, cr 5-8, or default"); } } else if (memcmp(config, "lat ", 4) == 0) { - _prefs->node_lat = atof(&config[4]); + _prefs->node_lat = cliIsDefault(&config[4]) ? cliDefaults()->node_lat + : atof(&config[4]); savePrefs(); strcpy(reply, "OK"); } else if (memcmp(config, "lon ", 4) == 0) { - _prefs->node_lon = atof(&config[4]); + _prefs->node_lon = cliIsDefault(&config[4]) ? cliDefaults()->node_lon + : atof(&config[4]); savePrefs(); strcpy(reply, "OK"); } else if (memcmp(config, "rxdelay ", 8) == 0) { - _prefs->rx_delay_base = atof(&config[8]); + _prefs->rx_delay_base = cliIsDefault(&config[8]) ? cliDefaults()->rx_delay_base + : (float)atof(&config[8]); savePrefs(); strcpy(reply, "OK (ignored: rxdelay is now adaptive)"); } else if (memcmp(config, "txdelay ", 8) == 0) { - _prefs->tx_delay_factor = atof(&config[8]); + _prefs->tx_delay_factor = cliIsDefault(&config[8]) ? cliDefaults()->tx_delay_factor + : (float)atof(&config[8]); savePrefs(); strcpy(reply, "OK (ignored: txdelay is now adaptive)"); } else if (memcmp(config, "flood.max.advert ", 17) == 0) { - int m = atoi(&config[17]); - if (m >= 0 && m <= 64) { + long m; + if (cliNum(&config[17], cliDefaults()->flood_max_advert, &m) && m >= 0 && m <= 64) { _prefs->flood_max_advert = (uint8_t)m; savePrefs(); strcpy(reply, "OK"); } else { - strcpy(reply, "Error: range 0-64"); + strcpy(reply, "Error: range 0-64, or default"); } } else if (memcmp(config, "flood.max.unscoped ", 19) == 0) { - int m = atoi(&config[19]); - if (m >= 0 && m <= 64) { + long m; + if (cliNum(&config[19], cliDefaults()->flood_max_unscoped, &m) && m >= 0 && m <= 64) { _prefs->flood_max_unscoped = (uint8_t)m; savePrefs(); strcpy(reply, "OK"); } else { - strcpy(reply, "Error: range 0-64"); + strcpy(reply, "Error: range 0-64, or default"); } } else if (memcmp(config, "flood.max ", 10) == 0) { - int m = atoi(&config[10]); - if (m >= 0 && m <= 64) { + long m; + if (cliNum(&config[10], cliDefaults()->flood_max, &m) && m >= 0 && m <= 64) { _prefs->flood_max = (uint8_t)m; savePrefs(); strcpy(reply, "OK"); } else { - strcpy(reply, "Error: range 0-64"); + strcpy(reply, "Error: range 0-64, or default"); } } else if (memcmp(config, "direct.txdelay ", 15) == 0) { - _prefs->direct_tx_delay_factor = atof(&config[15]); + _prefs->direct_tx_delay_factor = cliIsDefault(&config[15]) + ? cliDefaults()->direct_tx_delay_factor + : (float)atof(&config[15]); savePrefs(); strcpy(reply, "OK (ignored: direct.txdelay is now adaptive)"); } else if (memcmp(config, "backoff.multiplier ", 19) == 0) { @@ -824,8 +937,14 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch if (strcmp(_callbacks->getRole(), "companion") == 0) { strcpy(reply, "Error: not supported on companion"); } else { - float f = atof(&config[19]); - if (f >= 0.0f && f <= 2.0f) { + /* initNodePrefs() leaves this at 0.0 (memset) even though the + * live default is ContentionTracker::DEFAULT_BACKOFF_MULT and + * RepeaterDataStore migrates 0.0 -> 0.2 on load, so `default` + * uses the real value rather than the prefs blank. */ + float f; + if (!cliFloat(&config[19], 0.2f, &f)) { + strcpy(reply, "Error, range 0.0-2.0, or default"); + } else if (f >= 0.0f && f <= 2.0f) { _prefs->backoff_multiplier = f; _callbacks->setBackoffMultiplier(f); savePrefs(); @@ -846,13 +965,14 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch strcpy(reply, "OK"); } else if (memcmp(config, "path.hash.mode ", 15) == 0) { config += 15; - uint8_t mode = atoi(config); - if (mode < 3) { - _prefs->path_hash_mode = mode; + long mode; + if (cliNum(config, cliDefaults()->path_hash_mode, &mode) && + mode >= 0 && mode < 3) { + _prefs->path_hash_mode = (uint8_t)mode; savePrefs(); strcpy(reply, "OK"); } else { - strcpy(reply, "Error, must be 0,1, or 2"); + strcpy(reply, "Error, must be 0, 1, 2 or default"); } } else if (memcmp(config, "loop.detect ", 12) == 0) { /* Loop detection runs only in the Repeater/RoomServer forward @@ -871,9 +991,11 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch mode = LOOP_DETECT_MODERATE; } else if (memcmp(config, "strict", 6) == 0) { mode = LOOP_DETECT_STRICT; + } else if (cliIsDefault(config)) { + mode = cliDefaults()->loop_detect; } else { mode = 0xFF; - strcpy(reply, "Error, must be: off, minimal, moderate, or strict"); + strcpy(reply, "Error, must be: off, minimal, moderate, strict, or default"); } if (mode != 0xFF) { _prefs->loop_detect = mode; @@ -881,14 +1003,14 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch strcpy(reply, "OK"); } } else if (memcmp(config, "tx ", 3) == 0) { - char *end = nullptr; - long parsed = strtol(&config[3], &end, 10); + long parsed; int max_tx = 30; #ifdef CONFIG_ZEPHCORE_MAX_TX_POWER_DBM max_tx = CONFIG_ZEPHCORE_MAX_TX_POWER_DBM; #endif - if (end == &config[3] || *end != '\0' || parsed < -9 || parsed > max_tx) { - snprintf(reply, CLI_REPLY_SIZE, "Error: range -9 to %d dBm", max_tx); + if (!cliNum(&config[3], cliDefaults()->tx_power_dbm, &parsed) || + parsed < -9 || parsed > max_tx) { + snprintf(reply, CLI_REPLY_SIZE, "Error: range -9 to %d dBm, or default", max_tx); } else { _prefs->tx_power_dbm = (int8_t)parsed; savePrefs(); @@ -897,8 +1019,9 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch (int)_prefs->tx_power_dbm); } } else if (sender_timestamp == 0 && memcmp(config, "freq ", 5) == 0) { - float f = atof(&config[5]); - if (f >= 150.0f && f <= 2500.0f) { + float f; + if (cliFloat(&config[5], cliDefaults()->freq, &f) && + f >= 150.0f && f <= 2500.0f) { float old_freq = _prefs->freq; _prefs->freq = f; savePrefs(); @@ -909,7 +1032,7 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch _callbacks->freezeRadioParams(old_freq, _prefs->bw, _prefs->sf, _prefs->cr); strcpy(reply, "OK - reboot to apply"); } else { - strcpy(reply, "Error: range 150-2500 MHz"); + strcpy(reply, "Error: range 150-2500 MHz, or default"); } } else if (strcmp(config, "adc.multiplier target") == 0) { strcpy(reply, "Error: need mV target (e.g. set adc.multiplier target 4173)"); @@ -960,15 +1083,15 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch } } else if (memcmp(config, "adc.multiplier ", 15) == 0) { const char *arg = &config[15]; - float val = atof(arg); - /* Reject non-numeric, NaN, inf, negative, and out-of-range values. - * 0 is valid (resets to DTS default). Upper bound covers all real - * divider/reference combinations with margin. */ - bool bad = (val != 0.0f && val < 100.0f) || val > 30000.0f || val < 0.0f; - /* atof returns 0 for non-numeric strings — distinguish from literal "0" */ - if (val == 0.0f && arg[0] != '0') bad = true; + /* 0 is valid (resets to the DTS default). cliFloat() rejects the + * non-numeric input that atof() used to fold into a 0 here, so the + * old "distinguish from literal 0" dance is gone. Upper bound + * covers all real divider/reference combinations with margin. */ + float val; + bool bad = !cliFloat(arg, cliDefaults()->adc_multiplier, &val); + if (!bad) bad = (val != 0.0f && val < 100.0f) || val > 30000.0f || val < 0.0f; if (bad) { - strcpy(reply, "Error: invalid multiplier (0 to reset, or 100-30000)"); + strcpy(reply, "Error: invalid multiplier (0 to reset, 100-30000, or default)"); } else if (_board->setAdcMultiplier(val)) { _prefs->adc_multiplier = val; savePrefs(); @@ -981,11 +1104,7 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch strcpy(reply, "Error: unsupported by this board"); } } else if (memcmp(config, "radio.fem.rxgain ", 17) == 0) { - const char* arg = &config[17]; - int val = -1; - if (memcmp(arg, "on", 2) == 0) val = 1; - else if (memcmp(arg, "off", 3) == 0) val = 0; - else if (arg[0] == '0' || arg[0] == '1') val = atoi(arg); + int val = cliOnOff(&config[17], cliDefaults()->fem_rxgain); if (val == 0 || val == 1) { /* Same shape as radio.rxgain: always save, then apply live and * report when the radio driver has no FEM gate. */ @@ -1000,11 +1119,7 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch strcpy(reply, "Error: must be 0, 1, on, or off"); } } else if (memcmp(config, "radio.rxgain ", 13) == 0) { - const char* arg = &config[13]; - int val = -1; - if (memcmp(arg, "on", 2) == 0) val = 1; - else if (memcmp(arg, "off", 3) == 0) val = 0; - else if (arg[0] == '0' || arg[0] == '1') val = atoi(arg); + int val = cliOnOff(&config[13], cliDefaults()->rx_boost); if (val == 0 || val == 1) { /* Always save (upstream f3d4d8cd), then apply live and * report when the radio has no RX boost feature. */ @@ -1019,11 +1134,7 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch strcpy(reply, "Error: must be 0, 1, on, or off"); } } else if (memcmp(config, "rxduty ", 7) == 0) { - const char* arg = &config[7]; - int val = -1; - if (memcmp(arg, "on", 2) == 0) val = 1; - else if (memcmp(arg, "off", 3) == 0) val = 0; - else if (arg[0] == '0' || arg[0] == '1') val = atoi(arg); + int val = cliOnOff(&config[7], cliDefaults()->rx_duty_cycle); if (val == 0 || val == 1) { _prefs->rx_duty_cycle = (uint8_t)val; savePrefs(); @@ -1035,11 +1146,7 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch // Rotate the panel 180 degrees for cases that mount it upside down. // Applied immediately -- the SSD1306/SH1106 remap is two bytes on // the wire and the next frame comes out flipped, no redraw needed. - const char* arg = &config[15]; - int val = -1; - if (memcmp(arg, "on", 2) == 0) val = 1; - else if (memcmp(arg, "off", 3) == 0) val = 0; - else if (arg[0] == '0' || arg[0] == '1') val = atoi(arg); + int val = cliOnOff(&config[15], cliDefaults()->display_rotate); if (val != 0 && val != 1) { strcpy(reply, "Error: must be 0, 1, on, or off"); } else { @@ -1065,11 +1172,7 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch // Deliberately separate from display.rotate: a case can flip the // screen without flipping the stick, and boards whose panel cannot // rotate can still need the axis swap. - const char* arg = &config[13]; - int val = -1; - if (memcmp(arg, "on", 2) == 0) val = 1; - else if (memcmp(arg, "off", 3) == 0) val = 0; - else if (arg[0] == '0' || arg[0] == '1') val = atoi(arg); + int val = cliOnOff(&config[13], cliDefaults()->input_rotate); if (val == 0 || val == 1) { zephcore_input_set_flipped(val == 1); _prefs->input_rotate = (uint8_t)val; @@ -1081,12 +1184,8 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch } else if (memcmp(config, "gps diag", 8) == 0) { // set gps diag <0|1|on|off> — arm module-configuration reporting. // Not persisted: clears on reboot, by design. - const char* arg = config + 8; - while (*arg == ' ') arg++; - int val = -1; - if (memcmp(arg, "on", 2) == 0) val = 1; - else if (memcmp(arg, "off", 3) == 0) val = 0; - else if (arg[0] == '0' || arg[0] == '1') val = atoi(arg); + /* Not persisted, so "default" means the boot state: off. */ + int val = cliOnOff(config + 8, 0); if (val == 0 || val == 1) { gps_set_diag(val == 1); if (val == 1) { @@ -1127,19 +1226,15 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch else snprintf(reply, CLI_REPLY_SIZE, "OK - gps duty=%u s", (unsigned)val); } } else if (memcmp(config, "meshtimesync ", 13) == 0) { - const char* arg = &config[13]; + int on = cliOnOff(&config[13], cliDefaults()->meshtimesync); if (_callbacks->getMeshTimeSync() == nullptr) { strcpy(reply, "not available"); - } else if (memcmp(arg, "on", 2) == 0) { - _prefs->meshtimesync = 1; - savePrefs(); - strcpy(reply, "OK - meshtimesync on"); - } else if (memcmp(arg, "off", 3) == 0) { - _prefs->meshtimesync = 0; - savePrefs(); - strcpy(reply, "OK - meshtimesync off"); + } else if (on < 0) { + strcpy(reply, "Error: must be on, off or default"); } else { - strcpy(reply, "Error: must be on or off"); + _prefs->meshtimesync = (uint8_t)on; + savePrefs(); + snprintf(reply, CLI_REPLY_SIZE, "OK - meshtimesync %s", on ? "on" : "off"); } } else { snprintf(reply, CLI_REPLY_SIZE, "unknown config: %.230s", config); diff --git a/zephcore/helpers/NodePrefs.h b/zephcore/helpers/NodePrefs.h index 68ff6b6..b54d264 100644 --- a/zephcore/helpers/NodePrefs.h +++ b/zephcore/helpers/NodePrefs.h @@ -106,7 +106,7 @@ struct NodePrefs { uint8_t _reserved_apc_enabled; uint8_t _reserved_apc_margin; uint8_t meshtimesync; // 1 = mesh time-sync clock correction on (default off) - uint8_t cad_auto; // 1 = adaptive-CAD staircase acts on probe stats (default off = dry-run) + uint8_t cad_auto; // 1 = adaptive-CAD staircase acts on probe stats (default ON) int8_t cad_offset; // operating detPeak offset from family base (-4..4) uint8_t probe_interval; // seconds between periodic radio measurements: // one noise-floor sample, and the CAD probe that @@ -202,6 +202,11 @@ static inline void sanitizeNodePrefs(NodePrefs* p) { p->airtime_factor = sanePrefFloat(p->airtime_factor, 0.0f, 9.0f, 9.0f); p->rx_delay_base = sanePrefFloat(p->rx_delay_base, 0.0f, 3600.0f, 0.0f); p->adc_multiplier = sanePrefFloat(p->adc_multiplier, 0.0f, 30000.0f, 0.0f); + /* Range matches the CLI (0.0-2.0). 0.0 is a legal value meaning "reactive + * backoff off" and must survive: this guard exists for NaN and corruption + * only. RepeaterDataStore used to coerce 0.0 -> 0.2 here, which silently + * undid the documented way to disable the feature on every boot. */ + p->backoff_multiplier = sanePrefFloat(p->backoff_multiplier, 0.0f, 2.0f, 0.2f); p->node_lat = sanePrefFloat(p->node_lat, -90.0, 90.0, 0.0); p->node_lon = sanePrefFloat(p->node_lon, -180.0, 180.0, 0.0); @@ -226,7 +231,10 @@ static inline void sanitizeNodePrefs(NodePrefs* p) { p->rx_duty_cycle = saneBool(p->rx_duty_cycle, 0); p->leds_disabled = saneBool(p->leds_disabled, 0); p->meshtimesync = saneBool(p->meshtimesync, 0); - p->cad_auto = saneBool(p->cad_auto, 0); + /* Fallback must be the initNodePrefs() default (ON). It was 0, so a byte + * that was neither 0 nor 1 silently switched adaptive CAD off instead of + * restoring the default — left over from when the default was dry-run. */ + p->cad_auto = saneBool(p->cad_auto, 1); p->allow_read_only = saneBool(p->allow_read_only, 0); p->powersaving_enabled = saneBool(p->powersaving_enabled, 0); p->display_rotate = saneBool(p->display_rotate, 0); @@ -249,6 +257,7 @@ static inline void sanitizeNodePrefs(NodePrefs* p) { if (p->loop_detect > LOOP_DETECT_STRICT) p->loop_detect = LOOP_DETECT_MINIMAL; if (p->path_hash_mode > 2) p->path_hash_mode = 0; p->autoadd_max_hops = clampPref(p->autoadd_max_hops, 0, 64); + p->flood_max = clampPref(p->flood_max, 0, 64); p->flood_max_unscoped = clampPref(p->flood_max_unscoped, 0, 64); p->flood_max_advert = clampPref(p->flood_max_advert, 0, 64); p->cad_busycap = clampPref(p->cad_busycap, 0, 90); @@ -304,6 +313,12 @@ static inline void initNodePrefs(NodePrefs* prefs) { prefs->advert_interval = 0; // 0 = periodic local advert off; else minutes = value * 2 prefs->flood_advert_interval = 47; // hours prefs->rx_delay_base = 0.0f; + /* Must match mesh::ContentionTracker::DEFAULT_BACKOFF_MULT. Left at the + * memset 0 (= backoff disabled) until now, with RepeaterDataStore coercing + * 0.0 -> 0.2 on load to paper over it — which also undid a deliberate 0.0. + * Companions never apply this (only Repeater/RoomServer call + * setBackoffMultiplier), so setting it here changes no existing node. */ + prefs->backoff_multiplier = 0.2f; prefs->tx_delay_factor = 0.5f; prefs->direct_tx_delay_factor = 0.3f; prefs->allow_read_only = 0; @@ -335,4 +350,17 @@ static inline void initNodePrefs(NodePrefs* prefs) { prefs->input_rotate = 0; // Default OFF — joystick axes as the board wires them prefs->v_contact_enabled = 1; // Default ON — v-contact loopback admin chat (companion) prefs->v_battery_alert_mv = 0xFFFF; // Sentinel: derive from board auto-shutdown threshold + /* Companion-only feature, and main_companion.cpp used to assign this by + * hand right after calling us — so no node ever ran without it. It lives + * here now because a default listed only at one call site is invisible to + * every other caller of initNodePrefs(), which is the exact drift that + * zeroed probe_interval and cad_auto in the past. Matches what + * loadPrefs()'s absent-field fallback and sanitizeNodePrefs() already use. + * 0 means disabled and is a legal stored value, so sanitize passes it + * through untouched — this default only applies to a fresh prefs struct. */ +#ifdef CONFIG_ZEPHCORE_AUTO_SHUTDOWN_MILLIVOLTS + prefs->auto_shutdown_mv = CONFIG_ZEPHCORE_AUTO_SHUTDOWN_MILLIVOLTS; +#else + prefs->auto_shutdown_mv = 0; +#endif } diff --git a/zephcore/src/main_companion.cpp b/zephcore/src/main_companion.cpp index 73ebdf0..1752c0e 100644 --- a/zephcore/src/main_companion.cpp +++ b/zephcore/src/main_companion.cpp @@ -1446,8 +1446,12 @@ int main(void) * loadPrefs then keeps that 0 instead of the real default (this is what * zeroed probe_interval / cad_auto and, earlier, the GPS settings). */ initNodePrefs(&companion_mesh.prefs); - /* Companion-specific overrides vs. initNodePrefs defaults: */ - companion_mesh.prefs.auto_shutdown_mv = CONFIG_ZEPHCORE_AUTO_SHUTDOWN_MILLIVOLTS; /* low-batt cutoff (0=off) */ + /* Companion-specific overrides vs. initNodePrefs defaults. auto_shutdown_mv + * used to be set here too; it now comes from initNodePrefs() itself, which + * is what the comment above asks for — a value listed only here is invisible + * to every other caller of initNodePrefs(). gps_interval stays because + * initNodePrefs() hardcodes the 300 s companion figure while repeaters run a + * much longer duty; this line is how a board's Kconfig value wins. */ companion_mesh.prefs.gps_interval = CONFIG_ZEPHCORE_GPS_POLL_INTERVAL_SEC; /* 5-min duty cycle (0=always-on) */ /* Load prefs from storage */