mirror of
https://github.com/esphome/esphome.git
synced 2026-09-20 19:48:39 +00:00
[ota] Emit USE_OTA as build flag + define so .c files can see it
Root cause of the wake failure: ota/__init__.py only called
cg.add_define("USE_OTA"), which writes to the generated defines.h. That's
invisible to .c translation units that can't include defines.h — notably
lwip_fast_select.c, which can't include defines.h because macros.h →
Arduino.h under Arduino builds would break the C compile.
So the #ifdef USE_OTA gate inside lwip_fast_select.c's RCVPLUS callback
was always false, and esphome_wake_ota_component_any_context() was never
called. The listener callback fired (total rcvplus counter incremented
from the device), but OTA's pending-enable flag was never set, so the
main task woke and ran a loop iteration without touching the (disabled)
OTA component.
Fix: emit both cg.add_define (keeps defines.h in sync for static
analyzers / IDEs) and cg.add_build_flag("-DUSE_OTA") (passes it as a
compiler -D flag, visible to every .c TU). Also reverts all the
diagnostic scaffolding (per-loop-tick logs, debug counters, runtime
function pointer hook) that I added while chasing this.
This commit is contained in:
@@ -15,9 +15,6 @@
|
||||
#include "esphome/core/helpers.h"
|
||||
#include "esphome/core/log.h"
|
||||
#include "esphome/core/util.h"
|
||||
#ifdef USE_LWIP_FAST_SELECT
|
||||
#include "esphome/core/lwip_fast_select.h"
|
||||
#endif
|
||||
|
||||
#include <cerrno>
|
||||
#include <cstdio>
|
||||
@@ -37,13 +34,6 @@ void ESPHomeOTAComponent::setup() {
|
||||
this->server_failed_(LOG_STR("creation"));
|
||||
return;
|
||||
}
|
||||
// DEBUG: immediately after socket creation, ready() on an idle monitored socket
|
||||
// MUST return false. If it returns true, loop_monitored_ is false — meaning
|
||||
// App.register_socket() failed (likely because esphome_lwip_get_sock returned null
|
||||
// because the socket fd is outside the lwip socket table range), and our wake hook
|
||||
// was never installed on this socket. In that case the whole disable_loop+wake
|
||||
// approach silently degrades — ready() stays true forever and we poll every tick.
|
||||
ESP_LOGD(TAG, "setup: server_->ready() immediately after socket creation = %d (expect 0)", this->server_->ready());
|
||||
int enable = 1;
|
||||
int err = this->server_->setsockopt(SOL_SOCKET, SO_REUSEADDR, &enable, sizeof(int));
|
||||
if (err != 0) {
|
||||
@@ -79,7 +69,6 @@ void ESPHomeOTAComponent::setup() {
|
||||
// Register for socket wake notifications. loop() disables itself on its first
|
||||
// idle tick — no need to disable_loop() here explicitly.
|
||||
App.set_ota_wake_component(this);
|
||||
ESP_LOGD(TAG, "setup complete: registered wake component, listener fd ready");
|
||||
}
|
||||
|
||||
void ESPHomeOTAComponent::dump_config() {
|
||||
@@ -96,49 +85,26 @@ void ESPHomeOTAComponent::dump_config() {
|
||||
}
|
||||
|
||||
void ESPHomeOTAComponent::loop() {
|
||||
// Self-disabling idle loop. On the first tick after setup() (and after every
|
||||
// session cleanup and after 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.
|
||||
// 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.
|
||||
// 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.
|
||||
// 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.
|
||||
// DEBUG: self-disable removed; always poll so we can see ready/wake state.
|
||||
const uint32_t wake_count = App.ota_wake_count_debug();
|
||||
#ifdef USE_LWIP_FAST_SELECT
|
||||
const uint32_t total_rcvplus = esphome_fast_select_rcvplus_total_debug;
|
||||
const uint32_t shim_count = esphome_ota_shim_call_count_debug;
|
||||
#else
|
||||
const uint32_t total_rcvplus = 0;
|
||||
const uint32_t shim_count = 0;
|
||||
#endif
|
||||
const bool ready = this->server_->ready();
|
||||
static uint32_t last_wake_count = 0;
|
||||
static uint32_t last_total_rcvplus = 0;
|
||||
static uint32_t last_shim_count = 0;
|
||||
static bool last_ready = false;
|
||||
if (wake_count != last_wake_count || total_rcvplus != last_total_rcvplus || shim_count != last_shim_count ||
|
||||
ready != last_ready || this->client_ != nullptr) {
|
||||
ESP_LOGD(TAG, "loop tick: client=%p ready=%d wakes=%u shim=%u total_rcvplus=%u", (void *) this->client_.get(),
|
||||
ready, wake_count, shim_count, total_rcvplus);
|
||||
last_wake_count = wake_count;
|
||||
last_total_rcvplus = total_rcvplus;
|
||||
last_shim_count = shim_count;
|
||||
last_ready = ready;
|
||||
}
|
||||
if (this->client_ == nullptr && !ready) {
|
||||
return; // stay in LOOP state so we can poll every tick for diagnosis
|
||||
// Note: No need to check server_ for null — setup() marks the component failed if
|
||||
// server_ creation fails.
|
||||
if (this->client_ == nullptr && !this->server_->ready()) {
|
||||
this->disable_loop();
|
||||
return;
|
||||
}
|
||||
this->handle_handshake_();
|
||||
}
|
||||
@@ -159,13 +125,9 @@ void ESPHomeOTAComponent::handle_handshake_() {
|
||||
socklen_t addr_len = sizeof(source_addr);
|
||||
int enable = 1;
|
||||
|
||||
ESP_LOGD(TAG, "handle_handshake_: attempting accept");
|
||||
this->client_ = this->server_->accept_loop_monitored((struct sockaddr *) &source_addr, &addr_len);
|
||||
if (this->client_ == nullptr) {
|
||||
ESP_LOGD(TAG, "handle_handshake_: accept returned null (would-block)");
|
||||
if (this->client_ == nullptr)
|
||||
return;
|
||||
}
|
||||
ESP_LOGD(TAG, "handle_handshake_: accept ok, client=%p", (void *) this->client_.get());
|
||||
int err = this->client_->setsockopt(IPPROTO_TCP, TCP_NODELAY, &enable, sizeof(int));
|
||||
if (err != 0) {
|
||||
this->log_socket_error_(LOG_STR("nodelay"));
|
||||
|
||||
@@ -102,7 +102,13 @@ BASE_OTA_SCHEMA = cv.Schema(
|
||||
|
||||
@coroutine_with_priority(CoroPriority.OTA_UPDATES)
|
||||
async def to_code(config):
|
||||
# Both: add_define keeps defines.h in sync for static analyzers / IDEs that read it,
|
||||
# while add_build_flag passes -DUSE_OTA as a compiler flag so USE_OTA is visible in .c
|
||||
# translation units that cannot include defines.h (lwip_fast_select.c in particular —
|
||||
# including defines.h would drag in macros.h → Arduino.h under Arduino builds and
|
||||
# break the C compile). Needed for the fast-select OTA wake hook.
|
||||
cg.add_define("USE_OTA")
|
||||
cg.add_build_flag("-DUSE_OTA")
|
||||
CORE.add_job(final_step)
|
||||
|
||||
if CORE.is_rp2040 and CORE.using_arduino:
|
||||
|
||||
@@ -450,19 +450,13 @@ void Application::enable_pending_loops_() {
|
||||
}
|
||||
|
||||
#if defined(USE_OTA) && defined(USE_LWIP_FAST_SELECT)
|
||||
// DEBUG: directly-incremented C counter so we can tell whether the shim is being called at all,
|
||||
// independent of whether the App.wake_ota_component_any_context() call inside it is working.
|
||||
extern "C" {
|
||||
volatile uint32_t esphome_ota_shim_call_count_debug = 0;
|
||||
}
|
||||
// Called from the LwIP TCP/IP task via esphome_socket_event_callback() on NETCONN_EVT_RCVPLUS,
|
||||
// BEFORE the callback calls xTaskNotifyGive() — the flag-set must happen before the wake,
|
||||
// otherwise the main task could wake, run a full iteration, and miss the pending-enable.
|
||||
// BEFORE that callback calls xTaskNotifyGive() — pending-enable flags must be visible before
|
||||
// the main task wakes, or the main loop can run a full iteration without seeing the request.
|
||||
// Only marks the OTA component as pending loop-enable; does not itself wake the main task.
|
||||
extern "C" void esphome_wake_ota_component_any_context() {
|
||||
esphome_ota_shim_call_count_debug++;
|
||||
esphome::App.wake_ota_component_any_context();
|
||||
}
|
||||
// OTA's __init__.py emits USE_OTA as both a cg.add_define (for static analyzers) AND a
|
||||
// cg.add_build_flag (so the .c fast-select file also sees it).
|
||||
extern "C" void esphome_wake_ota_component_any_context() { App.wake_ota_component_any_context(); }
|
||||
#endif
|
||||
|
||||
#ifdef USE_LWIP_FAST_SELECT
|
||||
|
||||
@@ -572,15 +572,11 @@ class Application {
|
||||
/// Not IRAM_ATTR: all callers run in task / user-IRQ context, not a real ISR,
|
||||
/// and the LwIP event callbacks that invoke this are not IRAM-resident either.
|
||||
void wake_ota_component_any_context() {
|
||||
this->ota_wake_count_debug_++;
|
||||
if (this->ota_wake_component_ != nullptr) {
|
||||
this->ota_wake_component_->pending_enable_loop_ = true;
|
||||
this->has_pending_enable_loop_requests_ = true;
|
||||
}
|
||||
}
|
||||
/// DEBUG: monotonically increasing count of wake calls, regardless of whether
|
||||
/// ota_wake_component_ was set. Read from OTA::loop() to verify the hook fires.
|
||||
uint32_t ota_wake_count_debug() const { return this->ota_wake_count_debug_; }
|
||||
#endif
|
||||
|
||||
protected:
|
||||
@@ -659,8 +655,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
|
||||
volatile uint32_t ota_wake_count_debug_{0}; // DEBUG: incremented by wake_ota_component_any_context
|
||||
Component *ota_wake_component_{nullptr}; // Set by ESPHomeOTAComponent to receive socket-wake notifications
|
||||
#endif
|
||||
|
||||
// std::vector (3 pointers each: begin, end, capacity)
|
||||
|
||||
@@ -157,17 +157,15 @@ _Static_assert(offsetof(struct lwip_sock, rcvevent) == ESPHOME_LWIP_SOCK_RCVEVEN
|
||||
// Saved original event_callback pointer — written once in first hook_socket(), read from TCP/IP task.
|
||||
static netconn_callback s_original_callback = NULL;
|
||||
|
||||
// DEBUG: counts every RCVPLUS the wrapper callback observes, for any monitored socket.
|
||||
// Read by OTA's debug logging to distinguish "callback not firing at all" from
|
||||
// "callback fires for other sockets but not OTA's listener".
|
||||
volatile uint32_t esphome_fast_select_rcvplus_total_debug = 0;
|
||||
|
||||
#ifdef USE_OTA
|
||||
// Extern wake hook for the OTA component (implemented in application.cpp). Called from the
|
||||
// TCP/IP task on every NETCONN_EVT_RCVPLUS — not just OTA's listener, so this can be a false
|
||||
// wake from an unrelated monitored socket. OTA::loop() handles that by disabling itself again
|
||||
// when there is no pending work. The hook only marks the OTA component as pending loop-enable;
|
||||
// it does not itself wake the main task (the caller below already does that).
|
||||
// NOTE: USE_OTA reaches this file only because ota/__init__.py adds it as a build flag
|
||||
// (not a cg.add_define). defines.h cannot be included from this .c file (it pulls in
|
||||
// macros.h → Arduino.h under Arduino builds).
|
||||
extern void esphome_wake_ota_component_any_context(void);
|
||||
#endif
|
||||
|
||||
@@ -185,14 +183,11 @@ static void esphome_socket_event_callback(struct netconn *conn, enum netconn_evt
|
||||
// (rcvevent++ with a NULL pbuf or error in recvmbox), so error conditions
|
||||
// already wake the main loop through the RCVPLUS path.
|
||||
if (evt == NETCONN_EVT_RCVPLUS) {
|
||||
esphome_fast_select_rcvplus_total_debug++; // DEBUG
|
||||
#ifdef USE_OTA
|
||||
// Mark the OTA component loop to be re-enabled if it disabled itself while idle.
|
||||
// This MUST happen before xTaskNotifyGive below — otherwise the main task could
|
||||
// wake, run a full iteration, and finish before we set the pending-enable flags,
|
||||
// causing the wake event to be lost until the next unrelated socket activity.
|
||||
// Invoke the OTA wake hook BEFORE xTaskNotifyGive — if OTA is compiled in, this marks
|
||||
// its component pending-enable, and those flags must be visible before we wake the
|
||||
// main task. Otherwise the main loop could run a full iteration without seeing the
|
||||
// pending-enable request. When OTA is not compiled in, this function's body is empty.
|
||||
esphome_wake_ota_component_any_context();
|
||||
#endif
|
||||
TaskHandle_t task = esphome_main_task_handle;
|
||||
if (task != NULL) {
|
||||
xTaskNotifyGive(task);
|
||||
|
||||
@@ -53,13 +53,6 @@ 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);
|
||||
|
||||
// DEBUG counter: total RCVPLUS events the wrapper callback observed across all monitored sockets.
|
||||
extern volatile uint32_t esphome_fast_select_rcvplus_total_debug;
|
||||
|
||||
// DEBUG counter: number of times esphome_wake_ota_component_any_context() C shim was entered.
|
||||
// Independent of whether App.wake_ota_component_any_context() inside it actually ran.
|
||||
extern volatile uint32_t esphome_ota_shim_call_count_debug;
|
||||
|
||||
/// Set or clear TCP_NODELAY on a socket's tcp_pcb directly.
|
||||
/// Must be called with the TCPIP core lock held (LwIPLock in C++).
|
||||
/// This bypasses lwip_setsockopt() overhead (socket lookups, switch cascade,
|
||||
|
||||
Reference in New Issue
Block a user