Merge remote-tracking branch 'upstream/ota-disable-loop-when-idle' into integration

esphome/components/esphome/ota/ota_esphome.cpp

@@ -69,17 +69,11 @@ void ESPHomeOTAComponent::setup() {
     return;
   }
 
-  // Register for socket wake notifications. loop() disables itself on its first
-  // idle tick — no need to disable_loop() here explicitly.
+  // loop() self-disables on its first idle tick; no explicit disable_loop() needed here.
   App.set_ota_wake_component(this);
 #ifdef USE_LWIP_FAST_SELECT
-  // Install the listener filter so the fast-select RCVPLUS wake hook only fires for
-  // events on this listener's netconn (i.e. new incoming connections). Without this,
-  // every RCVPLUS across all monitored sockets (API client data, mDNS, etc.) would
-  // pay the inline hook's two volatile stores + memw barriers to mark OTA
-  // pending-enable, even though OTA would just re-disable itself on the next tick.
-  // Uses the existing public esphome_lwip_get_sock() lookup instead of adding a
-  // dedicated accessor on Socket — the lookup happens once at setup, not per event.
+  // Filter fast-select wakes to this listener only. If the sock lookup returns nullptr,
+  // no wakes fire and loop() falls back to the self-disable safety net.
   esphome_fast_select_set_ota_listener_sock(esphome_lwip_get_sock(this->server_->get_fd()));
 #endif
 }
@@ -98,23 +92,10 @@ void ESPHomeOTAComponent::dump_config() {
 }
 
 void ESPHomeOTAComponent::loop() {
-  // Self-disabling idle loop. On the first tick after setup() (and after every session
-  // cleanup and every false wake), if there's no client and the listener has nothing
-  // queued, we disable ourselves and go back to sleep. Socket-wake paths (LwIP fast
-  // select, raw TCP accept, host select) mark us pending-enable via
-  // App.wake_ota_component_any_context() when a monitored socket signals activity, and
-  // enable_pending_loops_() reactivates us.
-  //
-  // False wakes from unrelated monitored sockets are expected — the event callbacks
-  // fire on every RCVPLUS across all monitored sockets, not just OTA's listener — and
-  // they land here with no pending work.
-  //
-  // cleanup_connection_() deliberately does NOT call disable_loop() — letting loop()
-  // run one more iteration after a session ends guarantees we re-read server_->ready()
-  // and either accept a client that queued during the session or disable cleanly here.
-  //
-  // Note: No need to check server_ for null — setup() marks the component failed if
-  // server_ creation fails.
+  // Self-disabling idle loop. Runs when a wake path marks us pending-enable (fast-select
+  // listener filter, raw-TCP accept_fn_, or host select), finds no work, and goes back
+  // to sleep. cleanup_connection_() deliberately leaves the loop enabled for one more
+  // iteration so a connection queued mid-session is still caught here.
   if (this->client_ == nullptr && !this->server_->ready()) {
     this->disable_loop();
     return;
@@ -608,12 +589,9 @@ void ESPHomeOTAComponent::cleanup_connection_() {
 #ifdef USE_OTA_PASSWORD
   this->cleanup_auth_();
 #endif
-  // Do not disable_loop() here. loop() itself disables when idle. If a second
-  // connection was queued on the listener while we were busy, the wake flag was
-  // set while this component was in LOOP state — enable_pending_loops_() only
-  // scans the inactive section and would never clear it. Letting loop() run one
-  // more iteration guarantees we re-check server_->ready() and either accept the
-  // queued client or disable ourselves cleanly.
+  // Intentionally no disable_loop() — letting loop() run one more iteration catches
+  // any connection that queued on the listener mid-session (otherwise the wake flag,
+  // set while we were in LOOP state, would be lost to enable_pending_loops_()).
 }
 
 void ESPHomeOTAComponent::yield_and_feed_watchdog_() {

esphome/components/ota/__init__.py

@@ -103,11 +103,8 @@ BASE_OTA_SCHEMA = cv.Schema(
 @coroutine_with_priority(CoroPriority.OTA_UPDATES)
 async def to_code(config):
     cg.add_define("USE_OTA")
-    # Separate compiler -D flag using an ESPHOME_-prefixed name so .c translation units
-    # (which cannot include defines.h because macros.h → Arduino.h breaks the C compile
-    # under Arduino builds) can still tell OTA is compiled in. Needed by the fast-select
-    # OTA wake hook in lwip_fast_select.c. A distinct name avoids the "USE_OTA redefined"
-    # warning that would fire if we also emitted -DUSE_OTA — defines.h already has it.
+    # Separate -D flag so .c files can see it — .c can't include defines.h (macros.h
+    # pulls in Arduino.h). Different name avoids USE_OTA redefinition warnings.
     cg.add_build_flag("-DESPHOME_USE_OTA")
     CORE.add_job(final_step)
 

esphome/components/socket/lwip_raw_tcp_impl.cpp

@@ -11,7 +11,7 @@
 #include "esphome/core/wake.h"
 #include "esphome/core/log.h"
 #ifdef USE_OTA
-#include "esphome/core/application.h"  // for App.wake_ota_component_any_context()
+#include "esphome/core/application.h"
 #endif
 
 #ifdef USE_ESP8266
@@ -858,13 +858,8 @@ err_t LWIPRawListenImpl::accept_fn_(struct tcp_pcb *newpcb, err_t err) {
   tcp_recv(newpcb, LWIPRawListenImpl::s_queued_recv_fn);
   LWIP_LOG("Accepted connection, queue size: %d", this->accepted_socket_count_);
 #ifdef USE_OTA
-  // Mark the OTA component loop to be re-enabled if it disabled itself while idle.
-  // This MUST happen before wake_loop_any_context() below — otherwise the main loop
-  // could wake, run a full iteration, and finish before we set the pending-enable
-  // flags, losing the wake event. accept_fn_ only fires for the specific listener
-  // pcb it was registered on, so there's implicit filtering here (no "false wakes"
-  // from unrelated sockets, unlike the fast-select path). Safe from RP2040's
-  // low-priority user IRQ context — only writes a volatile bool, no locks.
+  // Mark OTA pending-enable before the wake below — flags must be visible before the
+  // main task runs. accept_fn_ is per-pcb, so filtering is implicit here.
   esphome::App.wake_ota_component_any_context();
 #endif
   // Wake the main loop immediately so it can accept the new connection.

esphome/core/application.cpp

@@ -450,10 +450,7 @@ void Application::enable_pending_loops_() {
 }
 
 #if defined(USE_OTA) && defined(USE_LWIP_FAST_SELECT)
-// Extern-C trampoline for the lwip fast-select callback to reach the C++ wake method.
-// Invoked only when the listener filter in lwip_fast_select.c matches — i.e. only on
-// actual incoming connections to the OTA listen socket — so the function-call overhead
-// here is paid at most once per real OTA attempt, not on every monitored-socket event.
+// C trampoline called from lwip_fast_select.c when the listener filter matches.
 extern "C" void esphome_wake_ota_component_any_context() { App.wake_ota_component_any_context(); }
 #endif
 
@@ -563,11 +560,7 @@ void Application::yield_with_select_(uint32_t delay_ms) {
     // ret == 0: timeout occurred - normal and expected
     if (ret >= 0) [[likely]] {
 #ifdef USE_OTA
-      // Dead code today — host does not currently support the esphome OTA platform,
-      // so ota_wake_component_ is always null and this is a no-op. Kept so the wake
-      // path works out of the box if host ever gains OTA support. Host has no listener
-      // filter (that lives in lwip_fast_select.c, ESP32/LibreTiny only), so any
-      // ret > 0 fires this — harmless when the pointer is null.
+      // No-op today — host has no esphome OTA platform, so ota_wake_component_ is null.
       if (ret > 0) {
         this->wake_ota_component_any_context();
       }

esphome/core/application.h

@@ -318,17 +318,9 @@ class Application {
   static void IRAM_ATTR wake_loop_any_context() { esphome::wake_loop_any_context(); }
 
 #ifdef USE_OTA
-  /// Register the OTA component so socket-wake paths can enable its loop when a new
-  /// connection arrives on the listening socket. OTA calls this once from setup();
-  /// the component self-disables its loop on its first idle tick and is re-enabled
-  /// via wake_ota_component_any_context() when the fast-select filter in
-  /// lwip_fast_select.c matches an incoming connection on the OTA listener.
   void set_ota_wake_component(Component *component) { this->ota_wake_component_ = component; }
-  /// Mark the registered OTA component pending loop-enable. Called from the LwIP
-  /// TCP/IP task (fast-select callback), raw-TCP accept callback, and host select
-  /// return path — all task / user-IRQ context, not a real ISR. Does NOT wake the
-  /// main task itself: every caller already does so separately. Application is a
-  /// friend of Component, so we set pending_enable_loop_ directly.
+  // Marks OTA pending-enable from socket wake paths. Does NOT wake the main task —
+  // every caller already does. Callable from LwIP TCP/IP task and user-IRQ context.
   void wake_ota_component_any_context() {
     if (this->ota_wake_component_ != nullptr) {
       this->ota_wake_component_->pending_enable_loop_ = true;
@@ -413,7 +405,7 @@ class Application {
   // Pointer-sized members first
   Component *current_component_{nullptr};
 #ifdef USE_OTA
-  Component *ota_wake_component_{nullptr};  // Set by ESPHomeOTAComponent to receive socket-wake notifications
+  Component *ota_wake_component_{nullptr};
 #endif
 
   // std::vector (3 pointers each: begin, end, capacity)

esphome/core/lwip_fast_select.c

@@ -158,20 +158,10 @@ _Static_assert(offsetof(struct lwip_sock, rcvevent) == ESPHOME_LWIP_SOCK_RCVEVEN
 static netconn_callback s_original_callback = NULL;
 
 #ifdef ESPHOME_USE_OTA
-// OTA listener netconn, captured via esphome_fast_select_set_ota_listener_sock() at OTA
-// setup(). The wake hook only fires when the callback's `conn` argument matches this
-// pointer — i.e. only for RCVPLUS events on the OTA listen socket (new accepts). Avoids
-// paying the wake-hook function call on every API client data packet, mDNS query, etc.
+// ESPHOME_USE_OTA (not USE_OTA): this .c file can't include defines.h — macros.h →
+// Arduino.h would break the C compile. ota/__init__.py emits -DESPHOME_USE_OTA.
 static struct netconn *s_ota_listener_conn = NULL;
-
-// Extern-C trampoline defined in application.cpp — calls App.wake_ota_component_any_context()
-// to mark the OTA component pending loop-enable. Out-of-line (not inlined) because with the
-// listener filter above, this only fires on actual OTA connection attempts — the function-call
-// cost is paid at most once per real wake, not on every monitored-socket RCVPLUS.
-// ESPHOME_USE_OTA (not USE_OTA) because USE_OTA only lives in defines.h, and this .c file
-// cannot include defines.h — macros.h → Arduino.h would break the C compile under Arduino
-// builds. ota/__init__.py emits -DESPHOME_USE_OTA as a build flag so this file can see it.
-extern void esphome_wake_ota_component_any_context(void);
+extern void esphome_wake_ota_component_any_context(void);  // trampoline in application.cpp
 
 void esphome_fast_select_set_ota_listener_sock(struct lwip_sock *sock) {
   s_ota_listener_conn = (sock != NULL) ? sock->conn : NULL;
@@ -195,13 +185,8 @@ static void esphome_socket_event_callback(struct netconn *conn, enum netconn_evt
   // already wake the main loop through the RCVPLUS path.
   if (evt == NETCONN_EVT_RCVPLUS) {
 #ifdef ESPHOME_USE_OTA
-    // Filter: only mark OTA pending-enable when the event is for OTA's listen socket.
-    // Without this, every RCVPLUS (API client data, mDNS, etc.) would pay the inline
-    // wake hook's two volatile stores + memw barriers. The setter that installs
-    // s_ota_listener_conn is called from OTA setup(); until then the pointer is NULL
-    // and the filter skips the wake work entirely, which is the correct idle behavior.
-    // MUST happen before xTaskNotifyGive below — the flags have to be visible before
-    // the main task wakes, or the main loop could run a full iteration and miss them.
+    // Mark OTA pending-enable only for events on its listen socket. MUST happen
+    // before xTaskNotifyGive so the flags are visible when the main task wakes.
     if (conn == s_ota_listener_conn) {
       esphome_wake_ota_component_any_context();
     }

esphome/core/lwip_fast_select.h

@@ -53,12 +53,10 @@ static inline bool esphome_lwip_socket_has_data(struct lwip_sock *sock) {
 /// The sock pointer must have been obtained from esphome_lwip_get_sock().
 void esphome_lwip_hook_socket(struct lwip_sock *sock);
 
-/// Filter the inline OTA wake hook in the fast-select callback so it only fires for
-/// RCVPLUS events on this specific listener's netconn. Without this, every monitored
-/// socket's RCVPLUS (API client data, web server, mDNS, etc.) would mark OTA
-/// pending-enable and force its loop to run a wake-up tick only to re-disable itself.
-/// Captured at OTA setup(); stays pointing at the listener for the device's lifetime.
-/// Pass NULL to clear the filter (wake fires on every RCVPLUS, pre-filter behavior).
+/// Set the listener netconn that the fast-select callback filters OTA wakes against.
+/// After this is called, the OTA wake hook only fires for RCVPLUS events whose `conn`
+/// matches this listener. Passing NULL disables OTA wakes (no event matches a NULL
+/// listener) — correct behavior before install and after teardown.
 void esphome_fast_select_set_ota_listener_sock(struct lwip_sock *sock);
 
 /// Set or clear TCP_NODELAY on a socket's tcp_pcb directly.