From 7aed59075b07af9906efb82c6dc8275393778b82 Mon Sep 17 00:00:00 2001 From: agessaman Date: Sat, 19 Sep 2026 10:23:12 -0700 Subject: [PATCH] fix(network): bound link-transition teardown and gate Ethernet restarts - Link down/switch edges now softDisconnect() every started slot instead of disconnect(). The full stop could wait forever for a DISCONNECTED event on the Core 0 MQTT task, and it destroyed and recreated every slot's esp-mqtt task on each Wi-Fi drop, where the bridge previously stopped none (measured: 62 s deauth, five slots, zero stops). softDisconnect() is bounded and keeps the task; reconnectSlotClient() then calls reconnect() on the new route. Rename the transition action to disconnect_started_slots to match. - Ethernet no-IP recovery and init retries rebuild the CH390 netif, so they now run inside the route-switch mutation gate. An OTA/WebConfig lock taken after maintain() samples the lock count can no longer have the interface torn down underneath it. --- MQTT_IMPLEMENTATION.md | 4 ++-- src/helpers/NetworkLink.cpp | 14 ++++++++++--- src/helpers/NetworkPolicy.h | 2 +- src/helpers/bridges/MQTTBridge.cpp | 20 ++++++++----------- .../test_network_policy.cpp | 8 ++++---- 5 files changed, 26 insertions(+), 22 deletions(-) diff --git a/MQTT_IMPLEMENTATION.md b/MQTT_IMPLEMENTATION.md index aeb4ebc3..67ecebea 100644 --- a/MQTT_IMPLEMENTATION.md +++ b/MQTT_IMPLEMENTATION.md @@ -874,8 +874,8 @@ the radio actually performs in that case. selection log includes the measured probe duration. - 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. When the same link returns, +- Ethernet/WiFi transitions are logged. A lost or changed route closes every live MQTT + transport with a bounded disconnect that keeps the client task. 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 diff --git a/src/helpers/NetworkLink.cpp b/src/helpers/NetworkLink.cpp index 10dfb23a..6314d181 100644 --- a/src/helpers/NetworkLink.cpp +++ b/src/helpers/NetworkLink.cpp @@ -573,8 +573,10 @@ class AutomaticNetworkLink final : public NetworkLink { _ethernet_retry_attempt.load(std::memory_order_relaxed); if (NetworkPolicy::ethernetInitRetryDue( attempt, now_ms, - _last_ethernet_init_attempt.load(std::memory_order_relaxed))) { + _last_ethernet_init_attempt.load(std::memory_order_relaxed)) && + beginUnlockedMutation()) { startOrRetryEthernet(now_ms, attempt != 0); + endUnlockedMutation(); } } // bootstrap() owns the initial choice. MQTT begin() is intentionally @@ -656,19 +658,25 @@ class AutomaticNetworkLink final : public NetworkLink { } else { _ethernet_no_ip_since.store(0, std::memory_order_relaxed); } + // Restarts rebuild the CH390 netif, so they take the same mutation gate as + // a route switch: an OTA/WebConfig lock taken after the sample above wins. if (!switching_locked && NetworkPolicy::ethernetNoIpRecoveryDue( ethernet_started, ethernet_link_up, _ethernet.isConnected(), - now_ms, _ethernet_no_ip_since.load(std::memory_order_relaxed))) { + now_ms, _ethernet_no_ip_since.load(std::memory_order_relaxed)) && + beginUnlockedMutation()) { ethernet_started = startOrRetryEthernet(now_ms, true); _ethernet_no_ip_since.store(0, std::memory_order_relaxed); + endUnlockedMutation(); } if (!ethernet_started && !switching_locked) { const uint8_t attempt = _ethernet_retry_attempt.load(std::memory_order_relaxed); if (NetworkPolicy::ethernetInitRetryDue( attempt, now_ms, - _last_ethernet_init_attempt.load(std::memory_order_relaxed))) { + _last_ethernet_init_attempt.load(std::memory_order_relaxed)) && + beginUnlockedMutation()) { ethernet_started = startOrRetryEthernet(now_ms, true); + endUnlockedMutation(); } } diff --git a/src/helpers/NetworkPolicy.h b/src/helpers/NetworkPolicy.h index 10d7da69..a4e7953a 100644 --- a/src/helpers/NetworkPolicy.h +++ b/src/helpers/NetworkPolicy.h @@ -29,7 +29,7 @@ enum class NetworkDiagnosticReason : uint8_t { namespace NetworkPolicy { struct MQTTTransitionActions { - bool stop_started_slots; + bool disconnect_started_slots; bool retry_disconnected_slots_now; bool reset_reconnect_backoff; }; diff --git a/src/helpers/bridges/MQTTBridge.cpp b/src/helpers/bridges/MQTTBridge.cpp index a9d72775..0f3c67cc 100644 --- a/src/helpers/bridges/MQTTBridge.cpp +++ b/src/helpers/bridges/MQTTBridge.cpp @@ -1986,10 +1986,8 @@ void MQTTBridge::teardownSlot(int index, bool force) { // A stopped client needs connect(): reconnect() is a documented no-op on one, so reaching // it here would strand the slot. The producer is a failed esp_mqtt_client_start(), which -// leaves _started false while initial_connect_done stays set. Not the WiFi-drop teardown, -// which only stops slots still marked connected: a publishing slot's socket fails first, so -// the guard skips it — measured across a 62 s deauth, five slots, zero stops. An idle slot -// with no traffic to fail on is the one case that could still reach here that way. +// leaves _started false while initial_connect_done stays set. Not a network transition, +// which only softDisconnect()s and keeps the client task. void MQTTBridge::reconnectSlotClient(int index) { if (index < 0 || index >= RUNTIME_MQTT_SLOTS) return; MQTTSlot& slot = _slots[index]; @@ -2760,16 +2758,14 @@ bool MQTTBridge::handleNetworkConnection(unsigned long now) { const NetworkPolicy::MQTTTransitionActions actions = NetworkPolicy::mqttActions(transition); - if (actions.stop_started_slots) { - // Broker ownership stays in the bridge. The physical adapter reports the - // edge; the bridge explicitly stops every started slot instead of waiting - // for eventual socket timeouts. Do not gate this on slot.connected: the - // ESP-MQTT disconnect callback can clear that flag before the network edge - // reaches this task, but its client task and transport can still be alive. + if (actions.disconnect_started_slots) { + // Close each live transport now instead of waiting for socket timeouts, but + // keep the esp-mqtt task: softDisconnect() is bounded where a full stop can + // wait forever, and reconnectSlotClient() then reconnects on the new route. for (int i = 0; i < RUNTIME_MQTT_SLOTS; i++) { if (_slots[i].client && _slots[i].client->isStarted()) { - MQTT_DEBUG_PRINTLN("MQTT%d stopping for network transition", i + 1); - _slots[i].client->disconnect(); + MQTT_DEBUG_PRINTLN("MQTT%d disconnecting for network transition", i + 1); + _slots[i].client->softDisconnect(); } _slots[i].connected = false; _slots[i].connected_at_ms = 0; diff --git a/test/test_network_policy/test_network_policy.cpp b/test/test_network_policy/test_network_policy.cpp index 7a057918..3558e1e3 100644 --- a/test/test_network_policy/test_network_policy.cpp +++ b/test/test_network_policy/test_network_policy.cpp @@ -4,28 +4,28 @@ TEST(NetworkPolicy, MqttDownDisconnectsSlotsWithoutRequestingImmediateRetry) { const auto actions = NetworkPolicy::mqttActions(NetworkTransition::Down); - EXPECT_TRUE(actions.stop_started_slots); + EXPECT_TRUE(actions.disconnect_started_slots); EXPECT_FALSE(actions.retry_disconnected_slots_now); EXPECT_FALSE(actions.reset_reconnect_backoff); } TEST(NetworkPolicy, MqttUpRetriesWithoutClearingBrokerCircuitBreaker) { const auto actions = NetworkPolicy::mqttActions(NetworkTransition::Up); - EXPECT_FALSE(actions.stop_started_slots); + EXPECT_FALSE(actions.disconnect_started_slots); EXPECT_TRUE(actions.retry_disconnected_slots_now); EXPECT_FALSE(actions.reset_reconnect_backoff); } TEST(NetworkPolicy, NoTransitionHasNoMqttSideEffects) { const auto actions = NetworkPolicy::mqttActions(NetworkTransition::None); - EXPECT_FALSE(actions.stop_started_slots); + EXPECT_FALSE(actions.disconnect_started_slots); EXPECT_FALSE(actions.retry_disconnected_slots_now); EXPECT_FALSE(actions.reset_reconnect_backoff); } TEST(NetworkPolicy, LinkSwitchReconnectsMqttSlotsImmediately) { const auto actions = NetworkPolicy::mqttActions(NetworkTransition::Switched); - EXPECT_TRUE(actions.stop_started_slots); + EXPECT_TRUE(actions.disconnect_started_slots); EXPECT_TRUE(actions.retry_disconnected_slots_now); EXPECT_TRUE(actions.reset_reconnect_backoff); }