From cae7e86d5d826a3cfabc11c73f2e337e58ce3e50 Mon Sep 17 00:00:00 2001 From: agessaman Date: Fri, 14 Aug 2026 18:41:20 -0700 Subject: [PATCH] fix(mqtt): reconnect the NTP-corrected slot instead of no-opping on a live client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit esp_mqtt_client_reconnect() is honoured only from MQTT_STATE_WAIT_RECONNECT, so the post-correction path minted a fresh token, staged it, and then asked a connected client to reconnect — a request esp-mqtt refuses. The slot kept running on the token the clock correction had just proven stale, and recovery became broker-driven rather than the clean reconnect this code intends. Split the three states the path can find: a stopped or waiting client goes through reconnectSlotClient() as before, and a live one has its transport closed first. Only where the broker enforces exp, though — waev leaves live sessions alone past expiry, so bouncing it would spend the 16 KiB contiguous handshake that the rest of this branch exists to avoid. Not a regression: the base branch called client->reconnect() directly at the same site. On ESP32 the block is reachable from the WiFi-reconnect resync and the CLI forced sync; the hourly refresh uses refreshNTP(), which does not carry it. --- src/helpers/bridges/MQTTBridge.cpp | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/src/helpers/bridges/MQTTBridge.cpp b/src/helpers/bridges/MQTTBridge.cpp index 7983f1617..a325d0ab3 100644 --- a/src/helpers/bridges/MQTTBridge.cpp +++ b/src/helpers/bridges/MQTTBridge.cpp @@ -4015,8 +4015,8 @@ bool MQTTBridge::syncTimeWithNTP(bool force, bool primary_only) { MQTT_DEBUG_PRINTLN("Time synced: %lu (via %s)", epochTime, ntp_server_used); // If slots are already set up and the time jumped significantly (e.g., SNTP - // initially returned stale RTC time, then a later sync corrected it), tear down - // and re-setup all JWT-authenticated slots so they get fresh tokens. + // initially returned stale RTC time, then a later sync corrected it), re-issue + // credentials for every JWT slot the correction left holding an expired token. if (_slots_setup_done && was_ntp_synced) { unsigned long current_time = (unsigned long)time(nullptr); // Every slot, not _max_active_slots: that is a count of positions, never an @@ -4033,12 +4033,27 @@ bool MQTTBridge::syncTimeWithNTP(bool force, bool primary_only) { // in place and reconnect the persistent client. No teardown needed. if (_slots[i].token_expires_at > 0 && current_time > _slots[i].token_expires_at) { MQTT_DEBUG_PRINTLN("MQTT%d token stale after time correction, re-creating", i + 1); - if (createSlotAuthToken(i)) { + const bool minted = createSlotAuthToken(i); + if (minted) { + // Staged only; the config is applied by connect()/reconnect() below. _slots[i].client->setCredentials(_jwt_username, _slots[i].auth_token); } - // Reuse the transport — the fault is stale credentials, not the transport — - // but via the helper, so a stopped client is started rather than no-opped. - reconnectSlotClient(i); + if (!_slots[i].client->connected()) { + // Reuse the transport — the fault is stale credentials, not the transport — + // but via the helper, so a stopped client is started rather than no-opped. + reconnectSlotClient(i); + } else if (minted && mqttPresetEnforcesTokenExp(_slots[i].preset)) { + // esp-mqtt honours reconnect() only from WAIT_RECONNECT, so on a live + // session it is refused and the slot keeps running on the stale token. + // Close the transport first — and only here, where the broker enforces + // exp: elsewhere that handshake buys nothing. + MQTT_DEBUG_PRINTLN("MQTT%d bouncing for the corrected-clock token", i + 1); + _slots[i].client->softDisconnect(); + _slots[i].client->reconnect(); + } else if (minted) { + MQTT_DEBUG_PRINTLN("MQTT%d token re-created, no bounce (broker does not enforce exp)", + i + 1); + } } } }