[wifi] Fix LibreTiny thread safety with queue-based event handling - #12833
Conversation
|
To use the changes from this PR as an external component, add the following to your ESPHome configuration YAML file: external_components:
- source: github://pr#12833
components: [wifi]
refresh: 1h(Added by the PR bot) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #12833 +/- ##
=======================================
Coverage 73.44% 73.44%
=======================================
Files 53 53
Lines 11303 11303
Branches 1534 1534
=======================================
Hits 8302 8302
Misses 2602 2602
Partials 399 399 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Memory Impact AnalysisComponents:
This analysis runs automatically when components change. Memory usage is measured from a representative test configuration. |
There was a problem hiding this comment.
Pull request overview
This PR fixes critical thread safety issues in LibreTiny WiFi event handling by implementing queue-based event processing, addressing race conditions that caused connection state changes to be missed by the main loop.
Key Changes:
- Introduces FreeRTOS queue-based event handling to safely pass WiFi events from driver thread to main loop
- Replaces boolean flags with a proper state machine (
LTWiFiSTAState) for connection tracking - Implements event data structure (
LTWiFiEvent) with deep copies to avoid lifetime issues
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| esphome/components/wifi/wifi_component_libretiny.cpp | Implements queue-based event handling with state machine, refactors callback to only queue events, adds main loop event processing, and updates MAC address formatting |
| esphome/components/wifi/wifi_component.h | Adds forward declaration for LTWiFiEvent struct and declares new wifi_process_event_ method |
There was a problem hiding this comment.
Looks good to me, but I can't test it right now.
|
All stable on all my test boards overnight. Definitely a lot more reliable. Logs look much better. No more missed connect events and needless retries or 10 minutes to connect to wifi because we try again when we are already connected |
There was a problem hiding this comment.
Tested on my bk72xx seems good.
|
Thanks! |
What does this implement/fix?
Fixes thread safety issues in LibreTiny WiFi event handling that caused the "Connected" log message to never appear and could lead to connection state being missed.
Problem
LibreTiny's
WiFi.onEvent()callback runs in the WiFi driver's thread context, not the main ESPHome loop. The previous implementation directly modified shared state variables (s_sta_connecting,error_from_callback_) from the callback, causing race conditions:This manifested as the
[I][wifi:1268]: Connectedlog message never appearing, even though the connection eventually succeeded.Solution
Implement queue-based event handling matching the ESP32 IDF pattern:
LTWiFiEventunion to hold all event types with copied dataLTWiFiSTAStateenum replacing separate boolean flagswifi_loop_()drains queue,wifi_process_event_()handles each eventAdditional improvements:
Types of changes
Related issue or feature (if applicable):
Pull request in esphome-docs with documentation (if applicable):
Test Environment
Example entry for
config.yaml:Checklist:
tests/folder).If user exposed functionality or configuration variables are added/changed:
Before (missing "Connected" log):
After (correct behavior):