Extract the 0.3s magic sleep into a named constant explaining why that
duration is chosen (feed_wdt re-dispatches every ~3 ms; 300 ms gives
~100 opportunities). Fix the idle-write check to snapshot AFTER the
clear instead of before it, so writes still in-flight from the error
phase don't inflate the delta.
Verifies that after clearing all status flags, re-setting a flag makes
status_led_light resume writing to its output. Guards against a future
idle optimization (like #15642) where status_led disables its own
loop() when idle: if the re-enable path were broken, the second set
would not produce writes.
Also checks that writes STOP after all flags are cleared (counter
should not keep growing), proving status_led_light correctly stops
blinking in steady state.
Drop the trailing underscore from any_component_has_status_flag now
that the method is public. Trailing underscores in the codebase are
reserved for protected/private members per clang-tidy naming rules,
which caused a CI failure on the previously-named public helper.
Add integration tests covering:
- Single-component status_set/clear for warning and error
- Multi-component OR semantics (both clear orders)
- Warning and error independence
- End-to-end proof that status_led_light::loop() reads App.app_state_
and writes its output when the bits are set (via a fake template
output whose write_action bumps a counter exposed as a sensor)
Application::loop() used to, on every iteration of the inner component
loop, OR each component's state into a local accumulator and then OR
that into this->app_state_, finally overwriting this->app_state_ with
the accumulator after the loop. That was ~5 instructions per component
per loop iteration on the hot path, paying to rebuild state that's
already kept current elsewhere.
STATUS_LED_WARNING / STATUS_LED_ERROR are the only app_state_ bits
anything reads. Component::set_status_flag_() already writes them to
App.app_state_ directly the moment they're set, so any reader (status_led
components, feed_wdt's LED re-dispatch) already sees them without the
per-iter OR. The per-iter OR only ever re-set bits that were already
there — pure dead code on the hot path.
The clear path is handled by moving the work into
status_clear_warning/error_slow_path_(): on a real set->clear transition
the helpers walk App.components_ once to check if any other component
still has the flag, and clear App.app_state_ if not. Clears are rare
relative to loop iterations (a logged transition event), so O(N) per
clear is far cheaper than O(N) per loop.
Setup subtlety: Application::setup()'s slow-setup busy-wait forces
STATUS_LED_WARNING on to blink the LED while a slow component initializes.
A component clear during setup would prematurely wipe that forced bit,
so status_clear_warning_slow_path_() gates its walk-and-clear behind
APP_STATE_SETUP_COMPLETE (a free bit on app_state_ — zero RAM cost).
Application::setup() reconciles the warning state once at the end and
sets the flag. STATUS_LED_ERROR is never forced so its clear path
always walks.
Also fixes a pre-existing bug: the old per-iter accumulation only iterated
looping_components_, so non-looping components' status bits would get
clobbered by the end-of-loop 'app_state_ = new_app_state' overwrite.
The walk-on-clear approach iterates components_ (all of them), so
non-looping components are respected.
Separate Application::feed_wdt() into two entry points so the hot path
callers stop paying for the time==0 check they never trigger:
- feed_wdt_with_time(time): inline, hot path. Rate-limit check in 3
Xtensa instructions (load + sub + branch). [[unlikely]] tells the
compiler the slow branch is rare so the common path stays
fall-through.
- feed_wdt(): cold, out of line. Fetches millis() and forwards through
the same rate limit. Used by setup loops, upload helpers, yield(),
and any other non-hot caller.
feed_wdt_slow_() is now pure worker code — 11 bytes. It just calls
arch_feed_wdt(), updates last_wdt_feed_, and runs the status LED
re-dispatch. Both entries have already confirmed the rate limit was
exceeded before calling.
Hot call sites updated:
- Application::loop() per-component feed
- Scheduler::execute_item_() after each scheduled item runs
- Application::teardown_components() inner loop (already has 'now')
Cleaner than the early-return form — the action (calling feed_wdt_slow_)
reads as the body of the conditional instead of falling through past a
guard clause. Logically identical and compiles to the same code.
The main loop used to feed the watchdog unconditionally right after
Scheduler::call() returned, regardless of whether the scheduler had any
actual work to do. On an idle device this meant every outer loop
iteration paid the inline rate-limit check (load + sub + branch) for no
benefit.
Move the feed into Scheduler::execute_item_() so it fires only after a
scheduled callback actually runs, and covers both the main heap path
and the defer queue path (both go through execute_item_). This also
bounds the max feed gap during a burst of back-to-back scheduled items
by max(item_runtime) instead of sum(item_runtime).
The top-of-loop feed in Application::before_loop_tasks_() is now
unnecessary — when Scheduler::call does no work, the only elapsed time
is the sleep wake plus a few instructions, and when it does have work,
it fed the wdt as it went.
Split Application::feed_wdt() into an ALWAYS_INLINE wrapper that checks
the 3ms rate limit against last_wdt_feed_ and a feed_wdt_slow_() callee
that performs the actual arch_feed_wdt() + status LED re-dispatch.
Callers on the hot path (loop_task before/after each component) that
already have a millis() timestamp in hand now pay only a load + sub +
branch on the no-op path instead of a full call8 / entry / retw.
Moves the rate-limit state from a function-local static to a class
member (last_wdt_feed_) so the inline can access it.
PR #15639 removed Application::register_socket / monitored_sockets_
on the fast-select path before the ota-disable-loop-when-idle merge.
The merge re-introduced the function definitions without the matching
declarations. Drop them.
Following the zephyr_mcumgr precedent, keep the single-instance pointer
as a file-scope static inside ota_esphome.cpp instead of plumbing an
ota_wake_component_ slot and setter through Application. The extern "C"
wake trampoline also lives in the same TU now, so nothing in core/
touches OTA-specific state.
Guard the C and C++ call sites on USE_OTA_PLATFORM_ESPHOME (added as a
-D flag from components/esphome/ota/__init__.py) instead of the broader
ESPHOME_USE_OTA that base ota/ used to emit — the trampoline symbol only
exists when the esphome OTA platform is actually compiled in.
Also drop the dead host yield_with_select_ wake hook (host has no OTA
platform today).
Disambiguates the verb-vs-noun parse of the original name. The new
form reads as 'hook this fd for the fast-select path', matching the
function's actual job.
- cached_sock_ comment in both impl headers: drop the 'iff' wording
since the pointer can also be null if esphome_lwip_get_sock() fails
on a fd that was requested to be monitored. Document all three null
cases explicitly, plus the close()-path nulling for UAF protection.
- application.h register_socket_fd / unregister_socket_fd comment
block: move inside the #ifdef USE_HOST so the generic
'register/unregister a socket' wording no longer implies these APIs
exist on fast-select builds. Add a forward reference to
fast_select_hook_fd for readers wondering where the ESP32/LibreTiny
equivalent went.
Restore the cached_sock_ = nullptr assignment inside close() on the
fast-select path. The lwip slot can be recycled for a new connection
as soon as the underlying close() returns, so any dereference of
cached_sock_ afterwards would touch an unrelated socket's pcb.
No current caller does this — setsockopt(TCP_NODELAY) and ready() are
the only consumers and neither is invoked post-close today — but
leaving the pointer dangling is a footgun for future changes. The
fd_ = -1 sentinel alone would catch the ready() path via closed
semantics, but setsockopt() reaches cached_sock_ directly and would
not be protected. Null the pointer so the protection is by
construction rather than by caller discipline.
Replace the separate closed_ bool with fd_ < 0 as the 'not open'
sentinel. close() now sets fd_ = -1 after the underlying close call,
so the destructor and double-close paths just check fd_ < 0. As a
side benefit, get_fd() on a closed socket now returns -1, making
use-after-close visible to callers instead of returning a stale
descriptor.
Drop loop_monitored_ on the USE_LWIP_FAST_SELECT path — the pointer
cached_sock_ already encodes monitoring state (non-null iff
monitored). On USE_HOST the bool is still needed because there is no
cached pointer to derive from.
Combined effect on the fast-select path:
Before: fd_(4) + cached_sock_(4) + closed_(1) + loop_monitored_(1)
+ pad(2) = 12 bytes per socket
After: fd_(4) + cached_sock_(4)
= 8 bytes per socket (aligned, no tail padding)
Saves 4 bytes per Socket instance on ESP32/LibreTiny. With typical
workloads running 5-10 sockets (API listen + clients + mDNS) that's
20-40 bytes of RAM.
Lift the lwip_sock resolve + event-callback hook sequence into
socket::fast_select_hook_fd() in socket.h so the USE_LWIP_FAST_SELECT
constructor blocks in lwip_sockets_impl.cpp and bsd_sockets_impl.cpp
stop drifting in lockstep. Both impls now collapse to a two-line call
site.
- Drop redundant cached_sock_ = nullptr assignment in close(). After
closed_ = true the socket is a corpse and no ready() or other member
access is valid, so the nulling is not load-bearing. The comment now
explains why on both impl variants.
- Reword the yield_with_select_ comment so the wake-source sentence
reads as a complete list rather than a trailing fragment.
The pre-sleep scan in Application::yield_with_select_() walks
monitored_sockets_ on every loop iteration, issuing a volatile
cross-thread read on each socket's lwip_sock::rcvevent to preserve
select() semantics when the FreeRTOS task notification counter had
been consumed but a socket still had unread data.
That scenario only existed because of a Socket::ready() contract
violation: callers could stop reading with rcvevent > 0, leaving
data behind with no pending notification. That contract is now
documented and enforced (#15590), and #15589 (the first failure
that reverted the earlier removal attempt #14475) has been fixed.
With the contract honoured, every rcvevent > 0 is paired with a
pending xTaskNotifyGive from the lwip event_callback wrapper (see
lwip_fast_select.c). ulTaskNotifyTake either returns immediately
(counter non-zero) or wakes the moment the notify lands — the scan
has nothing left to rescue.
Evidence: https://github.com/esphome/esphome/pull/15638 — an
instrumentation PR ran across 5 devices (ESP32 rev1/rev3.1/C3 on
Ethernet and WiFi, plus LibreTiny RTL8720CF) through Home Assistant
disconnect/reconnect cycles, multi-client API logger bursts, and
BLE GATT connect storms. Across ~275,000 scans and 4 observed
load-bearing candidates, every hit was in the 2–14µs range — the
instruction-level window between the lwip callback writing
rcvevent and calling xTaskNotifyGive a few instructions later.
Zero hits exceeded 100µs. No hit came anywhere near loop_interval
(16ms), which is the latency scale the scan was added to prevent.
In addition to being unused, the scan is actively harmful on the
hot path: N volatile 16-bit loads against cache-cold cross-thread
lwip_sock structures on every main-loop iteration, just to
reproduce a microsecond-scale ordering artifact the notification
path is already handling authoritatively.
This also removes the now-unused monitored_sockets_ vector and
Application::{register,unregister}_socket() on the fast-select
path. Socket implementations now call esphome_lwip_hook_socket()
directly to install the netconn event callback wrapper.
Per CLAUDE.md: comments should explain the non-obvious why, not narrate
the design journey. Remove the reasoning-aloud, before/after framing,
redundant restatements, and cross-file cross-references — the commit
messages and PR description already carry that context. Behavior
unchanged.
Per review feedback: if esphome_lwip_get_sock() ever returned nullptr
(shouldn't after successful listen(), but defensively), the listener
filter compare would never match and no fast-select wakes would fire.
loop()'s self-disable safety net + the first-tick-after-setup window
cover that degraded mode correctly. Spell it out in the comment so a
future reader doesn't treat the nullptr path as a silent bug.
Two doc-drift fixes flagged by copilot review:
1. esphome_fast_select_set_ota_listener_sock() header comment claimed
passing NULL 'clears the filter' so wake fires on every RCVPLUS. The
actual code stores NULL and the conn == s_ota_listener_conn check
never matches, so NULL means 'no wakes' not 'all wakes'. Rewrite to
describe the actual semantics (install a listener to enable filtered
wakes; NULL disables OTA wakes entirely).
2. ESPHomeOTAComponent::loop() docstring still claimed false wakes from
unrelated monitored sockets are expected. Post-filter that's no longer
true on fast-select (filtered to OTA listener netconn) or raw TCP
(per-pcb accept_fn_). Rewrite to describe the current behavior:
loop() runs ~once per real incoming OTA connection, with the idle
self-disable retained as a safety net for the few narrow cases where
a wake can land with no pending work (queued-during-session, filter
not yet installed, host select fallback).
J. Nick Koston