Skip to content

Commit

Permalink
[beken-72xx] Fix race condition when checking Wi-Fi SSID (#274)
Browse files Browse the repository at this point in the history
* 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 <kuba@szczodrzynski.pl>
Co-authored-by: Cossid <83468485+Cossid@users.noreply.github.com>
  • Loading branch information
3 people committed May 31, 2024
1 parent dfabfbb commit b255402
Showing 1 changed file with 43 additions and 6 deletions.
49 changes: 43 additions & 6 deletions cores/beken-72xx/arduino/libraries/WiFi/WiFiEvents.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}

0 comments on commit b255402

Please sign in to comment.