Preserve MQTT settings across slot changes

test/test_cli_command_utils/test_cli_command_utils.cpp

@@ -114,6 +114,22 @@ TEST(CLICommandUtils, NormalizesCapitalizedSetVerb) {
   EXPECT_STREQ("set altpath 600000,0d2784,F8DADA", command);
 }
 
+TEST(CLICommandUtils, MqttPortSupportsDefaultAndRejectsMalformedValues) {
+  uint16_t port = 99;
+  EXPECT_TRUE(mesh::cli::parseMqttPort("0", port));
+  EXPECT_EQ(0, port);
+  EXPECT_TRUE(mesh::cli::parseMqttPort("1883", port));
+  EXPECT_EQ(1883, port);
+  EXPECT_TRUE(mesh::cli::parseMqttPort("65535", port));
+  EXPECT_EQ(65535, port);
+  const char* invalid[] = {nullptr, "", "-1", "65536", "1883junk", "999999999999999999999"};
+  for (const char* value : invalid) {
+    port = 99;
+    EXPECT_FALSE(mesh::cli::parseMqttPort(value, port));
+    EXPECT_EQ(99, port);
+  }
+}
+
 TEST(CLICommandUtils, LoggingToggleHasOneGrammarForEveryRole) {
   bool enabled = false, reboot = false;
   ASSERT_TRUE(mesh::cli::parseLoggingToggle("on", enabled, reboot));

test/test_companion_mqtt_config.py

@@ -0,0 +1,166 @@
+#!/usr/bin/env python3
+"""Exercise the firmware's MQTT NVS loader and slot readiness on the host."""
+from pathlib import Path
+import subprocess
+import tempfile
+import unittest
+
+ROOT = Path(__file__).resolve().parents[1]
+SOURCE = ROOT / "src/helpers/CompanionMqttSetupPortal.cpp"
+
+
+def function(source, signature):
+    start = source.index(signature)
+    opening = source.index("{", start)
+    depth = 1
+    end = opening + 1
+    while depth:
+        depth += (source[end] == "{") - (source[end] == "}")
+        end += 1
+    return source[start:end] + "\n"
+
+
+PREAMBLE = r"""
+#include <cassert>
+#include <cctype>
+#include <cstring>
+#include <helpers/CompanionMqttPrefsNvs.h>
+constexpr int MAX_MQTT_SLOTS = MQTT_PREFS_SLOT_COUNT;
+constexpr int RUNTIME_MQTT_SLOTS = TEST_RUNTIME_SLOTS;
+constexpr int MQTT_TOPIC_MESHCORE = 0, MQTT_TOPIC_MESHRANK = 1;
+const char* MQTT_PRESET_CUSTOM = "custom";
+const char* MQTT_PRESET_NONE = "none";
+const char* NVS_NAMESPACE = "mesh-mqtt";
+const char* NVS_VERSION_KEY = "version";
+const char* NVS_PREFS_KEY = "prefs";
+struct MQTTPresetDef { int topic_style; bool username, password; };
+const MQTTPresetDef* findMQTTPreset(const char* name) {
+  static MQTTPresetDef analyzer{MQTT_TOPIC_MESHCORE, false, false};
+  static MQTTPresetDef credentials{MQTT_TOPIC_MESHCORE, true, false};
+  if (strcmp(name, "analyzer-us") == 0) return &analyzer;
+  if (strcmp(name, "username-only") == 0) return &credentials;
+  return nullptr;
+}
+bool mqttPresetNeedsSlotUsername(const MQTTPresetDef* p) { return p->username; }
+bool mqttPresetNeedsSlotPassword(const MQTTPresetDef* p) { return p->password; }
+void applyMQTTDefaults(MQTTPrefs* p) {
+  memset(p, 0, sizeof(*p));
+  for (auto& preset : p->mqtt_slot_preset) strcpy(preset, "none");
+}
+MQTTPrefs saved;
+struct Preferences {
+  bool begin(const char*, bool) { return true; }
+  bool isKey(const char*) { return true; }
+  uint16_t getUShort(const char*, uint16_t) { return MQTT_PREFS_VERSION; }
+  size_t getBytesLength(const char*) { return CompanionMqttPrefsNvs::kWriteSize; }
+  size_t getBytes(const char*, void* dest, size_t length) {
+    memcpy(dest, &saved, length);
+    return length;
+  }
+  void end() {}
+};
+struct CompanionMqttSetupPortal {
+  static bool loadStoredConfig(MQTTPrefs&);
+  static bool hasConfiguredSlot(const MQTTPrefs&);
+};
+"""
+
+SCENARIOS = r"""
+int main() {
+  MQTTPrefs result{};
+  applyMQTTDefaults(&saved);
+  // Disabled slots still carry reusable settings and credentials.
+  strcpy(saved.mqtt_slot_password[0], "keep-this-secret");
+  strcpy(saved.mqtt_slot_host[0], "configured.example");
+  assert(CompanionMqttSetupPortal::loadStoredConfig(result));
+  assert(memcmp(&result, &saved, sizeof(saved)) == 0);
+  assert(!CompanionMqttSetupPortal::hasConfiguredSlot(result));
+
+  // Saving a custom broker in stages must not reset it on the next boot.
+  strcpy(saved.mqtt_slot_preset[0], "custom");
+  assert(CompanionMqttSetupPortal::loadStoredConfig(result));
+  assert(strcmp(result.mqtt_slot_host[0], "configured.example") == 0);
+  assert(!CompanionMqttSetupPortal::hasConfiguredSlot(result));
+
+  // A full URI needs no separate port override. Topic replaces IATA for custom.
+  strcpy(saved.mqtt_slot_host[0], "mqtt://127.0.0.1:1");
+  strcpy(saved.mqtt_slot_topic[0], "test/{device}/{type}");
+  assert(CompanionMqttSetupPortal::loadStoredConfig(result));
+  assert(CompanionMqttSetupPortal::hasConfiguredSlot(result));
+
+  if (RUNTIME_MQTT_SLOTS > 1) {
+    // Slot 1 may be disabled while another slot is the only usable broker.
+    applyMQTTDefaults(&saved);
+    strcpy(saved.mqtt_slot_preset[1], "custom");
+    strcpy(saved.mqtt_slot_host[1], "mqtt://127.0.0.1:1");
+    strcpy(saved.mqtt_slot_topic[1], "test");
+    assert(CompanionMqttSetupPortal::loadStoredConfig(result));
+    assert(CompanionMqttSetupPortal::hasConfiguredSlot(result));
+    assert(strcmp(result.mqtt_slot_preset[0], "none") == 0);
+  }
+
+  // Saved slots outside this image's runtime limit remain stored, but do not run.
+  applyMQTTDefaults(&saved);
+  strcpy(saved.mqtt_slot_preset[MAX_MQTT_SLOTS - 1], "custom");
+  strcpy(saved.mqtt_slot_host[MAX_MQTT_SLOTS - 1], "mqtt://127.0.0.1:1");
+  strcpy(saved.mqtt_slot_topic[MAX_MQTT_SLOTS - 1], "test");
+  assert(CompanionMqttSetupPortal::loadStoredConfig(result));
+  assert(!CompanionMqttSetupPortal::hasConfiguredSlot(result));
+  assert(memcmp(&result, &saved, sizeof(saved)) == 0);
+
+  applyMQTTDefaults(&saved);
+  strcpy(saved.mqtt_slot_preset[0], "analyzer-us");
+  assert(CompanionMqttSetupPortal::loadStoredConfig(result));
+  assert(!CompanionMqttSetupPortal::hasConfiguredSlot(result));
+  strcpy(saved.mqtt_iata, "SEA");
+  assert(CompanionMqttSetupPortal::loadStoredConfig(result));
+  assert(CompanionMqttSetupPortal::hasConfiguredSlot(result));
+  strcpy(saved.mqtt_slot_preset[0], "username-only");
+  assert(CompanionMqttSetupPortal::loadStoredConfig(result));
+  assert(!CompanionMqttSetupPortal::hasConfiguredSlot(result));
+  strcpy(saved.mqtt_slot_username[0], "user");
+  assert(CompanionMqttSetupPortal::loadStoredConfig(result));
+  assert(CompanionMqttSetupPortal::hasConfiguredSlot(result));
+
+  // Corrupt records still fail without overwriting live preferences.
+  MQTTPrefs before = result;
+  saved.mqtt_tx_enabled = 255;
+  assert(!CompanionMqttSetupPortal::loadStoredConfig(result));
+  assert(memcmp(&before, &result, sizeof(before)) == 0);
+  saved.mqtt_tx_enabled = 0;
+  memset(saved.mqtt_slot_password[0], 'X', sizeof(saved.mqtt_slot_password[0]));
+  assert(!CompanionMqttSetupPortal::loadStoredConfig(result));
+  assert(memcmp(&before, &result, sizeof(before)) == 0);
+}
+"""
+
+
+class CompanionMqttConfigTests(unittest.TestCase):
+    def test_saved_settings_and_readiness_are_independent(self):
+        source = SOURCE.read_text()
+        actual = source[source.index("template <size_t N>"):
+                        source.index("static bool readLine")]
+        for signature in (
+            "static bool isThreeLetterCode",
+            "bool CompanionMqttSetupPortal::loadStoredConfig",
+            "bool CompanionMqttSetupPortal::hasConfiguredSlot",
+        ):
+            actual += function(source, signature)
+        with tempfile.TemporaryDirectory(prefix="companion-mqtt-config-") as tmp:
+            cpp = Path(tmp) / "config.cpp"
+            cpp.write_text(PREAMBLE + actual + SCENARIOS)
+            for slots in (1, 3):
+                with self.subTest(runtime_slots=slots):
+                    binary = Path(tmp) / ("config-%d" % slots)
+                    compile_result = subprocess.run([
+                        "c++", "-std=c++17", "-Wall", "-Wextra", "-Werror",
+                        "-DWITH_MQTT_BRIDGE=1", "-DTEST_RUNTIME_SLOTS=%d" % slots,
+                        "-I", str(ROOT / "src"), str(cpp), "-o", str(binary),
+                    ], text=True, capture_output=True)
+                    self.assertEqual(compile_result.returncode, 0, compile_result.stderr)
+                    result = subprocess.run([str(binary)], text=True, capture_output=True)
+                    self.assertEqual(result.returncode, 0, result.stderr)
+
+
+if __name__ == "__main__":
+    unittest.main()

test/test_companion_mqtt_stack.py

@@ -15,6 +15,7 @@ struct MQTTBridge { void end(); };
 struct CompanionMqttSetupPortal {
   static bool saveStoredConfig(const MQTTPrefs&);
   static bool loadStoredConfig(MQTTPrefs&);
+  static bool hasConfiguredSlot(const MQTTPrefs&);
 };
 struct MyMesh {
   void onConfigBatchEnd();

test/test_webconfig_ui_runtime.py

@@ -116,7 +116,7 @@ class WebConfigUiRuntimeTest(unittest.TestCase):
 
     def test_companion_console_runs_commands_and_uses_role_suggestions(self):
         status, config = self.setup_values()
-        status.update(mode="lan", cli=True, mqtt=True, active_slots=1,
+        status.update(mode="lan", cli=True, mqtt=True, active_slots=5, runtime_slots=1,
                       password_supported=False)
         prelude = """
 <script>
@@ -188,7 +188,7 @@ class WebConfigUiRuntimeTest(unittest.TestCase):
                         "set mqtt1.preset ", "get name", "get radio"):
             self.assertIn(command, suggestions)
         for command in ("erase", "password ", "setperm ", "get prv.key",
-                        "set bridge.enabled ", "get mqtt1.password"):
+                        "set bridge.enabled ", "get mqtt1.password", "set mqtt2.preset "):
             self.assertNotIn(command, suggestions)
 
     def test_console_stays_hidden_when_disabled_or_on_setup_ap(self):