From 4c683b13b30cb2acb5f29849fd4f758fd6c33c72 Mon Sep 17 00:00:00 2001 From: agessaman Date: Sat, 19 Sep 2026 10:14:16 -0700 Subject: [PATCH] fix(network): pre-merge review fixes - Guard the boot-time link bootstrap to observer ESP32 builds. It ran unconditionally in MyMesh::begin(), breaking every non-observer repeater/room server build (ESP32 and nRF52). - Rename NetworkInterface -> NetworkLink (class, accessor, files). Arduino-ESP32 3.x ships its own NetworkInterface class and header, which broke the ESP32-C6 builds. Drop WiFi.setAutoConnect(), a no-op on 2.x and removed in 3.x. - Refresh stored Wi-Fi credentials every bridge tick so the STA reconnect loop picks up `set wifi.ssid` / `set wifi.pwd` without a reboot, as the bridge did before the link moved out of it. Skip reconnects while the SSID is empty. - Restore the "WiFi connected: " / "WiFi disconnected: reason N" debug lines the bridge used to print. - Alert on Ethernet only once it has held a lease this boot or when no Wi-Fi is configured; Wi-Fi-only installs of an Ethernet-preferred image keep Wi-Fi alerts instead of reporting "Ethernet down". - Record wifi.setup_complete only for Ethernet LAN onboarding, so Wi-Fi builds keep the SSID-based first-boot portal rule. - Use seq_cst for the route-switch lock/mutation flag handshake. - Docs: SNMP RSSI sentinel is -127; describe link-return vs medium-switch reconnect behavior accurately; note runtime credential pickup. - Test: unknown keys inside a known /mqtt.json group are ignored, which keeps wifi.setup_complete downgrade-safe. --- MQTT_IMPLEMENTATION.md | 7 +- examples/simple_repeater/MyMesh.cpp | 14 +-- examples/simple_room_server/MyMesh.cpp | 14 +-- src/helpers/AlertReporter.cpp | 2 +- src/helpers/CommonCLI_Observer.cpp | 26 ++--- src/helpers/ESP32Board.cpp | 16 +-- .../{NetworkInterface.cpp => NetworkLink.cpp} | 97 ++++++++++++------- .../{NetworkInterface.h => NetworkLink.h} | 11 ++- src/helpers/SNMPAgent.cpp | 4 +- src/helpers/SNMPAgent.h | 2 +- src/helpers/bridges/MQTTBridge.cpp | 11 ++- src/helpers/bridges/MQTTBridge.h | 4 +- src/helpers/esp32/WebConfigServer.cpp | 22 ++--- .../test_mqtt_prefs_serializer.cpp | 14 +++ 14 files changed, 149 insertions(+), 95 deletions(-) rename src/helpers/{NetworkInterface.cpp => NetworkLink.cpp} (90%) rename src/helpers/{NetworkInterface.h => NetworkLink.h} (89%) diff --git a/MQTT_IMPLEMENTATION.md b/MQTT_IMPLEMENTATION.md index 60e96fc9..550513ec 100644 --- a/MQTT_IMPLEMENTATION.md +++ b/MQTT_IMPLEMENTATION.md @@ -872,8 +872,11 @@ the radio actually performs in that case. - CH390 startup initializes Arduino's shared network event runtime without associating WiFi; this keeps the framework's DNS and TLS hostname paths safe when Ethernet wins directly. - Ethernet/WiFi transitions are logged. A lost or changed route stops every started MQTT - client, including one whose disconnect callback arrived first; once a usable route returns, - route-caused backoff is cleared and one immediate reconnect is allowed + client, including one whose disconnect callback arrived first. When the same link returns, + each slot gets one immediate attempt at its current backoff rung (a tripped circuit breaker + gets one immediate probe); a switch to the other medium also clears backoff and breakers +- WiFi credentials changed at runtime (`set wifi.ssid` / `set wifi.pwd`) are used on the next + reconnect attempt without a reboot - Packets are queued while a slot is disconnected and flushed when it recovers ### Raw Radio Data Capture diff --git a/examples/simple_repeater/MyMesh.cpp b/examples/simple_repeater/MyMesh.cpp index 9ab4bd99..29518694 100644 --- a/examples/simple_repeater/MyMesh.cpp +++ b/examples/simple_repeater/MyMesh.cpp @@ -3,7 +3,7 @@ #include #include // for qsort() #include -#include +#include #include #if defined(ESP_PLATFORM) #include @@ -1148,7 +1148,8 @@ void MyMesh::begin(FILESYSTEM *fs) { } #endif - NetworkInterface& boot_network = activeNetworkInterface(); +#if defined(WITH_MQTT_BRIDGE) && defined(ESP_PLATFORM) + NetworkLink& boot_network = activeNetworkLink(); if (boot_network.isAutomatic()) { char network_hostname[NetworkHostname::kBufferSize]; NetworkHostname::build(network_hostname, sizeof(network_hostname), @@ -1165,6 +1166,7 @@ void MyMesh::begin(FILESYSTEM *fs) { boot_network.mediumName(), boot_network.statusName(), (unsigned long)(millis() - ethernet_probe_started_at)); } +#endif acl.load(_fs, self_id); // TODO: key_store.begin(); @@ -1504,13 +1506,13 @@ bool MyMesh::startWebConfig(bool force_ap, char* reply) { if (force_ap) { // The setup AP owns WiFi outright; refuse while the bridge holds the STA. if (bridge && bridge->isRunning() && - activeNetworkInterface().medium() != NetworkMedium::Ethernet) { + activeNetworkLink().medium() != NetworkMedium::Ethernet) { strcpy(reply, "Err: MQTT bridge is running - 'set bridge off' first"); return true; } _webconfig->startSetupMode(reply); - } else if (activeNetworkInterface().isConnected()) { - _webconfig->startLanMode(activeNetworkInterface().localIP(), + } else if (activeNetworkLink().isConnected()) { + _webconfig->startLanMode(activeNetworkLink().localIP(), !mqttNetworkSetupComplete(_cli.getObserverPrefs()), reply); } else if (!mqttNetworkSetupComplete(_cli.getObserverPrefs())) { _webconfig->startSetupMode(reply); @@ -1551,7 +1553,7 @@ void MyMesh::onConfigBatchEnd() { void MyMesh::buildStatsJson(char* buf, size_t buf_size) { char ip[20] = ""; char wifi_rssi[12] = "null"; - NetworkInterface& network = activeNetworkInterface(); + NetworkLink& network = activeNetworkLink(); const char* network_medium = network.mediumName(); if (network.isConnected()) { strncpy(ip, network.localIP().toString().c_str(), sizeof(ip) - 1); diff --git a/examples/simple_room_server/MyMesh.cpp b/examples/simple_room_server/MyMesh.cpp index 64c61a2e..f5b41860 100644 --- a/examples/simple_room_server/MyMesh.cpp +++ b/examples/simple_room_server/MyMesh.cpp @@ -2,7 +2,7 @@ #include #include #include -#include +#include #include #if defined(ESP_PLATFORM) #include @@ -949,7 +949,8 @@ void MyMesh::begin(FILESYSTEM *fs) { // load persisted prefs _cli.loadPrefs(_fs); - NetworkInterface& boot_network = activeNetworkInterface(); +#if defined(WITH_MQTT_BRIDGE) && defined(ESP_PLATFORM) + NetworkLink& boot_network = activeNetworkLink(); if (boot_network.isAutomatic()) { char network_hostname[NetworkHostname::kBufferSize]; NetworkHostname::build(network_hostname, sizeof(network_hostname), @@ -966,6 +967,7 @@ void MyMesh::begin(FILESYSTEM *fs) { boot_network.mediumName(), boot_network.statusName(), (unsigned long)(millis() - ethernet_probe_started_at)); } +#endif acl.load(_fs, self_id); region_map.load(_fs); @@ -1306,13 +1308,13 @@ bool MyMesh::startWebConfig(bool force_ap, char* reply) { if (force_ap) { // The setup AP owns WiFi outright; refuse while the bridge holds the STA. if (bridge && bridge->isRunning() && - activeNetworkInterface().medium() != NetworkMedium::Ethernet) { + activeNetworkLink().medium() != NetworkMedium::Ethernet) { strcpy(reply, "Err: MQTT bridge is running - 'set bridge off' first"); return true; } _webconfig->startSetupMode(reply); - } else if (activeNetworkInterface().isConnected()) { - _webconfig->startLanMode(activeNetworkInterface().localIP(), + } else if (activeNetworkLink().isConnected()) { + _webconfig->startLanMode(activeNetworkLink().localIP(), !mqttNetworkSetupComplete(_cli.getObserverPrefs()), reply); } else if (!mqttNetworkSetupComplete(_cli.getObserverPrefs())) { _webconfig->startSetupMode(reply); @@ -1353,7 +1355,7 @@ void MyMesh::onConfigBatchEnd() { void MyMesh::buildStatsJson(char* buf, size_t buf_size) { char ip[20] = ""; char wifi_rssi[12] = "null"; - NetworkInterface& network = activeNetworkInterface(); + NetworkLink& network = activeNetworkLink(); const char* network_medium = network.mediumName(); if (network.isConnected()) { strncpy(ip, network.localIP().toString().c_str(), sizeof(ip) - 1); diff --git a/src/helpers/AlertReporter.cpp b/src/helpers/AlertReporter.cpp index c473b274..4a58ad61 100644 --- a/src/helpers/AlertReporter.cpp +++ b/src/helpers/AlertReporter.cpp @@ -6,7 +6,7 @@ #include #ifdef WITH_MQTT_BRIDGE #include "AlertFaultPolicy.h" -#include "NetworkInterface.h" +#include "NetworkLink.h" #endif // Header layout for PAYLOAD_TYPE_GRP_TXT before encryption: diff --git a/src/helpers/CommonCLI_Observer.cpp b/src/helpers/CommonCLI_Observer.cpp index 273666be..f1d757f8 100644 --- a/src/helpers/CommonCLI_Observer.cpp +++ b/src/helpers/CommonCLI_Observer.cpp @@ -15,7 +15,7 @@ #include "TxtDataHelpers.h" #include "AlertReporter.h" // for alertReporterBannedChannelMatch[Hex]() #include "MQTTObserverValidation.h" // pure input validators (host-testable) -#include "NetworkInterface.h" +#include "NetworkLink.h" #include #include #include @@ -416,9 +416,9 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf // runs on the Arduino loop task, shared with mesh/radio processing and the // web config batch, so a synchronous wait of up to 30 s would stall the // node. The sync runs in the background; verify with `get mqtt.ntp.diag`. - if (!activeNetworkInterface().isConnected()) { + if (!activeNetworkLink().isConnected()) { snprintf(reply, 160, "OK - saved (%s not connected; NTP sync pending)", - activeNetworkInterface().mediumName()); + activeNetworkLink().mediumName()); } else if (!_callbacks->isMqttBridgeRunning()) { strcpy(reply, "OK - saved (MQTT bridge not running)"); } else if (_callbacks->syncMqttNtp()) { @@ -460,8 +460,8 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf _mqtt_prefs.wifi_power_save = ps_value; if (!persistObserverPrefs(reply)) return true; #ifdef ESP_PLATFORM - if (strcmp(activeNetworkInterface().mediumName(), "wifi") == 0 && - activeNetworkInterface().isConnected()) { + if (strcmp(activeNetworkLink().mediumName(), "wifi") == 0 && + activeNetworkLink().isConnected()) { wifi_ps_type_t ps_mode = (ps_value == 1) ? WIFI_PS_NONE : (ps_value == 2) ? WIFI_PS_MAX_MODEM : WIFI_PS_MIN_MODEM; esp_err_t ps_result = esp_wifi_set_ps(ps_mode); @@ -962,9 +962,9 @@ bool CommonCLI::handleObserverGetCmd(uint32_t sender_timestamp, const char* conf #ifdef ESP_PLATFORM // Connectivity probe across all configured NTP servers; never updates the clock. // Serial console (sender_timestamp == 0) gets a detailed table; LoRa gets a compact list. - if (!activeNetworkInterface().isConnected()) { + if (!activeNetworkLink().isConnected()) { snprintf(reply, 160, "Error: %s not connected", - activeNetworkInterface().mediumName()); + activeNetworkLink().mediumName()); } else if (!_callbacks->isMqttBridgeRunning()) { strcpy(reply, "Error: MQTT bridge not running"); } else if (!_callbacks->runMqttNtpDiag(reply, 160, sender_timestamp == 0)) { @@ -1039,10 +1039,10 @@ bool CommonCLI::handleObserverGetCmd(uint32_t sender_timestamp, const char* conf strcpy(reply, _mqtt_prefs.wifi_password[0] ? "> ******** (serial only)" : "> (not set)"); } } else if (strcmp(config, "link.diag") == 0) { - activeNetworkInterface().formatDiagnostics(reply, 160); + activeNetworkLink().formatDiagnostics(reply, 160); } else if (memcmp(config, "link.status", 11) == 0 || memcmp(config, "wifi.status", 11) == 0) { - NetworkInterface& network = activeNetworkInterface(); + NetworkLink& network = activeNetworkLink(); const bool wifi_alias = config[0] == 'w'; if (wifi_alias && strcmp(network.mediumName(), "wifi") != 0) { snprintf(reply, 160, "> n/a (%s selected; use get link.status)", @@ -1181,9 +1181,9 @@ bool CommonCLI::handleObserverCommand(uint32_t sender_timestamp, char* command, #ifdef WITH_MQTT_BRIDGE if (memcmp(command, "tls.bundletest ", 15) == 0) { #ifdef ESP_PLATFORM - if (!activeNetworkInterface().isConnected()) { + if (!activeNetworkLink().isConnected()) { snprintf(reply, 160, "ERR: %s not connected", - activeNetworkInterface().mediumName()); + activeNetworkLink().mediumName()); } else { size_t bundle_len = 0; if (rootca_crt_bundle_start != nullptr && @@ -1228,9 +1228,9 @@ bool CommonCLI::handleObserverCommand(uint32_t sender_timestamp, char* command, // ota check -> report available build, do not flash // ota update -> download and flash, then reboot #if defined(WITH_MQTT_BRIDGE) && defined(OTA_MANIFEST_BASE) - if (!activeNetworkInterface().isConnected()) { + if (!activeNetworkLink().isConnected()) { snprintf(reply, 160, "ERR: %s not connected", - activeNetworkInterface().mediumName()); + activeNetworkLink().mediumName()); } else if (memcmp(command, "ota check", 9) == 0) { // Check is synchronous so its result lands in this reply, and runs with the // MQTT bridge UP: the slim per-variant manifest is tiny, so the fetch only diff --git a/src/helpers/ESP32Board.cpp b/src/helpers/ESP32Board.cpp index 20e079b0..19c3e5cc 100644 --- a/src/helpers/ESP32Board.cpp +++ b/src/helpers/ESP32Board.cpp @@ -1,7 +1,7 @@ #ifdef ESP_PLATFORM #include "ESP32Board.h" -#include "NetworkInterface.h" +#include "NetworkLink.h" #include #if defined(ADMIN_PASSWORD) && !defined(DISABLE_WIFI_OTA) // Repeater or Room Server only @@ -46,7 +46,7 @@ void otaReleaseTransport() { ota_server_running = false; ota_started_at = 0; if (ota_network_locked) { - activeNetworkInterface().unlockSwitching(); + activeNetworkLink().unlockSwitching(); ota_network_locked = false; } HttpPort80Lease::release(HttpPort80Lease::Owner::Ota); @@ -67,7 +67,7 @@ bool ESP32Board::startOTAUpdate(const char* id, char reply[], bool force_ap) { } inhibit_sleep = true; // prevent sleep during OTA - activeNetworkInterface().lockSwitching(); + activeNetworkLink().lockSwitching(); ota_network_locked = true; // If the device is already on its selected network, serve ElegantOTA on that @@ -78,8 +78,8 @@ bool ESP32Board::startOTAUpdate(const char* id, char reply[], bool force_ap) { // station IP can't be reached. IPAddress ip; if (NetworkPolicy::startOtaUsesSelectedNetwork( - force_ap, activeNetworkInterface().isConnected())) { - ip = activeNetworkInterface().localIP(); + force_ap, activeNetworkLink().isConnected())) { + ip = activeNetworkLink().localIP(); } else { ota_raised_ap = WiFi.softAP("MeshCore-OTA", NULL); if (!ota_raised_ap) { @@ -290,7 +290,7 @@ bool ESP32Board::otaFromManifest(const char* current_ver, bool dry_run, char rep // mesh-receive call chain (it overflows the loopTask canary). Run the work in a // dedicated 24 KB-stack task and block here until it finishes. The big stack is // freed when the task exits; on a successful update the chip reboots inside it. - NetworkInterface& network = activeNetworkInterface(); + NetworkLink& network = activeNetworkLink(); network.lockSwitching(); OtaTaskArgs args = { this, current_ver, dry_run, reply, false, false }; TaskHandle_t handle = nullptr; @@ -312,9 +312,9 @@ bool ESP32Board::otaFromManifestImpl(const char* current_ver, bool dry_run, char strcpy(reply, "ERR: OTA not configured (build via build.sh)"); return false; #else - if (!activeNetworkInterface().isConnected()) { + if (!activeNetworkLink().isConnected()) { snprintf(reply, 160, "ERR: %s not connected", - activeNetworkInterface().mediumName()); + activeNetworkLink().mediumName()); return false; } diff --git a/src/helpers/NetworkInterface.cpp b/src/helpers/NetworkLink.cpp similarity index 90% rename from src/helpers/NetworkInterface.cpp rename to src/helpers/NetworkLink.cpp index 73f2411d..10dfb23a 100644 --- a/src/helpers/NetworkInterface.cpp +++ b/src/helpers/NetworkLink.cpp @@ -1,4 +1,4 @@ -#include "NetworkInterface.h" +#include "NetworkLink.h" #if defined(ESP_PLATFORM) @@ -21,9 +21,16 @@ extern void tcpipInit(); #endif +// Same "MQTT: " prefix as MQTT_DEBUG_PRINTLN so existing log greps keep matching. +#if defined(MQTT_DEBUG) + #define NETWORK_DEBUG_PRINTLN(F, ...) do { if (Serial.availableForWrite() > 0) { Serial.printf("MQTT: " F "\n", ##__VA_ARGS__); } } while (0) +#else + #define NETWORK_DEBUG_PRINTLN(...) do {} while (0) +#endif + namespace { -class NetworkInterfaceBase : public NetworkInterface { +class NetworkLinkBase : public NetworkLink { protected: std::atomic _outage_bits{AlertFaultPolicy::packOutageSnapshot({false, 0, 0})}; std::atomic _connected_at{0}; @@ -75,7 +82,7 @@ class NetworkInterfaceBase : public NetworkInterface { } }; -class WiFiNetworkInterface final : public NetworkInterfaceBase { +class WiFiNetworkLink final : public NetworkLinkBase { bool _event_registered = false; char _hostname[32] = {}; char _ssid[33] = {}; @@ -118,29 +125,36 @@ class WiFiNetworkInterface final : public NetworkInterfaceBase { _hostname[sizeof(_hostname) - 1] = '\0'; } - bool begin(const char* wifi_ssid, const char* wifi_password) override { - if (!configValid(wifi_ssid)) return false; - strncpy(_ssid, wifi_ssid, sizeof(_ssid) - 1); + void updateWifiCredentials(const char* wifi_ssid, const char* wifi_password) override { + strncpy(_ssid, wifi_ssid ? wifi_ssid : "", sizeof(_ssid) - 1); _ssid[sizeof(_ssid) - 1] = '\0'; strncpy(_password, wifi_password ? wifi_password : "", sizeof(_password) - 1); _password[sizeof(_password) - 1] = '\0'; + } + + bool begin(const char* wifi_ssid, const char* wifi_password) override { + if (!configValid(wifi_ssid)) return false; + updateWifiCredentials(wifi_ssid, wifi_password); // Arduino-ESP32 applies this stored value when it creates the STA netif. // It must be set before WiFi.mode()/begin() for the first DHCP exchange. if (_hostname[0] != '\0') WiFi.setHostname(_hostname); WiFi.mode(WIFI_STA); WiFi.setAutoReconnect(true); - WiFi.setAutoConnect(true); if (!_event_registered) { WiFi.onEvent([this](WiFiEvent_t event, WiFiEventInfo_t info) { switch (event) { case ARDUINO_EVENT_WIFI_STA_GOT_IP: + NETWORK_DEBUG_PRINTLN("WiFi connected: %s", + IPAddress(info.got_ip.ip_info.ip.addr).toString().c_str()); noteConnected(millis()); _reconnect_backoff_attempt = 0; break; case ARDUINO_EVENT_WIFI_STA_DISCONNECTED: noteDisconnected(millis(), info.wifi_sta_disconnected.reason); + NETWORK_DEBUG_PRINTLN("WiFi disconnected: reason %d", + info.wifi_sta_disconnected.reason); break; default: break; @@ -195,7 +209,7 @@ class WiFiNetworkInterface final : public NetworkInterfaceBase { const bool transitioned = _last_connected; if (transitioned) { _connected_at.store(0, std::memory_order_relaxed); - } else if (snapshot.down && MQTTConnectionPolicy::wifiReconnectDue( + } else if (snapshot.down && _ssid[0] != '\0' && MQTTConnectionPolicy::wifiReconnectDue( now_ms, snapshot.started_ms, (uint32_t)_last_reconnect_attempt, _reconnect_backoff_attempt)) { _last_reconnect_attempt = now_ms; @@ -224,7 +238,7 @@ class WiFiNetworkInterface final : public NetworkInterfaceBase { }; #if defined(NETWORK_PREFER_ETHERNET) -class EthernetNetworkInterface final : public NetworkInterfaceBase { +class EthernetNetworkLink final : public NetworkLinkBase { public: enum class EventState : uint8_t { None, @@ -402,9 +416,9 @@ class EthernetNetworkInterface final : public NetworkInterfaceBase { } }; -class AutomaticNetworkInterface final : public NetworkInterface { - EthernetNetworkInterface _ethernet; - WiFiNetworkInterface _wifi; +class AutomaticNetworkLink final : public NetworkLink { + EthernetNetworkLink _ethernet; + WiFiNetworkLink _wifi; std::atomic _selected{NetworkMedium::None}; std::atomic _ethernet_started{false}; std::atomic _wifi_started{false}; @@ -417,16 +431,23 @@ class AutomaticNetworkInterface final : public NetworkInterface { std::atomic _ethernet_no_ip_since{0}; std::atomic _switch_locks{0}; std::atomic _switch_in_progress{false}; + std::atomic _ethernet_seen{false}; // held a lease at least once this boot - NetworkInterface& selectedInterface(NetworkMedium selected) { - return selected == NetworkMedium::Ethernet - ? static_cast(_ethernet) - : static_cast(_wifi); + // Ethernet is the primary for alerts once it has carried traffic, or when it + // is the only configured medium; a Wi-Fi-only install keeps Wi-Fi alerts. + bool ethernetIsAlertPrimary() const { + return _ethernet_seen.load(std::memory_order_acquire) || !wifiConfigured(); } - const NetworkInterface& selectedInterface(NetworkMedium selected) const { + + NetworkLink& selectedInterface(NetworkMedium selected) { return selected == NetworkMedium::Ethernet - ? static_cast(_ethernet) - : static_cast(_wifi); + ? static_cast(_ethernet) + : static_cast(_wifi); + } + const NetworkLink& selectedInterface(NetworkMedium selected) const { + return selected == NetworkMedium::Ethernet + ? static_cast(_ethernet) + : static_cast(_wifi); } bool wifiConfigured() const { return _wifi_ssid[0] != '\0'; } @@ -444,14 +465,14 @@ class AutomaticNetworkInterface final : public NetworkInterface { std::memory_order_release); } + // Lock and mutation each publish one flag then read the other; seq_cst keeps + // both sides from missing each other. bool beginUnlockedMutation() { bool expected = false; - if (!_switch_in_progress.compare_exchange_strong( - expected, true, std::memory_order_acq_rel, - std::memory_order_relaxed)) { + if (!_switch_in_progress.compare_exchange_strong(expected, true)) { return false; } - if (_switch_locks.load(std::memory_order_acquire) != 0) { + if (_switch_locks.load() != 0) { _switch_in_progress.store(false, std::memory_order_release); return false; } @@ -532,6 +553,11 @@ class AutomaticNetworkInterface final : public NetworkInterface { } bool isAutomatic() const override { return true; } + void updateWifiCredentials(const char* wifi_ssid, const char* wifi_password) override { + rememberWifi(wifi_ssid, wifi_password); + _wifi.updateWifiCredentials(_wifi_ssid, _wifi_password); + } + void setHostname(const char* hostname) override { // Whichever medium wins now or during a later failover presents the same // stable DHCP identity to the LAN. @@ -589,6 +615,7 @@ class AutomaticNetworkInterface final : public NetworkInterface { delay(25); } + if (_ethernet.isConnected()) _ethernet_seen.store(true, std::memory_order_release); const NetworkMedium initial = NetworkPolicy::bootSelection( _ethernet.isConnected(), wifiConfigured()); if (initial == NetworkMedium::Ethernet) { @@ -606,7 +633,7 @@ class AutomaticNetworkInterface final : public NetworkInterface { _switch_locks.load(std::memory_order_acquire) != 0; const bool ethernet_stopped = ethernet_started && - _ethernet.eventState() == EthernetNetworkInterface::EventState::Stopped; + _ethernet.eventState() == EthernetNetworkLink::EventState::Stopped; if (ethernet_stopped) { _ethernet_started.store(false, std::memory_order_release); ethernet_started = false; @@ -615,11 +642,11 @@ class AutomaticNetworkInterface final : public NetworkInterface { _last_ethernet_init_attempt.store(now_ms, std::memory_order_relaxed); } } - const EthernetNetworkInterface::EventState ethernet_event = + const EthernetNetworkLink::EventState ethernet_event = _ethernet.eventState(); const bool ethernet_link_up = - ethernet_event == EthernetNetworkInterface::EventState::LinkUp || - ethernet_event == EthernetNetworkInterface::EventState::GotIp; + ethernet_event == EthernetNetworkLink::EventState::LinkUp || + ethernet_event == EthernetNetworkLink::EventState::GotIp; if (ethernet_started && ethernet_link_up && !_ethernet.isConnected()) { if (_ethernet_no_ip_since.load(std::memory_order_relaxed) == 0) { uint32_t started_at = now_ms; @@ -647,6 +674,7 @@ class AutomaticNetworkInterface final : public NetworkInterface { const NetworkTransition ethernet_transition = _ethernet.maintain(now_ms, wifi_power_save); + if (_ethernet.isConnected()) _ethernet_seen.store(true, std::memory_order_release); const bool wifi_started = _wifi_started.load(std::memory_order_acquire); const NetworkTransition wifi_transition = wifi_started ? _wifi.maintain(now_ms, wifi_power_save) @@ -721,9 +749,8 @@ class AutomaticNetworkInterface final : public NetworkInterface { void lockSwitching() override { uint8_t value = _switch_locks.load(std::memory_order_relaxed); while (value != UINT8_MAX && !_switch_locks.compare_exchange_weak( - value, static_cast(value + 1), - std::memory_order_acq_rel, std::memory_order_relaxed)) {} - while (_switch_in_progress.load(std::memory_order_acquire)) delay(1); + value, static_cast(value + 1))) {} + while (_switch_in_progress.load()) delay(1); } void unlockSwitching() override { uint8_t value = _switch_locks.load(std::memory_order_relaxed); @@ -819,21 +846,21 @@ class AutomaticNetworkInterface final : public NetworkInterface { : selectedInterface(selected).outageSnapshot(); } NetworkMedium alertMedium() const override { - return NetworkMedium::Ethernet; + return ethernetIsAlertPrimary() ? NetworkMedium::Ethernet : NetworkMedium::WiFi; } AlertFaultPolicy::OutageSnapshot alertOutageSnapshot() const override { - return _ethernet.outageSnapshot(); + return ethernetIsAlertPrimary() ? _ethernet.outageSnapshot() : _wifi.outageSnapshot(); } }; #endif } // namespace -NetworkInterface& activeNetworkInterface() { +NetworkLink& activeNetworkLink() { #if defined(NETWORK_PREFER_ETHERNET) - static AutomaticNetworkInterface network; + static AutomaticNetworkLink network; #else - static WiFiNetworkInterface network; + static WiFiNetworkLink network; #endif return network; } diff --git a/src/helpers/NetworkInterface.h b/src/helpers/NetworkLink.h similarity index 89% rename from src/helpers/NetworkInterface.h rename to src/helpers/NetworkLink.h index 339c1470..afc125a9 100644 --- a/src/helpers/NetworkInterface.h +++ b/src/helpers/NetworkLink.h @@ -20,9 +20,9 @@ * MQTT shutdown deliberately does not stop this interface because an OTA * download runs after the broker clients have been released. */ -class NetworkInterface { +class NetworkLink { public: - virtual ~NetworkInterface() = default; + virtual ~NetworkLink() = default; virtual const char* mediumName() const = 0; virtual NetworkMedium medium() const = 0; @@ -33,6 +33,11 @@ class NetworkInterface { // retain it for any later fallback interface start. virtual void setHostname(const char* hostname) = 0; virtual bool begin(const char* wifi_ssid, const char* wifi_password) = 0; + // Latest stored credentials; the next reconnect attempt uses them without a reboot. + virtual void updateWifiCredentials(const char* wifi_ssid, const char* wifi_password) { + (void)wifi_ssid; + (void)wifi_password; + } virtual NetworkTransition maintain(uint32_t now_ms, uint8_t wifi_power_save) = 0; // Automatic selectors are boot-owned so first-run services can use the @@ -71,6 +76,6 @@ class NetworkInterface { }; /** Build-selected singleton. Wi-Fi is the compatibility default. */ -NetworkInterface& activeNetworkInterface(); +NetworkLink& activeNetworkLink(); #endif diff --git a/src/helpers/SNMPAgent.cpp b/src/helpers/SNMPAgent.cpp index 8394a588..eb29bd68 100644 --- a/src/helpers/SNMPAgent.cpp +++ b/src/helpers/SNMPAgent.cpp @@ -1,7 +1,7 @@ #ifdef WITH_SNMP #include "SNMPAgent.h" -#include "NetworkInterface.h" +#include "NetworkLink.h" #include #include @@ -79,7 +79,7 @@ void MeshSNMPAgent::loop() { _psram_free = 0; #endif - const int signal = activeNetworkInterface().rssi(); + const int signal = activeNetworkLink().rssi(); _wifi_rssi = signal == INT_MIN ? -127 : signal; _snmp.loop(); diff --git a/src/helpers/SNMPAgent.h b/src/helpers/SNMPAgent.h index 2f792634..fe94b10f 100644 --- a/src/helpers/SNMPAgent.h +++ b/src/helpers/SNMPAgent.h @@ -14,7 +14,7 @@ // .2.x.0 = radio (packets, RSSI, SNR, noise floor, air time) // .3.x.0 = mqtt (connected slots, queue depth, skipped publishes) // .4.x.0 = memory (free heap, max alloc, internal free, PSRAM free) -// .5.x.0 = network (RSSI, or 0 when the selected medium has no RSSI) +// .5.x.0 = network (RSSI, or -127 when disconnected or the medium has no RSSI) class MeshSNMPAgent { public: diff --git a/src/helpers/bridges/MQTTBridge.cpp b/src/helpers/bridges/MQTTBridge.cpp index 78273d3a..a9d72775 100644 --- a/src/helpers/bridges/MQTTBridge.cpp +++ b/src/helpers/bridges/MQTTBridge.cpp @@ -101,7 +101,7 @@ void MQTTBridge::getEffectiveMqttOrigin(const NodePrefs* np, const MQTTPrefs* ob } static bool isNetworkConfigValid(const MQTTPrefs* obs) { - return obs && activeNetworkInterface().configValid(obs->wifi_ssid); + return obs && activeNetworkLink().configValid(obs->wifi_ssid); } #ifdef WITH_MQTT_BRIDGE @@ -221,7 +221,7 @@ static void agentLogHeap(const char* location, const char* message, const char* static MQTTBridge* s_mqtt_bridge_instance = nullptr; unsigned long MQTTBridge::getWifiConnectedAtMillis() { - return activeNetworkInterface().connectedAtMillis(); + return activeNetworkLink().connectedAtMillis(); } #if defined(WITH_MQTT_NEIGHBORS) @@ -413,10 +413,10 @@ int MQTTBridge::getMaxActiveSlots() { } uint8_t MQTTBridge::getLastWifiDisconnectReason() { - return activeNetworkInterface().lastDisconnectReason(); + return activeNetworkLink().lastDisconnectReason(); } unsigned long MQTTBridge::getLastWifiDisconnectTime() { - return activeNetworkInterface().lastDisconnectTime(); + return activeNetworkLink().lastDisconnectTime(); } unsigned long MQTTBridge::getSlotCurrentOutageStartMs(int slot_index) const { @@ -624,7 +624,7 @@ static inline uint32_t mqttStopTimeoutForSlots(int slots) { MQTTBridge::MQTTBridge(NodePrefs *prefs, MQTTPrefs *obs, mesh::PacketManager *mgr, mesh::RTCClock *rtc, mesh::LocalIdentity *identity) : BridgeBase(prefs, mgr, rtc), _obs(obs), - _network(&activeNetworkInterface()), + _network(&activeNetworkLink()), _queue_count(0), _last_status_publish(0), _last_status_retry(0), _status_interval(300000), _ntp_client(_ntp_udp, effectiveNtpPrimary(obs), 0, 60000), _last_ntp_sync(0), _ntp_synced(false), _ntp_sync_pending(false), _slots_setup_done(false), _max_active_slots(RUNTIME_MQTT_SLOTS), @@ -2740,6 +2740,7 @@ void MQTTBridge::checkConfigurationMismatch() { bool MQTTBridge::handleNetworkConnection(unsigned long now) { const NetworkMedium previous_medium = _network->medium(); + _network->updateWifiCredentials(_obs->wifi_ssid, _obs->wifi_password); const NetworkTransition transition = _network->maintain((uint32_t)now, _obs->wifi_power_save); const NetworkMedium selected_medium = _network->medium(); diff --git a/src/helpers/bridges/MQTTBridge.h b/src/helpers/bridges/MQTTBridge.h index 04e27649..46ef18d4 100644 --- a/src/helpers/bridges/MQTTBridge.h +++ b/src/helpers/bridges/MQTTBridge.h @@ -2,7 +2,7 @@ #include "MeshCore.h" #include "helpers/bridges/BridgeBase.h" -#include "helpers/NetworkInterface.h" +#include "helpers/NetworkLink.h" #include #include #include @@ -525,7 +525,7 @@ private: // Observer config (MQTT/WiFi/timezone/SNMP/alert), persisted to /mqtt.json. // _prefs (held by BridgeBase) still provides upstream fields (freq/sf/node_name…). MQTTPrefs* _obs = nullptr; - NetworkInterface* _network = nullptr; + NetworkLink* _network = nullptr; public: MQTTBridge(NodePrefs *prefs, MQTTPrefs *obs, mesh::PacketManager *mgr, mesh::RTCClock *rtc, mesh::LocalIdentity *identity); diff --git a/src/helpers/esp32/WebConfigServer.cpp b/src/helpers/esp32/WebConfigServer.cpp index 5c75a1f1..510d9b75 100644 --- a/src/helpers/esp32/WebConfigServer.cpp +++ b/src/helpers/esp32/WebConfigServer.cpp @@ -13,7 +13,7 @@ #include #include -#include +#include #include #include #include @@ -232,7 +232,7 @@ bool WebConfigServer::getSetupInfo(char* ssid, size_t ssid_len, char* ip, size_t } if (ip && ip_len > 0) { const IPAddress address = w->_mode == MODE_LAN - ? activeNetworkInterface().localIP() : WiFi.softAPIP(); + ? activeNetworkLink().localIP() : WiFi.softAPIP(); snprintf(ip, ip_len, "%s", address.toString().c_str()); } return true; @@ -291,7 +291,7 @@ bool WebConfigServer::startSetupMode(char reply[]) { _mode = MODE_SETUP; _initial_setup = !mqttNetworkSetupComplete(_obs); createServer(); - activeNetworkInterface().lockSwitching(); + activeNetworkLink().lockSwitching(); _network_locked = true; _was_setup_ap = true; _last_activity = millis(); @@ -311,10 +311,10 @@ bool WebConfigServer::startLanMode(IPAddress ip, bool initial_setup, char reply[ HttpPort80Lease::ownerName()); return false; } - activeNetworkInterface().lockSwitching(); + activeNetworkLink().lockSwitching(); _network_locked = true; - if (!activeNetworkInterface().isConnected() || ip == IPAddress()) { - activeNetworkInterface().unlockSwitching(); + if (!activeNetworkLink().isConnected() || ip == IPAddress()) { + activeNetworkLink().unlockSwitching(); _network_locked = false; HttpPort80Lease::release(HttpPort80Lease::Owner::WebConfig); strcpy(reply, "Err: selected network not connected"); @@ -324,7 +324,7 @@ bool WebConfigServer::startLanMode(IPAddress ip, bool initial_setup, char reply[ uint8_t session_entropy[sizeof(_session_secret) + 6]; const size_t entropy_size = sizeof(_session_secret) + (_initial_setup ? 6 : 0); if (!fillRandomBytes(session_entropy, entropy_size)) { - activeNetworkInterface().unlockSwitching(); + activeNetworkLink().unlockSwitching(); _network_locked = false; HttpPort80Lease::release(HttpPort80Lease::Owner::WebConfig); strcpy(reply, "Err: secure random source unavailable"); @@ -445,7 +445,7 @@ void WebConfigServer::finalizeTeardown() { _setup_code[0] = 0; _setup_reminder_at = 0; if (_network_locked) { - activeNetworkInterface().unlockSwitching(); + activeNetworkLink().unlockSwitching(); _network_locked = false; } HttpPort80Lease::release(HttpPort80Lease::Owner::WebConfig); @@ -474,7 +474,7 @@ void WebConfigServer::tick(uint32_t now) { if (_mode == MODE_LAN && _initial_setup && _setup_reminder_at != 0 && (int32_t)(now - _setup_reminder_at) >= 0) { Serial.printf("WC: Ethernet setup http://%s/ code %s\n", - activeNetworkInterface().localIP().toString().c_str(), + activeNetworkLink().localIP().toString().c_str(), _setup_code); _setup_reminder_at = now + 60000; if (_setup_reminder_at == 0) _setup_reminder_at = 1; @@ -581,8 +581,8 @@ void WebConfigServer::drainBatch(uint32_t now) { return; // more commands next tick } } - if (_initial_setup && _admin_pwd_set && _batch_all_ok && - (_mode == MODE_LAN || _obs->wifi_ssid[0] != '\0')) { + // Wi-Fi onboarding is recorded by the stored SSID; only Ethernet LAN setup needs the marker. + if (_initial_setup && _admin_pwd_set && _batch_all_ok && _mode == MODE_LAN) { if (_cb->onInitialSetupComplete()) { _initial_setup = false; _setup_code[0] = 0; diff --git a/test/test_mqtt_prefs_serializer/test_mqtt_prefs_serializer.cpp b/test/test_mqtt_prefs_serializer/test_mqtt_prefs_serializer.cpp index 3427bb60..d94c658c 100644 --- a/test/test_mqtt_prefs_serializer/test_mqtt_prefs_serializer.cpp +++ b/test/test_mqtt_prefs_serializer/test_mqtt_prefs_serializer.cpp @@ -485,6 +485,20 @@ TEST(MQTTPrefsSerializer, UnknownGroupsAreIgnoredSoAppendedKeysAreDowngradeSafe) EXPECT_EQ(45, prefs.display_timeout_secs); } +TEST(MQTTPrefsSerializer, UnknownKeysInsideAKnownGroupAreDowngradeSafe) { + // Firmware that predates wifi.setup_complete sees it as an unknown key in a + // known group; the rest of the group must still load. + MQTTPrefs prefs = defaults(); + InputStream input( + "{version:1,wifi:{ssid:\"home\",future_key:1,power_save:2}}"); + MQTTPrefsSerializer serializer(&prefs); + ASSERT_TRUE(serializer.loadSerial(input)); + bool repaired = false; + ASSERT_TRUE(serializer.apply(&repaired)); + EXPECT_STREQ("home", prefs.wifi_ssid); + EXPECT_EQ(2, prefs.wifi_power_save); +} + int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS();