rf/meshcore-repeater
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Compare commits

base: rf:main
Branches Tags
rf:main
rf:fix/improve-error-logging
rf:fix/memory-leaks
rf:refactor/unify-network-code
rf:fix/mqtt-exponential-backoff
rf:fix/wifi-manager-dangling-pointer
..
compare: rf:fix/memory-leaks
Branches Tags
rf:main
rf:fix/improve-error-logging
rf:fix/memory-leaks
rf:refactor/unify-network-code
rf:fix/mqtt-exponential-backoff
rf:fix/wifi-manager-dangling-pointer

No commits in common. "main" and "fix/memory-leaks" have entirely different histories.
main ... fix/memory

4 changed files with 22 additions and 67 deletions
Show Stats Expand all files Collapse all files

60
src/MQTTBridge.cpp
View File

@ -17,7 +17,6 @@ MQTTBridge::MQTTBridge(INetworkManager* network, mesh::PacketManager* mgr, mesh:
_last_status_publish(0), _last_status_publish(0),
_last_stats_publish(0), _last_stats_publish(0),
_connected_since(0), _connected_since(0),
_current_backoff_ms(BACKOFF_MIN_MS),
_messages_sent(0), _messages_sent(0),
_messages_received(0), _messages_received(0),
_reconnect_count(0), _reconnect_count(0),
@ -46,11 +45,10 @@ void MQTTBridge::begin(const MQTTConfig& config, const uint8_t* self_pubkey) {
if (_config.use_tls) { if (_config.use_tls) {
_wifi_client_secure.setInsecure(); // Skip cert verification _wifi_client_secure.setInsecure(); // Skip cert verification
_mqtt_client.setClient(_wifi_client_secure); _mqtt_client.setClient(_wifi_client_secure);
Serial.println("[MQTT] TLS enabled"); Serial.printf("[MQTT] TLS enabled\n");
Serial.println("[MQTT] WARNING: Certificate verification DISABLED - vulnerable to MITM attacks!");
} else { } else {
_mqtt_client.setClient(_wifi_client); _mqtt_client.setClient(_wifi_client);
Serial.println("[MQTT] Plain TCP (no encryption)"); Serial.printf("[MQTT] Plain TCP\n");
} }
_mqtt_client.setServer(_config.broker, _config.port); _mqtt_client.setServer(_config.broker, _config.port);
@ -75,15 +73,14 @@ void MQTTBridge::loop() {
if (!_network->isConnected()) { if (!_network->isConnected()) {
if (_state == MQTTState::CONNECTED) { if (_state == MQTTState::CONNECTED) {
_state = MQTTState::DISCONNECTED; _state = MQTTState::DISCONNECTED;
Serial.println("[MQTT] Network connection lost, disconnecting"); Serial.println("[MQTTS] WiFi lost, disconnected");
} }
return; return;
} }
switch (_state) { switch (_state) {
case MQTTState::DISCONNECTED: case MQTTState::DISCONNECTED:
// Use exponential backoff for reconnection attempts if (millis() - _last_connect_attempt > 5000) {
if (millis() - _last_connect_attempt > _current_backoff_ms) {
attemptConnection(); attemptConnection();
} }
break; break;
@ -94,9 +91,8 @@ void MQTTBridge::loop() {
case MQTTState::CONNECTED: case MQTTState::CONNECTED:
if (!_mqtt_client.connected()) { if (!_mqtt_client.connected()) {
_state = MQTTState::DISCONNECTED; _state = MQTTState::DISCONNECTED;
Serial.println("[MQTT] Connection lost"); Serial.println("[MQTTS] Connection lost");
_last_connect_attempt = millis(); _last_connect_attempt = millis();
// Don't reset backoff on connection loss - broker might be down
} else { } else {
_mqtt_client.loop(); _mqtt_client.loop();
@ -108,8 +104,7 @@ void MQTTBridge::loop() {
break; break;
case MQTTState::ERROR: case MQTTState::ERROR:
// Use backoff for error recovery too if (millis() - _last_connect_attempt > 30000) {
if (millis() - _last_connect_attempt > _current_backoff_ms) {
_state = MQTTState::DISCONNECTED; _state = MQTTState::DISCONNECTED;
} }
break; break;
@ -130,7 +125,7 @@ void MQTTBridge::end() {
_state = MQTTState::DISCONNECTED; _state = MQTTState::DISCONNECTED;
_initialized = false; _initialized = false;
Serial.println("[MQTT] Stopped"); Serial.println("[MQTTS] Stopped");
} }
void MQTTBridge::attemptConnection() { void MQTTBridge::attemptConnection() {
@ -139,8 +134,7 @@ void MQTTBridge::attemptConnection() {
_state = MQTTState::CONNECTING; _state = MQTTState::CONNECTING;
_last_connect_attempt = millis(); _last_connect_attempt = millis();
Serial.printf("[MQTT] Connecting to %s:%d (backoff=%lums)...\n", Serial.printf("[MQTTS] Connecting to %s:%d...\n", _config.broker, _config.port);
_config.broker, _config.port, _current_backoff_ms);
String client_id = String(_config.client_id); String client_id = String(_config.client_id);
if (client_id.length() == 0) { if (client_id.length() == 0) {
@ -163,41 +157,15 @@ void MQTTBridge::attemptConnection() {
_state = MQTTState::CONNECTED; _state = MQTTState::CONNECTED;
_connected_since = millis(); _connected_since = millis();
_reconnect_count++; _reconnect_count++;
// Reset backoff on successful connection Serial.println("[MQTTS] Connected!");
_current_backoff_ms = BACKOFF_MIN_MS;
Serial.println("[MQTT] Connected!");
subscribeToCommands(); subscribeToCommands();
publishStatus(); publishStatus();
_last_status_publish = millis(); _last_status_publish = millis();
} else { } else {
int rc = _mqtt_client.state(); int rc = _mqtt_client.state();
const char* error_str = getMQTTErrorString(rc); Serial.printf("[MQTTS] Connection failed, rc=%d\n", 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; _state = MQTTState::ERROR;
// Exponential backoff: double the delay (up to max)
_current_backoff_ms = min(_current_backoff_ms * BACKOFF_MULTIPLIER, BACKOFF_MAX_MS);
Serial.printf("[MQTTS] Next retry in %lums\n", _current_backoff_ms);
}
}
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";
} }
} }
@ -220,7 +188,7 @@ void MQTTBridge::subscribeToCommands() {
char topic[100]; char topic[100];
snprintf(topic, sizeof(topic), "%s#", _topic_cmd_prefix); snprintf(topic, sizeof(topic), "%s#", _topic_cmd_prefix);
_mqtt_client.subscribe(topic); _mqtt_client.subscribe(topic);
Serial.printf("[MQTT] Subscribed to: %s\n", topic); Serial.printf("[MQTTS] Subscribed to: %s\n", topic);
} }
void MQTTBridge::updateConfig(const MQTTConfig& config) { void MQTTBridge::updateConfig(const MQTTConfig& config) {
@ -365,7 +333,7 @@ void MQTTBridge::publishMessage(const char* topic, const char* payload, bool ret
if (_mqtt_client.publish(topic, payload, retained)) { if (_mqtt_client.publish(topic, payload, retained)) {
_messages_sent++; _messages_sent++;
} else { } else {
Serial.printf("[MQTT] Publish failed to %s\n", topic); Serial.printf("[MQTTS] Publish failed to %s\n", topic);
} }
} }
@ -412,10 +380,10 @@ void MQTTBridge::handleMessage(char* topic, uint8_t* payload, unsigned int lengt
if (strncmp(topic, _topic_cmd_prefix, strlen(_topic_cmd_prefix)) == 0) { if (strncmp(topic, _topic_cmd_prefix, strlen(_topic_cmd_prefix)) == 0) {
const char* cmd = topic + strlen(_topic_cmd_prefix); const char* cmd = topic + strlen(_topic_cmd_prefix);
Serial.printf("[MQTT] Command received: %s\n", cmd); Serial.printf("[MQTTS] Command received: %s\n", cmd);
if (strcmp(cmd, "reboot") == 0) { if (strcmp(cmd, "reboot") == 0) {
Serial.println("[MQTT] Reboot requested"); Serial.println("[MQTTS] Reboot requested");
publishStatus(); publishStatus();
delay(100); delay(100);
ESP.restart(); ESP.restart();

9
src/MQTTBridge.h
View File

@ -85,12 +85,6 @@ private:
unsigned long _last_stats_publish; unsigned long _last_stats_publish;
unsigned long _connected_since; unsigned long _connected_since;
// Exponential backoff for reconnection
static constexpr uint32_t BACKOFF_MIN_MS = 1000; // Start at 1 second
static constexpr uint32_t BACKOFF_MAX_MS = 60000; // Max 60 seconds
static constexpr uint32_t BACKOFF_MULTIPLIER = 2; // Double each attempt
uint32_t _current_backoff_ms;
uint32_t _messages_sent; uint32_t _messages_sent;
uint32_t _messages_received; uint32_t _messages_received;
uint32_t _reconnect_count; uint32_t _reconnect_count;
@ -117,9 +111,6 @@ private:
void publishMessage(const char* topic, const char* payload, bool retained = false); void publishMessage(const char* topic, const char* payload, bool retained = false);
void publishJson(const char* topic, JsonDocument& doc, 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); static uint32_t fnv1a_hash(const uint8_t* data, size_t len);
bool isDuplicate(const uint8_t* data, size_t len); bool isDuplicate(const uint8_t* data, size_t len);

19
src/WiFiManager.cpp
View File

@ -10,7 +10,6 @@ WiFiManager::WiFiManager()
_retry_count(0), _retry_count(0),
_initialized(false) { _initialized(false) {
memset(&_config, 0, sizeof(_config)); memset(&_config, 0, sizeof(_config));
memset(_cached_ssid, 0, sizeof(_cached_ssid));
} }
void WiFiManager::begin(const WiFiConfig& config) { void WiFiManager::begin(const WiFiConfig& config) {
@ -83,9 +82,6 @@ void WiFiManager::loop() {
_state = WiFiState::CONNECTED; _state = WiFiState::CONNECTED;
_connected_since = millis(); _connected_since = millis();
_retry_count = 0; _retry_count = 0;
// Cache SSID to avoid dangling pointer from WiFi.SSID().c_str()
strncpy(_cached_ssid, WiFi.SSID().c_str(), sizeof(_cached_ssid) - 1);
_cached_ssid[sizeof(_cached_ssid) - 1] = '\0';
Serial.printf("[WiFi] Connected! IP: %s, RSSI: %d dBm\n", Serial.printf("[WiFi] Connected! IP: %s, RSSI: %d dBm\n",
WiFi.localIP().toString().c_str(), WiFi.RSSI()); WiFi.localIP().toString().c_str(), WiFi.RSSI());
} else if (status == WL_NO_SSID_AVAIL) { } else if (status == WL_NO_SSID_AVAIL) {
@ -188,9 +184,6 @@ void WiFiManager::startAPMode() {
if (success) { if (success) {
_state = WiFiState::AP_MODE; _state = WiFiState::AP_MODE;
// Cache AP SSID to avoid dangling pointer
strncpy(_cached_ssid, ap_ssid.c_str(), sizeof(_cached_ssid) - 1);
_cached_ssid[sizeof(_cached_ssid) - 1] = '\0';
Serial.printf("[WiFi] AP started: SSID='%s', IP=%s\n", Serial.printf("[WiFi] AP started: SSID='%s', IP=%s\n",
ap_ssid.c_str(), WiFi.softAPIP().toString().c_str()); ap_ssid.c_str(), WiFi.softAPIP().toString().c_str());
} else { } else {
@ -228,8 +221,10 @@ int32_t WiFiManager::getRSSI() const {
} }
const char* WiFiManager::getSSID() const { const char* WiFiManager::getSSID() const {
if (_state == WiFiState::CONNECTED || _state == WiFiState::AP_MODE) { if (_state == WiFiState::CONNECTED) {
return _cached_ssid; return WiFi.SSID().c_str();
} else if (_state == WiFiState::AP_MODE) {
return WiFi.softAPSSID().c_str();
} }
return ""; return "";
} }
@ -269,8 +264,10 @@ NetworkState WiFiManager::getState() const {
} }
const char* WiFiManager::getConnectionName() const { const char* WiFiManager::getConnectionName() const {
if (_state == WiFiState::CONNECTED || _state == WiFiState::AP_MODE) { if (_state == WiFiState::CONNECTED) {
return _cached_ssid; return WiFi.SSID().c_str();
} else if (_state == WiFiState::AP_MODE) {
return WiFi.softAPSSID().c_str();
} }
return "Not connected"; return "Not connected";
} }

1
src/WiFiManager.h
View File

@ -61,7 +61,6 @@ private:
unsigned long _connected_since; unsigned long _connected_since;
uint8_t _retry_count; uint8_t _retry_count;
bool _initialized; bool _initialized;
char _cached_ssid[33]; // Cached SSID to avoid dangling pointer from WiFi.SSID().c_str()
void attemptConnection(); void attemptConnection();
void checkConnection(); void checkConnection();