From 68d5968405bd37a892127125e5bb9fc3244656ca Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 4 Sep 2026 06:14:00 +0200 Subject: [PATCH] Skip the dial delay when no inbound path waits, keep dial-out alive when the listener fails, check the socket results this PR made meaningful, and cover the clean reboot branch --- .../api/api_outgoing_connection.cpp | 6 ++ esphome/components/api/api_server.cpp | 85 +++++++++++-------- esphome/components/api/api_server.h | 1 + .../components/async_tcp/async_tcp_socket.cpp | 9 +- .../components/esphome/ota/ota_esphome.cpp | 5 +- esphome/components/udp/udp_component.cpp | 5 +- .../components/wake_on_lan/wake_on_lan.cpp | 4 + ...imeout_after_authenticated_disconnect.yaml | 7 ++ tests/integration/test_api_reboot_timeout.py | 29 ++++++- 9 files changed, 113 insertions(+), 38 deletions(-) create mode 100644 tests/integration/fixtures/api_reboot_timeout_after_authenticated_disconnect.yaml diff --git a/esphome/components/api/api_outgoing_connection.cpp b/esphome/components/api/api_outgoing_connection.cpp index 7be13a2077..cd5bb7dfb7 100644 --- a/esphome/components/api/api_outgoing_connection.cpp +++ b/esphome/components/api/api_outgoing_connection.cpp @@ -43,8 +43,14 @@ void OutgoingConnectionManager::loop(APIServer *server) { const uint32_t now = App.get_loop_component_start_time(); switch (this->state_) { case DialState::DIAL_STATE_IDLE: +#if defined(API_OUTGOING_CONNECTION_HOST) || defined(USE_DEEP_SLEEP) + // A fixed host has no inbound path worth waiting for, and a deep + // sleep wake window is too short to spend on the delay + this->schedule_wait_(now, BACKOFF_MIN_MS); +#else // Target went away; give it the configured delay to reconnect first this->schedule_wait_(now, API_OUTGOING_CONNECTION_DELAY); +#endif break; case DialState::DIAL_STATE_WAITING: if (now - this->state_ts_ >= this->wait_) { diff --git a/esphome/components/api/api_server.cpp b/esphome/components/api/api_server.cpp index 24df48ceba..228b9b8fbf 100644 --- a/esphome/components/api/api_server.cpp +++ b/esphome/components/api/api_server.cpp @@ -34,7 +34,52 @@ APIServer::APIServer() { global_api_server = this; } void APIServer::socket_failed_(const LogString *msg) { ESP_LOGW(TAG, "Socket %s: errno %d", LOG_STR_ARG(msg), errno); this->destroy_socket_(); +#ifdef USE_API_OUTGOING_CONNECTION + // Dial-out needs no listener; degrade instead of stopping the component + this->status_set_error(LOG_STR("listen socket failed")); +#else this->mark_failed(); +#endif +} + +bool APIServer::create_listen_socket_() { + this->socket_ = socket::socket_ip_loop_monitored(SOCK_STREAM, 0).release(); // monitored for incoming connections + if (this->socket_ == nullptr) { + this->socket_failed_(LOG_STR("creation")); + return false; + } + int enable = 1; + int err = this->socket_->setsockopt(SOL_SOCKET, SO_REUSEADDR, &enable, sizeof(int)); + if (err != 0) { + ESP_LOGW(TAG, "Socket reuseaddr: errno %d", errno); + // we can still continue + } + err = this->socket_->setblocking(false); + if (err != 0) { + this->socket_failed_(LOG_STR("nonblocking")); + return false; + } + + struct sockaddr_storage server; + + socklen_t sl = socket::set_sockaddr_any((struct sockaddr *) &server, sizeof(server), this->port_); + if (sl == 0) { + this->socket_failed_(LOG_STR("set sockaddr")); + return false; + } + + err = this->socket_->bind((struct sockaddr *) &server, sl); + if (err != 0) { + this->socket_failed_(LOG_STR("bind")); + return false; + } + + err = this->socket_->listen(this->listen_backlog_); + if (err != 0) { + this->socket_failed_(LOG_STR("listen")); + return false; + } + return true; } void APIServer::setup() { @@ -53,42 +98,14 @@ void APIServer::setup() { #endif #endif - this->socket_ = socket::socket_ip_loop_monitored(SOCK_STREAM, 0).release(); // monitored for incoming connections - if (this->socket_ == nullptr) { - this->socket_failed_(LOG_STR("creation")); - return; - } - int enable = 1; - int err = this->socket_->setsockopt(SOL_SOCKET, SO_REUSEADDR, &enable, sizeof(int)); - if (err != 0) { - ESP_LOGW(TAG, "Socket reuseaddr: errno %d", errno); - // we can still continue - } - err = this->socket_->setblocking(false); - if (err != 0) { - this->socket_failed_(LOG_STR("nonblocking")); - return; - } - - struct sockaddr_storage server; - - socklen_t sl = socket::set_sockaddr_any((struct sockaddr *) &server, sizeof(server), this->port_); - if (sl == 0) { - this->socket_failed_(LOG_STR("set sockaddr")); - return; - } - - err = this->socket_->bind((struct sockaddr *) &server, sl); - if (err != 0) { - this->socket_failed_(LOG_STR("bind")); - return; - } - - err = this->socket_->listen(this->listen_backlog_); - if (err != 0) { - this->socket_failed_(LOG_STR("listen")); +#ifdef USE_API_OUTGOING_CONNECTION + // A dead listener degrades to an error status; dial-out still runs + this->create_listen_socket_(); +#else + if (!this->create_listen_socket_()) { return; } +#endif #ifdef USE_LOGGER if (logger::global_logger != nullptr) { diff --git a/esphome/components/api/api_server.h b/esphome/components/api/api_server.h index 0ccfab1cf9..d186143c99 100644 --- a/esphome/components/api/api_server.h +++ b/esphome/components/api/api_server.h @@ -312,6 +312,7 @@ class APIServer final : public Component, this->socket_ = nullptr; } void socket_failed_(const LogString *msg); + bool create_listen_socket_(); // Pointers and pointer-like types first (4 bytes each) socket::ListenSocket *socket_{nullptr}; #ifdef USE_API_CLIENT_CONNECTED_TRIGGER diff --git a/esphome/components/async_tcp/async_tcp_socket.cpp b/esphome/components/async_tcp/async_tcp_socket.cpp index 59100166d7..2c2771b566 100644 --- a/esphome/components/async_tcp/async_tcp_socket.cpp +++ b/esphome/components/async_tcp/async_tcp_socket.cpp @@ -42,7 +42,14 @@ bool AsyncClient::connect(const char *host, uint16_t port) { return false; } - socket_->setblocking(false); + if (socket_->setblocking(false) != 0) { + // A blocking connect()/read() would stall the whole loop + ESP_LOGE(TAG, "Failed to set nonblocking: errno %d", errno); + socket_.reset(); + if (error_cb_) + error_cb_(error_arg_, this, errno); + return false; + } int err = socket_->connect((struct sockaddr *) &addr, addrlen); if (err == 0) { diff --git a/esphome/components/esphome/ota/ota_esphome.cpp b/esphome/components/esphome/ota/ota_esphome.cpp index 9f15eaaede..f61af5a406 100644 --- a/esphome/components/esphome/ota/ota_esphome.cpp +++ b/esphome/components/esphome/ota/ota_esphome.cpp @@ -358,7 +358,10 @@ void ESPHomeOTAComponent::handle_data_() { tv.tv_usec = 0; this->client_->setsockopt(SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)); this->client_->setsockopt(SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof(tv)); - this->client_->setblocking(true); + if (this->client_->setblocking(true) != 0) { + this->log_socket_error_(LOG_STR("blocking")); + goto error; // NOLINT(cppcoreguidelines-avoid-goto) + } // Acknowledge auth OK - 1 byte this->write_byte_(ota::OTA_RESPONSE_AUTH_OK); diff --git a/esphome/components/udp/udp_component.cpp b/esphome/components/udp/udp_component.cpp index c144212ecf..e805fc6cf9 100644 --- a/esphome/components/udp/udp_component.cpp +++ b/esphome/components/udp/udp_component.cpp @@ -13,7 +13,10 @@ void UDPComponent::setup() { #if defined(USE_SOCKET_IMPL_BSD_SOCKETS) || defined(USE_SOCKET_IMPL_LWIP_SOCKETS) for (const auto &address : this->addresses_) { struct sockaddr saddr {}; - socket::set_sockaddr(&saddr, sizeof(saddr), address, this->broadcast_port_); + if (socket::set_sockaddr(&saddr, sizeof(saddr), address, this->broadcast_port_) == 0) { + ESP_LOGW(TAG, "Invalid address %s", address); + continue; + } this->sockaddrs_.push_back(saddr); } // set up broadcast socket diff --git a/esphome/components/wake_on_lan/wake_on_lan.cpp b/esphome/components/wake_on_lan/wake_on_lan.cpp index a514a55d80..7851c02726 100644 --- a/esphome/components/wake_on_lan/wake_on_lan.cpp +++ b/esphome/components/wake_on_lan/wake_on_lan.cpp @@ -34,6 +34,10 @@ void WakeOnLanButton::press_action() { struct sockaddr_storage saddr {}; auto addr_len = socket::set_sockaddr(reinterpret_cast(&saddr), sizeof(saddr), "255.255.255.255", this->port_); + if (addr_len == 0) { + ESP_LOGW(TAG, "Invalid broadcast address"); + return; + } uint8_t buffer[6 + sizeof this->macaddr_ * 16]; memcpy(buffer, PREFIX, sizeof(PREFIX)); for (size_t i = 0; i != 16; i++) { diff --git a/tests/integration/fixtures/api_reboot_timeout_after_authenticated_disconnect.yaml b/tests/integration/fixtures/api_reboot_timeout_after_authenticated_disconnect.yaml new file mode 100644 index 0000000000..881bb5b2fc --- /dev/null +++ b/tests/integration/fixtures/api_reboot_timeout_after_authenticated_disconnect.yaml @@ -0,0 +1,7 @@ +esphome: + name: api-reboot-test +host: +api: + reboot_timeout: 0.5s # Very short timeout for fast testing +logger: + level: DEBUG diff --git a/tests/integration/test_api_reboot_timeout.py b/tests/integration/test_api_reboot_timeout.py index 1a0881ece5..958a98ad68 100644 --- a/tests/integration/test_api_reboot_timeout.py +++ b/tests/integration/test_api_reboot_timeout.py @@ -5,7 +5,7 @@ import re import pytest -from .types import RunCompiledFunction +from .types import APIClientConnectedFactory, RunCompiledFunction @pytest.mark.asyncio @@ -35,3 +35,30 @@ async def test_api_reboot_timeout( pytest.fail("Device did not reboot within expected timeout") # Test passes if we get here - reboot was detected + + +@pytest.mark.asyncio +async def test_api_reboot_timeout_after_authenticated_disconnect( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """An authenticated disconnect resets the flag; the clean branch reboots.""" + loop = asyncio.get_running_loop() + reboot_future = loop.create_future() + reboot_pattern = re.compile(r"No clients; rebooting") + + def check_output(line: str) -> None: + """Check output for reboot message.""" + if not reboot_future.done() and reboot_pattern.search(line): + reboot_future.set_result(True) + + async with run_compiled(yaml_config, line_callback=check_output): + # An authenticated session refreshes the watchdog and clears the + # unauthenticated flag the harness probe set + async with api_client_connected() as client: + await client.device_info() + try: + await asyncio.wait_for(reboot_future, timeout=2.0) + except TimeoutError: + pytest.fail("Device did not reboot within expected timeout")