mirror of
https://github.com/esphome/esphome.git
synced 2026-09-15 17:18:40 +00:00
[esphome.ota] Trim verbose comments
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.
This commit is contained in:
@@ -69,22 +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 wake hook cost 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. If the lookup ever returns nullptr (shouldn't after a
|
||||
// successful listen()), the filter is installed with nullptr and the callback
|
||||
// compare never matches, so no wakes fire — loop() falls back to the self-disable
|
||||
// safety net. That degraded mode is the correct behavior: OTA will still work on
|
||||
// the first loop tick (which runs during setup before disable), and any subsequent
|
||||
// wakes go through the wake_loop_any_context() path that fires unconditionally.
|
||||
// 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
|
||||
}
|
||||
@@ -103,31 +92,10 @@ void ESPHomeOTAComponent::dump_config() {
|
||||
}
|
||||
|
||||
void ESPHomeOTAComponent::loop() {
|
||||
// Self-disabling idle loop. On the first tick after setup() (and after every session
|
||||
// ends), if there's no client and the listener has nothing queued, we disable
|
||||
// ourselves and go back to sleep. Wake paths (LwIP fast select with listener filter,
|
||||
// raw TCP accept_fn_, host select) mark us pending-enable via
|
||||
// App.wake_ota_component_any_context() when an incoming connection arrives on the OTA
|
||||
// listen socket; enable_pending_loops_() reactivates us.
|
||||
//
|
||||
// On fast-select platforms (ESP32 / LibreTiny), the callback is filtered to OTA's
|
||||
// listener netconn, so unrelated monitored sockets (API client data, mDNS, etc.) do
|
||||
// NOT wake OTA through that path. Raw TCP (ESP8266 / RP2040) is inherently filtered:
|
||||
// accept_fn_ is registered per listener pcb and only fires on completed handshakes
|
||||
// to that specific listener. So in normal steady state this loop() runs exactly
|
||||
// once per real incoming OTA connection.
|
||||
//
|
||||
// We can still land here with no pending work in a few narrow cases — a second
|
||||
// connection queued on the listener while an OTA session was active, a listener
|
||||
// filter that hasn't been installed yet, or the host select path (no filter) — which
|
||||
// is why the idle-check and self-disable are retained as a safety net.
|
||||
//
|
||||
// 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;
|
||||
@@ -611,12 +579,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_() {
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
|
||||
@@ -560,17 +560,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;
|
||||
@@ -655,7 +647,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)
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
|
||||
@@ -53,16 +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);
|
||||
|
||||
/// Install an OTA listener netconn as the wake-filter target for the fast-select
|
||||
/// callback. After this is called, the OTA wake hook only fires for RCVPLUS events
|
||||
/// whose `conn` argument matches this listener's netconn — i.e. only on actual
|
||||
/// incoming connections to the OTA listen socket. 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.
|
||||
/// Passing NULL disables OTA wake notifications entirely (no RCVPLUS event will match
|
||||
/// a null listener), which is the correct behavior before the listener is installed
|
||||
/// and after it's torn down.
|
||||
/// 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.
|
||||
|
||||
Reference in New Issue
Block a user