Merge branch 'feat/clean-up-merge' into restore/upstream-duty-cycle

MQTT_INTERNALS.md

@@ -47,7 +47,11 @@ Remaining integration points in upstream files:
 bytes of a legacy (headerless) file, whose payload begins with the `mqtt_origin`
 string. Bump `MQTT_PREFS_VERSION` when the payload layout changes incompatibly; a file
 whose version this firmware doesn't recognize is left untouched and the in-memory prefs
-fall back to defaults (no downgrade, no misread).
+fall back to defaults (no downgrade, no misread). `saveMQTTPrefs()` also refuses to
+write while such a file is present (`_mqtt_prefs_hold`), so a `set` command after a
+firmware downgrade can't clobber the newer config — observer settings changed in that
+state simply don't persist. The frozen legacy layouts are pinned with `static_assert`s
+in `CommonCLI.h`, so every target build re-verifies the fleet's file offsets.
 
 Adding a field to the current version stays backward compatible: append it to the end
 of `MQTTPrefs`. An older, shorter payload still loads and the missing tail keeps its

src/helpers/AlertReporter.cpp

@@ -206,7 +206,7 @@ void AlertReporter::formatAge(unsigned long age_ms, char* out, size_t out_size)
 }
 
 void AlertReporter::onLoop(unsigned long now_ms) {
-  if (!_prefs || !_obs->alert_enabled) return;
+  if (!_prefs || !_obs || !_obs->alert_enabled) return;
   if (!_mesh) return;
 
   // Throttle: ~5 s cadence. The thresholds are minutes-scale so this is fine.

src/helpers/CommonCLI.cpp

@@ -395,6 +395,7 @@ static File openMqttPrefsRead(FILESYSTEM* fs) {
 void CommonCLI::loadMQTTPrefs(FILESYSTEM* fs) {
   // Initialize with defaults first
   setMQTTPrefsDefaults(&_mqtt_prefs);
+  _mqtt_prefs_hold = false;
 
   // Whether the loaded /mqtt_prefs already contained the observer fields (snmp/
   // watchdog/alert) appended in Phase 2 — if not, they may be carried over from an
@@ -428,8 +429,10 @@ void CommonCLI::loadMQTTPrefs(FILESYSTEM* fs) {
             }
           } else {
             // Unknown (newer) version: don't risk misreading a layout we don't know.
-            // Keep defaults for this boot and leave the file untouched (no downgrade).
-            MESH_DEBUG_PRINTLN("MQTT: /mqtt_prefs version unsupported, using defaults");
+            // Keep defaults for this boot and hold the file so later savePrefs()
+            // calls can't overwrite the newer config (no downgrade).
+            _mqtt_prefs_hold = true;
+            MESH_DEBUG_PRINTLN("MQTT: /mqtt_prefs version unsupported, using defaults (file preserved)");
           }
         }
       }
@@ -628,6 +631,13 @@ void CommonCLI::loadMQTTPrefs(FILESYSTEM* fs) {
 }
 
 void CommonCLI::saveMQTTPrefs(FILESYSTEM* fs) {
+  if (_mqtt_prefs_hold) {
+    // /mqtt_prefs was written by newer firmware; overwriting it here (v1 header +
+    // this boot's defaults) would destroy that config. Observer settings changed
+    // this boot are not persisted until current-or-older firmware is flashed.
+    MESH_DEBUG_PRINTLN("MQTT: /mqtt_prefs from newer firmware, not overwriting");
+    return;
+  }
 #if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM)
   fs->remove("/mqtt_prefs");
   File file = fs->open("/mqtt_prefs", FILE_O_WRITE);

src/helpers/CommonCLI.h

@@ -242,6 +242,17 @@ struct Legacy6SlotMQTTPrefs {
   char mqtt_ntp_server[64];
 };
 
+// The legacy layouts above describe files already written to the deployed fleet's
+// flash, so their sizes are frozen forever — loadMQTTPrefs() tells the eras apart
+// by file size and reads each file as a raw struct dump. These asserts pin the
+// layouts on every target toolchain; if one fires, the compiler (or an edit to a
+// legacy struct or MAX_MQTT_SLOTS) has changed a layout and fleet files would be
+// read at wrong offsets.
+static_assert(sizeof(MQTTPrefsHeader) == 8, "versioned /mqtt_prefs header must stay 8 bytes");
+static_assert(sizeof(OldMQTTPrefs) == 472, "frozen pre-slot /mqtt_prefs layout changed");
+static_assert(sizeof(ThreeSlotMQTTPrefs) == 1464, "frozen 3-slot /mqtt_prefs layout changed");
+static_assert(sizeof(Legacy6SlotMQTTPrefs) == 2904, "frozen deployed-fleet /mqtt_prefs layout changed");
+
 // Observer settings captured from the trailing block of an old-format /com_prefs
 // (fork firmware that predates the NodePrefs -> MQTTPrefs split). loadPrefsInt()
 // fills this in when it detects the old file layout; loadMQTTPrefs() then applies
@@ -373,6 +384,10 @@ class CommonCLI {
 #ifdef WITH_MQTT_BRIDGE
   MQTTPrefs _mqtt_prefs;
   LegacyObserverTail _legacy_tail;
+  // /mqtt_prefs carries a version newer than this firmware understands (a downgrade).
+  // The in-memory prefs run on defaults and saveMQTTPrefs() must not overwrite the
+  // file, or the first `set` command would destroy the newer config.
+  bool _mqtt_prefs_hold = false;
 #endif
   bool _com_prefs_needs_upgrade = false;  // old-format /com_prefs detected; rewrite once after load