8000
Skip to content

[wifi] Fix LibreTiny thread safety with queue-based event handling - #12833

Merged
bdraco merged 4 commits into
devfrom
libretiny_thread_safe_wifi
Jan 4, 2026
Merged

[wifi] Fix LibreTiny thread safety with queue-based event handling#12833
bdraco merged 4 commits into
devfrom
libretiny_thread_safe_wifi

Conversation

@bdraco
@bdraco bdraco commented Jan 3, 2026
Copy link
Copy Markdown
Member

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:

  • The main loop may never see state changes (values cached in registers)
  • State changes may be visible in inconsistent order
  • LibreTiny targets (BK7231, RTL8720) lack atomic instructions (no LDREX/STREX)

This manifested as the [I][wifi:1268]: Connected log message never appearing, even though the connection eventually succeeded.

Solution

Implement queue-based event handling matching the ESP32 IDF pattern:

  1. Event queue: FreeRTOS queue to pass events from callback thread to main loop
  2. Event struct: LTWiFiEvent union to hold all event types with copied data
  3. State machine: LTWiFiSTAState enum replacing separate boolean flags
  4. Callback: Only queues events - no logging, no state changes
  5. Main loop processing: wifi_loop_() drains queue, wifi_process_event_() handles each event

Additional improvements:

  • Queue overflow detection with warning logged from main loop
  • Spurious disconnect filter for benign "Association Leave" events during connection
  • Real failures like "AP Not Found" still processed even with empty SSID

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Developer breaking change (an API change that could break external components)
  • Code quality improvements to existing code or addition of tests
  • Other

Related issue or feature (if applicable):

  • Related to WiFi connection reliability on LibreTiny platforms

Pull request in esphome-docs with documentation (if applicable):

  • N/A (internal fix, no user-facing changes)

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040
  • BK72xx
  • RTL87xx
  • nRF52840

Example entry for config.yaml:

# No configuration changes needed - this is an internal fix
wifi:
  ssid: !secret wifi_ssid
  password: !secret wifi_password

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

Before (missing "Connected" log):

[I][wifi:848]: Connecting to 'MyNetwork'...
[V][wifi_lt:480]: Ignoring disconnect event with empty ssid while connecting (reason=Association Leave)
# ... timeout, never shows "Connected" even when connection succeeds

After (correct behavior):

[I][wifi:848]: Connecting to 'MyNetwork'...
[V][wifi_lt:480]: Ignoring disconnect event with empty ssid while connecting (reason=Association Leave)
[V][wifi_lt:448]: Connected ssid='MyNetwork' bssid=XX:XX:XX:XX:XX:XX channel=1, authmode=WPA2 PSK
[V][wifi_lt:530]: static_ip=192.168.1.100 gateway=192.168.1.1
[I][wifi:1268]: Connected

@github-actions
github-actions Bot commented Jan 3, 2026
Copy link
Copy Markdown
Contributor

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-commenter
codecov-commenter commented Jan 3, 2026
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.44%. Comparing base (c34665f) to head (eada23d).
⚠️ Report is 1 commits behind head on dev.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
github-actions Bot commented Jan 3, 2026
Copy link
Copy Markdown
Contributor

Memory Impact Analysis

Components: wifi
Platform: esp8266-ard

Metric Target Branch This PR Change
RAM 28,524 bytes 28,524 bytes ➡️ +0 bytes (0.00%)
Flash 321,983 bytes 321,983 bytes ➡️ +0 bytes (0.00%)

Note: This analysis measures static RAM and Flash usage only (compile-time allocation).
Dynamic memory (heap) cannot be measured automatically.
⚠️ You must test this PR on a real device to measure free heap and ensure no runtime memory issues.

This analysis runs automatically when components change. Memory usage is measured from a representative test configuration.

@bdraco
bdraco marked this pull request as ready for review January 3, 2026 20:27
Copilot AI review requested due to automatic review settings January 3, 2026 20:27
@bdraco
bdraco marked this pull request as draft January 3, 2026 20:27
Copilot AI left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread esphome/components/wifi/wifi_component_libretiny.cpp Outdated
Comment thread esphome/components/wifi/wifi_component_libretiny.cpp Outdated
Comment thread esphome/components/wifi/wifi_component_libretiny.cpp Outdated
Comment thread esphome/components/wifi/wifi_component_libretiny.cpp
Comment thread esphome/components/wifi/wifi_component_libretiny.cpp Outdated
@bdraco
bdraco marked this pull request as ready for review January 3, 2026 20:45
@kuba2k2 kuba2k2 left a comment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, but I can't test it right now.

@bdraco
bdraco commented Jan 4, 2026
Copy link
Copy Markdown
Member Author

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

@swoboda1337 swoboda1337 left a comment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on my bk72xx seems good.

@bdraco
bdraco commented Jan 4, 2026
Copy link
Copy Markdown
Member Author

Thanks!

@bdraco
bdraco merged commit 7e75826 into dev Jan 4, 2026
24 checks passed
@bdraco
bdraco deleted the libretiny_thread_safe_wifi branch January 4, 2026 21:25
@github-actions github-actions Bot locked and limited conversation to collaborators Jan 6, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

0