diff --git a/src/graphics/draw/MenuHandler.cpp b/src/graphics/draw/MenuHandler.cpp index 7479a1595..b631fa841 100644 --- a/src/graphics/draw/MenuHandler.cpp +++ b/src/graphics/draw/MenuHandler.cpp @@ -28,9 +28,6 @@ #include "mesh/RadioLibInterface.h" #include "modules/AdminModule.h" #include "modules/CannedMessageModule.h" -#if !MESHTASTIC_EXCLUDE_MQTT -#include "mqtt/MQTT.h" -#endif #include "modules/ExternalNotificationModule.h" #include "modules/GeofenceModule.h" #include "modules/KeyVerificationModule.h" @@ -295,10 +292,6 @@ static void applyLoraRegion(meshtastic_Config_LoRaConfig_RegionCode region, bool if (strncmp(moduleConfig.mqtt.root, default_mqtt_root, strlen(default_mqtt_root)) == 0) { snprintf(moduleConfig.mqtt.root, sizeof(moduleConfig.mqtt.root), "%s/%s", default_mqtt_root, myRegion->name); changes |= SEGMENT_MODULECONFIG; -#if !MESHTASTIC_EXCLUDE_MQTT - if (mqtt) - mqtt->reinitTopics(); -#endif } #if !MESHTASTIC_EXCLUDE_GPS // Enable gps if it was previously disabled due to region not being set diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index 6c46e804c..4fae5b143 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -1071,10 +1071,6 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c, bool fromOthers) if (strncmp(moduleConfig.mqtt.root, default_mqtt_root, strlen(default_mqtt_root)) == 0) { // Default root is in use, so subscribe to the appropriate MQTT topic for this region snprintf(moduleConfig.mqtt.root, sizeof(moduleConfig.mqtt.root), "%s/%s", default_mqtt_root, myRegion->name); -#if !MESHTASTIC_EXCLUDE_MQTT - if (mqtt) - mqtt->reinitTopics(); -#endif } changes |= SEGMENT_CONFIG | SEGMENT_MODULECONFIG; } else { @@ -1115,12 +1111,8 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c, bool fromOthers) if (strncmp(moduleConfig.mqtt.root, default_mqtt_root, strlen(default_mqtt_root)) == 0) { // Default root is in use, so subscribe to the appropriate MQTT topic for this region snprintf(moduleConfig.mqtt.root, sizeof(moduleConfig.mqtt.root), "%s/%s", default_mqtt_root, myRegion->name); -#if !MESHTASTIC_EXCLUDE_MQTT - if (mqtt) - mqtt->reinitTopics(); -#endif } - changes = SEGMENT_CONFIG | SEGMENT_MODULECONFIG; + changes |= SEGMENT_CONFIG | SEGMENT_MODULECONFIG; } // use_preset and bandwidth are coerced into valid values by the check. } diff --git a/src/mqtt/MQTT.cpp b/src/mqtt/MQTT.cpp index 28e37c860..99f88c491 100644 --- a/src/mqtt/MQTT.cpp +++ b/src/mqtt/MQTT.cpp @@ -376,6 +376,7 @@ void mqttInit() void MQTT::reinitTopics() { + topicRoot = moduleConfig.mqtt.root; const std::string root = *moduleConfig.mqtt.root ? moduleConfig.mqtt.root : default_mqtt_root; cryptTopic = root + "/2/e/"; mapTopic = root + "/2/map/"; @@ -580,6 +581,9 @@ int32_t MQTT::runOnce() { if (!moduleConfig.mqtt.enabled || !(moduleConfig.mqtt.map_reporting_enabled || channels.anyMqttEnabled())) return disable(); + // A region change rewrites the root at runtime, from several call sites + if (topicRoot != moduleConfig.mqtt.root) + reinitTopics(); bool wantConnection = wantsLink(); perhapsReportToMap(); diff --git a/src/mqtt/MQTT.h b/src/mqtt/MQTT.h index af316ff1b..5a905433c 100644 --- a/src/mqtt/MQTT.h +++ b/src/mqtt/MQTT.h @@ -58,9 +58,6 @@ class MQTT : private concurrency::OSThread bool isUsingDefaultServer() { return isConfiguredForDefaultServer; } bool isUsingDefaultRootTopic() { return isConfiguredForDefaultRootTopic; } - /// Rebuild topics from moduleConfig.mqtt.root and force a resubscribe. - void reinitTopics(); - /// Validate the meshtastic_ModuleConfig_MQTTConfig. static bool isValidConfig(const meshtastic_ModuleConfig_MQTTConfig &config) { return isValidConfig(config, nullptr); } @@ -101,6 +98,7 @@ class MQTT : private concurrency::OSThread explicit MQTT(std::unique_ptr mqttClient); #endif + std::string topicRoot; // moduleConfig.mqtt.root the topics below were built from std::string cryptTopic = "/2/e/"; // msh/2/e/CHANNELID/NODEID std::string mapTopic = "/2/map/"; // For protobuf-encoded MapReport messages @@ -114,6 +112,9 @@ class MQTT : private concurrency::OSThread */ void reconnect(); + /// Rebuild topics from moduleConfig.mqtt.root and force a resubscribe. + void reinitTopics(); + /** Tell the server what subscriptions we want (based on channels.downlink_enabled) */ void sendSubscriptions(); diff --git a/test/test_mqtt/MQTT.cpp b/test/test_mqtt/MQTT.cpp index ca1f0c191..e957fba00 100644 --- a/test/test_mqtt/MQTT.cpp +++ b/test/test_mqtt/MQTT.cpp @@ -1360,8 +1360,9 @@ void test_customMqttRoot(void) [] { return pubsub->subscriptions_.count("custom/2/e/test/+") && pubsub->subscriptions_.count("custom/2/e/PKI/+"); })); } -// After a LoRa region change, reinitTopics() updates the publish topic and forces -// the broker to reconnect with updated subscriptions. +// A LoRa region change rewrites moduleConfig.mqtt.root without telling MQTT (AdminModule, MenuHandler, +// InkHUD). runOnce() must pick it up, rebuild the topics and resubscribe; otherwise the node keeps +// publishing and subscribing under the old region's root until reboot. void test_reinitTopicsUpdatesOnRegionChange(void) { // Start MQTT with a US region root. @@ -1371,11 +1372,10 @@ void test_reinitTopicsUpdatesOnRegionChange(void) TEST_ASSERT_TRUE(loopUntil( [] { return pubsub->subscriptions_.count("msh/US/2/e/test/+") && pubsub->subscriptions_.count("msh/US/2/e/PKI/+"); })); - // Simulate region change: update the root and call reinitTopics(). + // Simulate region change: only the root changes, nobody notifies MQTT. strcpy(moduleConfig.mqtt.root, "msh/EU_868"); pubsub->subscriptions_.clear(); pubsub->published_.clear(); - mqtt->reinitTopics(); // Verify that subscriptions are refreshed with the new region prefix. TEST_ASSERT_TRUE(loopUntil([] {