diff --git a/MQTT_INTERNALS.md b/MQTT_INTERNALS.md index 1adb9713..ff65c8b6 100644 --- a/MQTT_INTERNALS.md +++ b/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 diff --git a/src/helpers/AlertReporter.cpp b/src/helpers/AlertReporter.cpp index 793a8d79..a6841277 100644 --- a/src/helpers/AlertReporter.cpp +++ b/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. diff --git a/src/helpers/CommonCLI.cpp b/src/helpers/CommonCLI.cpp index 9d832f93..26ce8ad9 100644 --- a/src/helpers/CommonCLI.cpp +++ b/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); diff --git a/src/helpers/CommonCLI.h b/src/helpers/CommonCLI.h index 604ca77f..ef4a797c 100644 --- a/src/helpers/CommonCLI.h +++ b/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