Fix MQTT config bugs (#7446)

* Fix mqtt config bugs

* Apply suggestion from @Copilot

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Add client notification

* Verbiage

* Update src/mqtt/MQTT.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/mqtt/MQTT.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/mqtt/MQTT.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/mqtt/MQTT.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Remove test that is no longer true

* This test no longer exists

* Fix client notification crap

* Suppress false positive

* Revert "Suppress false positive"

This reverts commit bead96eaee.

* Try macro exclusion

* Derp

* Fix

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Jason P <applewiz@mac.com>

src/mesh/MeshService.h

@@ -146,7 +146,7 @@ class MeshService
     virtual void sendMqttMessageToClientProxy(meshtastic_MqttClientProxyMessage *m);
 
     /// Send a ClientNotification to the phone
-    void sendClientNotification(meshtastic_ClientNotification *cn);
+    virtual void sendClientNotification(meshtastic_ClientNotification *cn);
 
     /// Send an error response to the phone
     void sendRoutingErrorResponse(meshtastic_Routing_Error error, const meshtastic_MeshPacket *mp);

src/modules/AdminModule.cpp

@@ -798,8 +798,11 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c)
 
 bool AdminModule::handleSetModuleConfig(const meshtastic_ModuleConfig &c)
 {
-    if (!hasOpenEditTransaction)
+    // If we are in an open transaction or configuring MQTT, defer disabling Bluetooth
+    // Otherwise, disable Bluetooth to prevent the phone from interfering with the config
+    if (!hasOpenEditTransaction && c.which_payload_variant != meshtastic_ModuleConfig_mqtt_tag)
         disableBluetooth();
+
     switch (c.which_payload_variant) {
     case meshtastic_ModuleConfig_mqtt_tag:
 #if MESHTASTIC_EXCLUDE_MQTT
@@ -810,6 +813,8 @@ bool AdminModule::handleSetModuleConfig(const meshtastic_ModuleConfig &c)
         if (!MQTT::isValidConfig(c.payload_variant.mqtt)) {
             return false;
         }
+        // Disable Bluetooth to prevent interference during MQTT configuration
+        disableBluetooth();
         moduleConfig.has_mqtt = true;
         moduleConfig.mqtt = c.payload_variant.mqtt;
 #endif

src/mqtt/MQTT.cpp

@@ -39,6 +39,7 @@
 #include <machine/endian.h>
 #define ntohl __ntohl
 #endif
+#include <RTC.h>
 
 MQTT *mqtt;
 
@@ -624,18 +625,32 @@ bool MQTT::isValidConfig(const meshtastic_ModuleConfig_MQTTConfig &config, MQTTC
             return connectPubSub(parsed, *pubSub, (client != nullptr) ? *client : *clientConnection);
         }
 #else
-        LOG_ERROR("Invalid MQTT config: proxy_to_client_enabled must be enabled on nodes that do not have a network");
+        const char *warning = "Invalid MQTT config: proxy_to_client_enabled must be enabled on nodes that do not have a network";
+        LOG_ERROR(warning);
+#if !IS_RUNNING_TESTS
+        meshtastic_ClientNotification *cn = clientNotificationPool.allocZeroed();
+        cn->level = meshtastic_LogRecord_Level_ERROR;
+        cn->time = getValidTime(RTCQualityFromNet);
+        strncpy(cn->message, warning, sizeof(cn->message) - 1);
+        cn->message[sizeof(cn->message) - 1] = '\0'; // Ensure null termination
+        service->sendClientNotification(cn);
+#endif
         return false;
 #endif
     }
 
     const bool defaultServer = isDefaultServer(parsed.serverAddr);
-    if (defaultServer && config.tls_enabled) {
-        LOG_ERROR("Invalid MQTT config: TLS was enabled, but the default server does not support TLS");
-        return false;
-    }
     if (defaultServer && parsed.serverPort != PubSubConfig::defaultPort) {
-        LOG_ERROR("Invalid MQTT config: Unsupported port '%d' for the default MQTT server", parsed.serverPort);
+        const char *warning = "Invalid MQTT config: default server address must not have a port specified";
+        LOG_ERROR(warning);
+#if !IS_RUNNING_TESTS
+        meshtastic_ClientNotification *cn = clientNotificationPool.allocZeroed();
+        cn->level = meshtastic_LogRecord_Level_ERROR;
+        cn->time = getValidTime(RTCQualityFromNet);
+        strncpy(cn->message, warning, sizeof(cn->message) - 1);
+        cn->message[sizeof(cn->message) - 1] = '\0'; // Ensure null termination
+        service->sendClientNotification(cn);
+#endif
         return false;
     }
     return true;

test/test_mqtt/MQTT.cpp

@@ -27,6 +27,12 @@
 #include <utility>
 #include <variant>
 
+#if defined(UNIT_TEST)
+#define IS_RUNNING_TESTS 1
+#else
+#define IS_RUNNING_TESTS 0
+#endif
+
 namespace
 {
 // Minimal router needed to receive messages from MQTT.
@@ -56,7 +62,13 @@ class MockMeshService : public MeshService
         messages_.emplace_back(*m);
         releaseMqttClientProxyMessageToPool(m);
     }
-    std::list<meshtastic_MqttClientProxyMessage> messages_; // Messages received from the MeshService.
+    void sendClientNotification(meshtastic_ClientNotification *n) override
+    {
+        notifications_.emplace_back(*n);
+        releaseClientNotificationToPool(n);
+    }
+    std::list<meshtastic_MqttClientProxyMessage> messages_;  // Messages received from the MeshService.
+    std::list<meshtastic_ClientNotification> notifications_; // Notifications received from the MeshService.
 };
 
 // Minimal NodeDB needed to return values from getMeshNode.
@@ -823,14 +835,6 @@ void test_configWithDefaultServerAndInvalidPort(void)
     TEST_ASSERT_FALSE(MQTT::isValidConfig(config));
 }
 
-// Configuration with the default server and tls_enabled = true is invalid.
-void test_configWithDefaultServerAndInvalidTLSEnabled(void)
-{
-    meshtastic_ModuleConfig_MQTTConfig config = {.tls_enabled = true};
-
-    TEST_ASSERT_FALSE(MQTT::isValidConfig(config));
-}
-
 // isValidConfig connects to a custom host and port.
 void test_configCustomHostAndPort(void)
 {
@@ -911,7 +915,6 @@ void setup()
     RUN_TEST(test_configEnabledEmptyIsValid);
     RUN_TEST(test_configWithDefaultServer);
     RUN_TEST(test_configWithDefaultServerAndInvalidPort);
-    RUN_TEST(test_configWithDefaultServerAndInvalidTLSEnabled);
     RUN_TEST(test_configCustomHostAndPort);
     RUN_TEST(test_configWithConnectionFailure);
     RUN_TEST(test_configWithTLSEnabled);