From b255402659004472e6ac7f3698ab645a63f15b5c Mon Sep 17 00:00:00 2001 From: devgs Date: Fri, 31 May 2024 15:54:41 +0300 Subject: [PATCH] [beken-72xx] Fix race condition when checking Wi-Fi SSID (#274) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix for a race condition in WiFi connection loop There seems to be the race between the event RW_EVT_STA_CONNECTED and an actual valid SSID value returned by BDK. If even a small delay is injected immediately after the event reception the valid value becomes available. Without this fix, due to a polling nature of ESPHome WiFiComponent::check_connecting_finished function may observe the WiFiSTAConnectStatus::CONNECTED status but with an empty SSID value, leading to `Incomplete connection.` warning and immediate attempt to start another connection, while the current one was actually established. * Fixed clang format conformance * Apply suggestions from code review * Update cores/beken-72xx/arduino/libraries/WiFi/WiFiEvents.cpp Co-authored-by: Cossid <83468485+Cossid@users.noreply.github.com> --------- Co-authored-by: Kuba SzczodrzyƄski Co-authored-by: Cossid <83468485+Cossid@users.noreply.github.com> --- .../arduino/libraries/WiFi/WiFiEvents.cpp | 49 ++++++++++++++++--- 1 file changed, 43 insertions(+), 6 deletions(-) diff --git a/cores/beken-72xx/arduino/libraries/WiFi/WiFiEvents.cpp b/cores/beken-72xx/arduino/libraries/WiFi/WiFiEvents.cpp index 6aa3cef34..f95ad19cd 100644 --- a/cores/beken-72xx/arduino/libraries/WiFi/WiFiEvents.cpp +++ b/cores/beken-72xx/arduino/libraries/WiFi/WiFiEvents.cpp @@ -27,6 +27,40 @@ static void wifiEventTask(void *arg) { } } +// There is a race condition, when we have an event about a successful +// connection but no SSID yet returned by BDK. Even a single millisecond +// delay should prevent this from happening. It's better to waste a bit +// of time here than to lose a valid connection down the line. +static String waitForValidSSID(WiFiClass *pWiFi) { + String result; + + // Read the initial value that might just be available already. + result = pWiFi->SSID(); + + if (!result.length()) { + std::size_t i = 0; + for (; i < 10; i++) { + // Delay and query again. + delay(1); + result = pWiFi->SSID(); + + if (result.length()) { + LT_DM(WIFI, "Got valid SSID after %u delays", i + 1); + break; + } + + // It's a good idea to yield. + yield(); + } + + if (!result.length()) { + LT_WM(WIFI, "Could not obtain a valid SSID after %u delays", i); + } + } + + return result; +} + void wifiEventSendArduino(EventId event) { event = (EventId)(RW_EVT_ARDUINO | event); wifiStatusCallback((rw_evt_type *)&event); @@ -52,11 +86,6 @@ void wifiEventHandler(rw_evt_type event) { LT_DM(WIFI, "BK event %u", event); - if (event <= RW_EVT_STA_GOT_IP) - pDATA->lastStaEvent = event; - else - pDATA->lastApEvent = event; - EventId eventId; EventInfo eventInfo; String ssid; @@ -103,7 +132,7 @@ void wifiEventHandler(rw_evt_type event) { case RW_EVT_STA_CONNECTED: eventId = ARDUINO_EVENT_WIFI_STA_CONNECTED; - ssid = pWiFi->SSID(); + ssid = waitForValidSSID(pWiFi); eventInfo.wifi_sta_connected.ssid_len = ssid.length(); eventInfo.wifi_sta_connected.channel = pWiFi->channel(); eventInfo.wifi_sta_connected.authmode = pWiFi->getEncryption(); @@ -145,5 +174,13 @@ void wifiEventHandler(rw_evt_type event) { break; } + // Publish state update only after the event data is retrieved. + // This relates to the race condition with RW_EVT_STA_CONNECTED. + if (event <= RW_EVT_STA_GOT_IP) { + pDATA->lastStaEvent = event; + } else { + pDATA->lastApEvent = event; + } + pWiFi->postEvent(eventId, eventInfo); }