From ea37542b416a620d9cc799c2b38d9fc8c6788780 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 23 Mar 2026 13:05:15 -1000 Subject: [PATCH 1/3] [wifi] Track roam target BSSID to detect successful roam via retry When a roam connection fails on the first attempt, retry_connect() transitions to RECONNECTING. The subsequent scan-based retry may connect to the same better AP, but the success handler treated all RECONNECTING connections as failed roams and preserved the attempts counter. This meant a successful roam (that needed two tries) would incorrectly consume an attempt. Fix by storing the roam target BSSID and checking it on reconnection. If the device connected to the intended target, reset the counter (successful roam). If it fell back to a different AP, preserve the counter (failed roam, prevent ping-pong). --- esphome/components/wifi/wifi_component.cpp | 37 ++++++++++++++++------ esphome/components/wifi/wifi_component.h | 3 +- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/esphome/components/wifi/wifi_component.cpp b/esphome/components/wifi/wifi_component.cpp index 7f8e5269e8..714bf635ca 100644 --- a/esphome/components/wifi/wifi_component.cpp +++ b/esphome/components/wifi/wifi_component.cpp @@ -287,19 +287,25 @@ bool CompactString::operator==(const StringRef &other) const { /// │ │ (counter reset to 0) │ │ (retry_connect called) │ /// │ └──────────────────────────────────┘ └───────────┬─────────────┘ /// │ │ │ -/// │ ↓ │ -/// │ ┌───────────────────────┐ │ -/// │ │ → IDLE │ │ -/// │ │ (counter preserved!) │ │ -/// │ └───────────────────────┘ │ +/// │ ┌─────────┴─────────┐ │ +/// │ ↓ ↓ │ +/// │ on target BSSID on other AP │ +/// │ │ │ │ +/// │ ↓ ↓ │ +/// │ ┌──────────────────┐ ┌────────────┐│ +/// │ │ → IDLE │ │ → IDLE ││ +/// │ │ (counter reset) │ │ (counter ││ +/// │ │ (roam worked!) │ │ preserved)││ +/// │ └──────────────────┘ └────────────┘│ /// │ │ /// │ Key behaviors: │ /// │ - After 3 checks: attempts >= 3, stop checking │ /// │ - Non-roaming disconnect: clear_roaming_state_() resets counter │ -/// │ - Disconnect during scan (SCANNING→RECONNECTING): counter preserved │ +/// │ - Disconnect during scan (SCANNING→RECONNECTING): counter preserved │ /// │ - Disconnect after scan (within grace period): counter preserved │ /// │ - Roaming success (CONNECTING→IDLE): counter reset (can roam again) │ -/// │ - Roaming fail (RECONNECTING→IDLE): counter preserved (ping-pong) │ +/// │ - Roaming success via retry (on target BSSID): counter reset │ +/// │ - Roaming fail (RECONNECTING on other AP): counter preserved │ /// └──────────────────────────────────────────────────────────────────────┘ // Use if-chain instead of switch to avoid jump table in RODATA (wastes RAM on ESP8266) @@ -1577,17 +1583,26 @@ void WiFiComponent::check_connecting_finished(uint32_t now) { // Only preserve attempts if reconnecting after a failed roam attempt // This prevents ping-pong between APs when a roam target is unreachable if (this->roaming_state_ == RoamingState::CONNECTING) { - // Successful roam to better AP - reset attempts so we can roam again later + // Successful roam to better AP on first try - reset attempts so we can roam again later ESP_LOGD(TAG, "Roam successful"); this->roaming_attempts_ = 0; } else if (this->roaming_state_ == RoamingState::RECONNECTING) { - // Failed roam, reconnected via normal recovery - keep attempts to prevent ping-pong - ESP_LOGD(TAG, "Reconnected after failed roam (attempt %u/%u)", this->roaming_attempts_, ROAMING_MAX_ATTEMPTS); + // Check if we ended up on the roam target despite needing a retry + // (e.g., first connect failed but scan-based retry found and connected to the same better AP) + bssid_t current_bssid = this->wifi_bssid(); + if (this->roaming_target_bssid_ != bssid_t{} && current_bssid == this->roaming_target_bssid_) { + ESP_LOGD(TAG, "Roam successful (via retry)"); + this->roaming_attempts_ = 0; + } else { + // Failed roam, reconnected to different AP - keep attempts to prevent ping-pong + ESP_LOGD(TAG, "Reconnected after failed roam (attempt %u/%u)", this->roaming_attempts_, ROAMING_MAX_ATTEMPTS); + } } else { // Normal connection (boot, credentials changed, etc.) this->roaming_attempts_ = 0; } this->roaming_state_ = RoamingState::IDLE; + this->roaming_target_bssid_ = {}; // Clear all priority penalties - the next reconnect will happen when an AP disconnects, // which means the landscape has likely changed and previous tracked failures are stale @@ -2317,6 +2332,7 @@ void WiFiComponent::clear_roaming_state_() { this->roaming_attempts_ = 0; this->roaming_last_check_ = 0; this->roaming_scan_end_ = 0; + this->roaming_target_bssid_ = {}; this->roaming_state_ = RoamingState::IDLE; } @@ -2453,6 +2469,7 @@ void WiFiComponent::process_roaming_scan_() { // Mark as roaming attempt - affects retry behavior if connection fails this->roaming_state_ = RoamingState::CONNECTING; + this->roaming_target_bssid_ = best->get_bssid(); // Connect directly - wifi_sta_connect_ handles disconnect internally this->start_connecting(roam_params); diff --git a/esphome/components/wifi/wifi_component.h b/esphome/components/wifi/wifi_component.h index 427b1d3a5d..8c7e1f9be6 100644 --- a/esphome/components/wifi/wifi_component.h +++ b/esphome/components/wifi/wifi_component.h @@ -790,7 +790,8 @@ class WiFiComponent final : public Component { uint32_t last_connected_{0}; uint32_t reboot_timeout_{}; uint32_t roaming_last_check_{0}; - uint32_t roaming_scan_end_{0}; // Timestamp when last roaming scan completed + uint32_t roaming_scan_end_{0}; // Timestamp when last roaming scan completed + bssid_t roaming_target_bssid_{}; // BSSID of the AP we're trying to roam to #ifdef USE_WIFI_AP uint32_t ap_timeout_{}; #endif From b357758369270371ae2a89790b609074a0fa2dc7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 23 Mar 2026 13:09:37 -1000 Subject: [PATCH 2/3] [wifi] Fix use-after-free: read target BSSID before releasing scan results best is a pointer into scan_result_. Move roaming_target_bssid_ assignment before release_scan_results_() to avoid dangling pointer. --- esphome/components/wifi/wifi_component.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/esphome/components/wifi/wifi_component.cpp b/esphome/components/wifi/wifi_component.cpp index 714bf635ca..3448ce156e 100644 --- a/esphome/components/wifi/wifi_component.cpp +++ b/esphome/components/wifi/wifi_component.cpp @@ -2465,11 +2465,12 @@ void WiFiComponent::process_roaming_scan_() { WiFiAP roam_params = *selected; apply_scan_result_to_params(roam_params, *best); - this->release_scan_results_(); // Mark as roaming attempt - affects retry behavior if connection fails this->roaming_state_ = RoamingState::CONNECTING; - this->roaming_target_bssid_ = best->get_bssid(); + this->roaming_target_bssid_ = best->get_bssid(); // Must read before releasing scan results + + this->release_scan_results_(); // Connect directly - wifi_sta_connect_ handles disconnect internally this->start_connecting(roam_params); From 0814685af03cf8e787e8233951ca15b3fb222759 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 23 Mar 2026 13:11:42 -1000 Subject: [PATCH 3/3] [wifi] Review fixes: clear grace period on reconnect, log BSSID, fix layout - Clear roaming_scan_end_ on successful reconnect to prevent grace period from incorrectly applying to a second disconnect - Log target BSSID in "Roam successful (via retry)" message - Move bssid_t roaming_target_bssid_ to 1-byte section to avoid 2 bytes struct padding before ap_timeout_ --- esphome/components/wifi/wifi_component.cpp | 5 ++++- esphome/components/wifi/wifi_component.h | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/esphome/components/wifi/wifi_component.cpp b/esphome/components/wifi/wifi_component.cpp index 3448ce156e..d495efc82c 100644 --- a/esphome/components/wifi/wifi_component.cpp +++ b/esphome/components/wifi/wifi_component.cpp @@ -1591,7 +1591,9 @@ void WiFiComponent::check_connecting_finished(uint32_t now) { // (e.g., first connect failed but scan-based retry found and connected to the same better AP) bssid_t current_bssid = this->wifi_bssid(); if (this->roaming_target_bssid_ != bssid_t{} && current_bssid == this->roaming_target_bssid_) { - ESP_LOGD(TAG, "Roam successful (via retry)"); + char bssid_buf[MAC_ADDRESS_PRETTY_BUFFER_SIZE]; + format_mac_addr_upper(current_bssid.data(), bssid_buf); + ESP_LOGD(TAG, "Roam successful (via retry) to %s", bssid_buf); this->roaming_attempts_ = 0; } else { // Failed roam, reconnected to different AP - keep attempts to prevent ping-pong @@ -1603,6 +1605,7 @@ void WiFiComponent::check_connecting_finished(uint32_t now) { } this->roaming_state_ = RoamingState::IDLE; this->roaming_target_bssid_ = {}; + this->roaming_scan_end_ = 0; // Clear all priority penalties - the next reconnect will happen when an AP disconnects, // which means the landscape has likely changed and previous tracked failures are stale diff --git a/esphome/components/wifi/wifi_component.h b/esphome/components/wifi/wifi_component.h index 8c7e1f9be6..99b23436f7 100644 --- a/esphome/components/wifi/wifi_component.h +++ b/esphome/components/wifi/wifi_component.h @@ -790,8 +790,7 @@ class WiFiComponent final : public Component { uint32_t last_connected_{0}; uint32_t reboot_timeout_{}; uint32_t roaming_last_check_{0}; - uint32_t roaming_scan_end_{0}; // Timestamp when last roaming scan completed - bssid_t roaming_target_bssid_{}; // BSSID of the AP we're trying to roam to + uint32_t roaming_scan_end_{0}; // Timestamp when last roaming scan completed #ifdef USE_WIFI_AP uint32_t ap_timeout_{}; #endif @@ -816,6 +815,7 @@ class WiFiComponent final : public Component { bool error_from_callback_{false}; RetryHiddenMode retry_hidden_mode_{RetryHiddenMode::BLIND_RETRY}; RoamingState roaming_state_{RoamingState::IDLE}; + bssid_t roaming_target_bssid_{}; // BSSID of the AP we're trying to roam to #if defined(USE_ESP32) && defined(USE_WIFI_RUNTIME_POWER_SAVE) WiFiPowerSaveMode configured_power_save_{WIFI_POWER_SAVE_NONE}; #endif