From 3d5fb9461c7f6fe0a633b6222b7c11e198bb332a Mon Sep 17 00:00:00 2001 From: Dan Theisen Date: Sat, 19 Sep 2026 10:20:26 +0000 Subject: [PATCH] Fix GPS duty interval read before loadPrefs() on repeater/room server main_repeater.cpp and main_room_server.cpp applied the GPS duty interval ~55 lines before data_store.loadPrefs() ran, so getNodePrefs()->gps_interval still held the initNodePrefs() value of 300 s. That 300 is the *companion* default. loadPrefs() is not a pure read on these roles: its first-boot path is the only place the 48 h repeater/room-server default is established, and it persists that to flash so later code can treat flash as authoritative. A stored value is likewise only in the struct after loadPrefs() returns. Reading gps_interval before that call therefore cannot see either the role default or the operator's setting -- only the companion constant. Two consequences, on every boot: - A stock GPS-equipped repeater duty-cycled its GPS every 5 minutes instead of every 48 hours, ~576x more wakeups than intended. On battery nodes this is a straight power regression. - Any operator value never reached the hardware. "set gps duty 0" (always-on) persisted to flash correctly, and `get gps duty` reports the manager's now-effective value, so it answered 300 -- readable as a stale default rather than as the setting being ignored. The GPS manager itself is correct: gps_start_acquiring(), the fix-done callback, gps_enable() and the fresh-fix request all guard standby on gps_duty_cycling(), so the always-on invariant holds once the manager is actually told the interval is zero. Same ordering class as the two bugs already documented in these files -- radio frequency binding and the LED master switch -- where CLI readback looked right and the hardware disagreed. main_companion.cpp has always had the correct order (loadPrefs() at 1461, gps_set_poll_interval_sec() at 1616) and is unaffected. Move the interval and the mode switch that starts acquisition to just after loadPrefs(). Callback registration stays where it was: it has no prefs dependency and must precede acquisition. Co-Authored-By: Claude Opus 5 (1M context) --- zephcore/src/main_repeater.cpp | 30 +++++++++++++++++++++++++----- zephcore/src/main_room_server.cpp | 30 +++++++++++++++++++++++++----- 2 files changed, 50 insertions(+), 10 deletions(-) diff --git a/zephcore/src/main_repeater.cpp b/zephcore/src/main_repeater.cpp index a6ab9e7..c25d096 100644 --- a/zephcore/src/main_repeater.cpp +++ b/zephcore/src/main_repeater.cpp @@ -695,14 +695,11 @@ int main(void) } } - /* Set GPS to repeater mode: power off now, wake every 48h for time sync only. - * This prevents GPS from draining power on boards that have it (e.g., Wio Tracker). */ + /* GPS callbacks only. The duty interval and the mode switch that starts + * acquisition are applied after loadPrefs() below — see the block there. */ if (gps_is_available()) { gps_set_fix_callback(gps_fix_callback); gps_set_event_callback(gps_event_callback); - /* Apply persisted GPS duty interval (repeater default 48h; 0 = always on) */ - gps_set_poll_interval_sec(repeater_mesh.getNodePrefs()->gps_interval); - gps_set_repeater_mode(true); } /* Initialize UI (display + buttons). Shows splash screen, then auto- @@ -758,6 +755,29 @@ int main(void) data_store.loadPrefs(*repeater_mesh.getNodePrefs()); lora_radio.setPrefs(repeater_mesh.getNodePrefs()); + /* Set GPS to repeater mode: power off now, wake every duty interval for + * time sync only. This prevents GPS from draining power on boards that + * have it (e.g., Wio Tracker). + * + * MUST come after loadPrefs(), for the same reason as the LED switch + * below -- but note loadPrefs() is not a pure read here. It is also the + * only place this role's GPS default is established: initNodePrefs() + * sets gps_interval=300 (the companion value), and the 48 h default is + * applied and PERSISTED by loadPrefs()'s first-boot path. + * + * This block used to sit ~55 lines earlier, where getNodePrefs() still + * held that 300. So the GPS manager was armed from the companion default + * on every boot and nothing re-applied the real value afterwards: + * - a stock repeater woke its GPS every 5 min instead of every 48 h; + * - any operator value, notably "set gps duty 0" (always-on), was + * stored and read back correctly but never reached the hardware. + * Same failure shape as the radio-frequency and LED ordering bugs in this + * file. main_companion.cpp has always had this order right. */ + if (gps_is_available()) { + gps_set_poll_interval_sec(repeater_mesh.getNodePrefs()->gps_interval); + gps_set_repeater_mode(true); + } + /* Apply the persisted LED master switch ("set leds on|off"). MUST come * after loadPrefs(): this used to sit just after ui_init(), ~45 lines * earlier, where getNodePrefs() still held the initNodePrefs() default of diff --git a/zephcore/src/main_room_server.cpp b/zephcore/src/main_room_server.cpp index 0c7718e..612f75d 100644 --- a/zephcore/src/main_room_server.cpp +++ b/zephcore/src/main_room_server.cpp @@ -581,14 +581,11 @@ int main(void) } } - /* Set GPS to repeater mode: power off now, wake every 48h for time sync only. - * This prevents GPS from draining power on boards that have it (e.g., Wio Tracker). */ + /* GPS callbacks only. The duty interval and the mode switch that starts + * acquisition are applied after loadPrefs() below — see the block there. */ if (gps_is_available()) { gps_set_fix_callback(gps_fix_callback); gps_set_event_callback(gps_event_callback); - /* Apply persisted GPS duty interval (repeater default 48h; 0 = always on) */ - gps_set_poll_interval_sec(room_mesh.getNodePrefs()->gps_interval); - gps_set_repeater_mode(true); } /* Initialize UI (display + buttons). Shows splash screen, then auto- @@ -643,6 +640,29 @@ int main(void) data_store.loadPrefs(*room_mesh.getNodePrefs()); lora_radio.setPrefs(room_mesh.getNodePrefs()); + /* Set GPS to repeater mode: power off now, wake every duty interval for + * time sync only. This prevents GPS from draining power on boards that + * have it (e.g., Wio Tracker). + * + * MUST come after loadPrefs(), for the same reason as the LED switch + * below -- but note loadPrefs() is not a pure read here. It is also the + * only place this role's GPS default is established: initNodePrefs() + * sets gps_interval=300 (the companion value), and the 48 h default is + * applied and PERSISTED by loadPrefs()'s first-boot path. + * + * This block used to sit ~55 lines earlier, where getNodePrefs() still + * held that 300. So the GPS manager was armed from the companion default + * on every boot and nothing re-applied the real value afterwards: + * - a stock repeater woke its GPS every 5 min instead of every 48 h; + * - any operator value, notably "set gps duty 0" (always-on), was + * stored and read back correctly but never reached the hardware. + * Same failure shape as the radio-frequency and LED ordering bugs in this + * file. main_companion.cpp has always had this order right. */ + if (gps_is_available()) { + gps_set_poll_interval_sec(room_mesh.getNodePrefs()->gps_interval); + gps_set_repeater_mode(true); + } + /* Apply the persisted LED master switch ("set leds on|off"). MUST come * after loadPrefs(): this used to sit just after ui_init(), ~45 lines * earlier, where getNodePrefs() still held the initNodePrefs() default of