From 2506e3ce3e398ab71b9c9f8af4a21c2375116cd1 Mon Sep 17 00:00:00 2001 From: Ryan Malloy Date: Thu, 5 Feb 2026 10:02:15 -0700 Subject: [PATCH] fix: improve error logging for MQTT connection failures - Add TLS security warning when certificate verification is disabled - Add getMQTTErrorString() to translate PubSubClient error codes - Show broker/user info on connection failure for debugging - Standardize log prefix to [MQTT] (was inconsistent [MQTTS]) - Use transport-agnostic "Network" instead of "WiFi" in messages - Fix WiFiState/NetworkState enum mismatch in getWiFiStatus() Error codes now show meaningful messages like: "Bad credentials (check username/password)" instead of "rc=4" --- src/MQTTBridge.cpp | 45 +++++++++++++++++++++++++++++++++------------ src/MQTTBridge.h | 3 +++ src/MyMesh.cpp | 6 +++--- 3 files changed, 39 insertions(+), 15 deletions(-) diff --git a/src/MQTTBridge.cpp b/src/MQTTBridge.cpp index 27c532d..a3469f4 100644 --- a/src/MQTTBridge.cpp +++ b/src/MQTTBridge.cpp @@ -45,10 +45,11 @@ void MQTTBridge::begin(const MQTTConfig& config, const uint8_t* self_pubkey) { if (_config.use_tls) { _wifi_client_secure.setInsecure(); // Skip cert verification _mqtt_client.setClient(_wifi_client_secure); - Serial.printf("[MQTT] TLS enabled\n"); + Serial.println("[MQTT] TLS enabled"); + Serial.println("[MQTT] WARNING: Certificate verification DISABLED - vulnerable to MITM attacks!"); } else { _mqtt_client.setClient(_wifi_client); - Serial.printf("[MQTT] Plain TCP\n"); + Serial.println("[MQTT] Plain TCP (no encryption)"); } _mqtt_client.setServer(_config.broker, _config.port); @@ -73,7 +74,7 @@ void MQTTBridge::loop() { if (!_network->isConnected()) { if (_state == MQTTState::CONNECTED) { _state = MQTTState::DISCONNECTED; - Serial.println("[MQTTS] WiFi lost, disconnected"); + Serial.println("[MQTT] Network connection lost, disconnecting"); } return; } @@ -91,7 +92,7 @@ void MQTTBridge::loop() { case MQTTState::CONNECTED: if (!_mqtt_client.connected()) { _state = MQTTState::DISCONNECTED; - Serial.println("[MQTTS] Connection lost"); + Serial.println("[MQTT] Connection lost"); _last_connect_attempt = millis(); } else { _mqtt_client.loop(); @@ -125,7 +126,7 @@ void MQTTBridge::end() { _state = MQTTState::DISCONNECTED; _initialized = false; - Serial.println("[MQTTS] Stopped"); + Serial.println("[MQTT] Stopped"); } void MQTTBridge::attemptConnection() { @@ -134,7 +135,7 @@ void MQTTBridge::attemptConnection() { _state = MQTTState::CONNECTING; _last_connect_attempt = millis(); - Serial.printf("[MQTTS] Connecting to %s:%d...\n", _config.broker, _config.port); + Serial.printf("[MQTT] Connecting to %s:%d...\n", _config.broker, _config.port); String client_id = String(_config.client_id); if (client_id.length() == 0) { @@ -157,18 +158,38 @@ void MQTTBridge::attemptConnection() { _state = MQTTState::CONNECTED; _connected_since = millis(); _reconnect_count++; - Serial.println("[MQTTS] Connected!"); + Serial.println("[MQTT] Connected!"); subscribeToCommands(); publishStatus(); _last_status_publish = millis(); } else { int rc = _mqtt_client.state(); - Serial.printf("[MQTTS] Connection failed, rc=%d\n", rc); + const char* error_str = getMQTTErrorString(rc); + Serial.printf("[MQTT] Connection failed: %s (code %d)\n", error_str, rc); + Serial.printf("[MQTT] Broker: %s:%d, User: %s\n", _config.broker, _config.port, + strlen(_config.user) > 0 ? _config.user : "(none)"); _state = MQTTState::ERROR; } } +const char* MQTTBridge::getMQTTErrorString(int rc) { + // PubSubClient state() return codes + switch (rc) { + case -4: return "Connection timeout"; + case -3: return "Connection lost"; + case -2: return "Connect failed (network)"; + case -1: return "Disconnected cleanly"; + case 0: return "Connected"; + case 1: return "Bad protocol version"; + case 2: return "Client ID rejected"; + case 3: return "Server unavailable"; + case 4: return "Bad credentials (check username/password)"; + case 5: return "Not authorized"; + default: return "Unknown error"; + } +} + void MQTTBridge::setupTopics() { snprintf(_topic_status, sizeof(_topic_status), "%s/gateway/%s/status", _config.topic_prefix, _gateway_id); @@ -188,7 +209,7 @@ void MQTTBridge::subscribeToCommands() { char topic[100]; snprintf(topic, sizeof(topic), "%s#", _topic_cmd_prefix); _mqtt_client.subscribe(topic); - Serial.printf("[MQTTS] Subscribed to: %s\n", topic); + Serial.printf("[MQTT] Subscribed to: %s\n", topic); } void MQTTBridge::updateConfig(const MQTTConfig& config) { @@ -333,7 +354,7 @@ void MQTTBridge::publishMessage(const char* topic, const char* payload, bool ret if (_mqtt_client.publish(topic, payload, retained)) { _messages_sent++; } else { - Serial.printf("[MQTTS] Publish failed to %s\n", topic); + Serial.printf("[MQTT] Publish failed to %s\n", topic); } } @@ -380,10 +401,10 @@ void MQTTBridge::handleMessage(char* topic, uint8_t* payload, unsigned int lengt if (strncmp(topic, _topic_cmd_prefix, strlen(_topic_cmd_prefix)) == 0) { const char* cmd = topic + strlen(_topic_cmd_prefix); - Serial.printf("[MQTTS] Command received: %s\n", cmd); + Serial.printf("[MQTT] Command received: %s\n", cmd); if (strcmp(cmd, "reboot") == 0) { - Serial.println("[MQTTS] Reboot requested"); + Serial.println("[MQTT] Reboot requested"); publishStatus(); delay(100); ESP.restart(); diff --git a/src/MQTTBridge.h b/src/MQTTBridge.h index f495447..4e38fd8 100644 --- a/src/MQTTBridge.h +++ b/src/MQTTBridge.h @@ -111,6 +111,9 @@ private: void publishMessage(const char* topic, const char* payload, bool retained = false); void publishJson(const char* topic, JsonDocument& doc, bool retained = false); + // Error code translation for better diagnostics + static const char* getMQTTErrorString(int rc); + static uint32_t fnv1a_hash(const uint8_t* data, size_t len); bool isDuplicate(const uint8_t* data, size_t len); diff --git a/src/MyMesh.cpp b/src/MyMesh.cpp index 30c9a12..6447302 100644 --- a/src/MyMesh.cpp +++ b/src/MyMesh.cpp @@ -1289,9 +1289,9 @@ const char* MyMesh::getMQTTStatus() const { const char* MyMesh::getWiFiStatus() const { switch (_wifi_mgr.getState()) { - case WiFiState::CONNECTED: return "connected"; - case WiFiState::CONNECTING: return "connecting"; - case WiFiState::AP_MODE: return "ap_mode"; + case NetworkState::CONNECTED: return "connected"; + case NetworkState::CONNECTING: return "connecting"; + case NetworkState::AP_MODE: return "ap_mode"; default: return "disconnected"; } }