From cbcd2b2a707f5f545d1400a3e1e7e858dd6c3425 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 21 Jan 2026 12:14:27 -1000 Subject: [PATCH 01/16] [http_request] Fix OTA failures on ESP8266/Arduino by making read semantics consistent --- .../components/http_request/http_request.h | 76 +++++++++++++++++++ .../http_request/http_request_idf.cpp | 34 +++++++-- .../http_request/ota/ota_http_request.cpp | 60 ++++++++------- .../update/http_request_update.cpp | 19 ++--- 4 files changed, 145 insertions(+), 44 deletions(-) diff --git a/esphome/components/http_request/http_request.h b/esphome/components/http_request/http_request.h index a8c2cdfc638..ca7dcaa6b81 100644 --- a/esphome/components/http_request/http_request.h +++ b/esphome/components/http_request/http_request.h @@ -79,6 +79,49 @@ inline bool is_redirect(int const status) { */ inline bool is_success(int const status) { return status >= HTTP_STATUS_OK && status < HTTP_STATUS_MULTIPLE_CHOICES; } +/// Status of a read operation +enum class HttpReadStatus : uint8_t { + OK, ///< Read completed successfully + ERROR, ///< Read error occurred + TIMEOUT, ///< Timeout waiting for data +}; + +/// Result of an HTTP read operation +struct HttpReadResult { + HttpReadStatus status; ///< Status of the read operation + int error_code; ///< Error code from read() on failure, 0 on success +}; + +/// Result of processing a non-blocking read with timeout (for manual loops) +enum class HttpReadLoopResult : uint8_t { + DATA, ///< Data was read, process it + RETRY, ///< No data yet, already delayed, caller should continue loop + ERROR, ///< Read error, caller should exit loop + TIMEOUT, ///< Timeout waiting for data, caller should exit loop +}; + +/// Process a read result with timeout tracking and delay handling +/// @param bytes_read_or_error Return value from read() - positive for bytes read, negative for error +/// @param last_data_time Time of last successful read, updated when data received +/// @param timeout_ms Maximum time to wait for data +/// @return DATA if data received, RETRY if should continue loop, ERROR/TIMEOUT if should exit +inline HttpReadLoopResult http_read_loop_result(int bytes_read_or_error, uint32_t &last_data_time, + uint32_t timeout_ms) { + if (bytes_read_or_error > 0) { + last_data_time = millis(); + return HttpReadLoopResult::DATA; + } + if (bytes_read_or_error < 0) { + return HttpReadLoopResult::ERROR; + } + // bytes_read_or_error == 0: no data available yet + if (millis() - last_data_time >= timeout_ms) { + return HttpReadLoopResult::TIMEOUT; + } + delay(1); // Small delay to prevent tight spinning + return HttpReadLoopResult::RETRY; +} + class HttpRequestComponent; class HttpContainer : public Parented { @@ -110,6 +153,38 @@ class HttpContainer : public Parented { std::map> response_headers_{}; }; +/// Read data from HTTP container into buffer with timeout handling +/// Handles feed_wdt, yield, and timeout checking internally +/// @param container The HTTP container to read from +/// @param buffer Buffer to read into +/// @param total_size Total bytes to read +/// @param chunk_size Maximum bytes per read call +/// @param timeout_ms Read timeout in milliseconds +/// @return HttpReadResult with status and error_code on failure +inline HttpReadResult http_read_fully(HttpContainer *container, uint8_t *buffer, size_t total_size, size_t chunk_size, + uint32_t timeout_ms) { + size_t read_index = 0; + uint32_t last_data_time = millis(); + + while (read_index < total_size) { + int read_bytes_or_error = container->read(buffer + read_index, std::min(chunk_size, total_size - read_index)); + + App.feed_wdt(); + yield(); + + auto result = http_read_loop_result(read_bytes_or_error, last_data_time, timeout_ms); + if (result == HttpReadLoopResult::RETRY) + continue; + if (result == HttpReadLoopResult::ERROR) + return {HttpReadStatus::ERROR, read_bytes_or_error}; + if (result == HttpReadLoopResult::TIMEOUT) + return {HttpReadStatus::TIMEOUT, 0}; + + read_index += read_bytes_or_error; + } + return {HttpReadStatus::OK, 0}; +} + class HttpRequestResponseTrigger : public Trigger, std::string &> { public: void process(const std::shared_ptr &container, std::string &response_body) { @@ -124,6 +199,7 @@ class HttpRequestComponent : public Component { void set_useragent(const char *useragent) { this->useragent_ = useragent; } void set_timeout(uint32_t timeout) { this->timeout_ = timeout; } + uint32_t get_timeout() const { return this->timeout_; } void set_watchdog_timeout(uint32_t watchdog_timeout) { this->watchdog_timeout_ = watchdog_timeout; } uint32_t get_watchdog_timeout() const { return this->watchdog_timeout_; } void set_follow_redirects(bool follow_redirects) { this->follow_redirects_ = follow_redirects; } diff --git a/esphome/components/http_request/http_request_idf.cpp b/esphome/components/http_request/http_request_idf.cpp index eedd321d801..b19d236c6b6 100644 --- a/esphome/components/http_request/http_request_idf.cpp +++ b/esphome/components/http_request/http_request_idf.cpp @@ -100,6 +100,7 @@ std::shared_ptr HttpRequestIDF::perform(const std::string &url, c config.buffer_size = this->buffer_size_rx_; config.buffer_size_tx = this->buffer_size_tx_; + config.is_async = true; // Enable non-blocking mode const uint32_t start = millis(); watchdog::WatchdogManager wdm(this->get_watchdog_timeout()); @@ -213,15 +214,36 @@ int HttpContainerIDF::read(uint8_t *buf, size_t max_len) { const uint32_t start = millis(); watchdog::WatchdogManager wdm(this->parent_->get_watchdog_timeout()); - this->feed_wdt(); - int read_len = esp_http_client_read(this->client_, (char *) buf, max_len); - this->feed_wdt(); - if (read_len > 0) { - this->bytes_read_ += read_len; + // Check if we've already read all expected content + if (this->bytes_read_ >= this->content_length) { + this->duration_ms += (millis() - start); + return 0; // All content read } + + this->feed_wdt(); + int read_len_or_error = esp_http_client_read(this->client_, (char *) buf, max_len); + this->feed_wdt(); + this->duration_ms += (millis() - start); - return read_len; + if (read_len_or_error > 0) { + this->bytes_read_ += read_len_or_error; + return read_len_or_error; + } + + if (read_len_or_error == 0) { + // Connection closed gracefully + return 0; + } + + // read_len_or_error < 0: check for EAGAIN (no data available in non-blocking mode) + // ESP_ERR_HTTP_EAGAIN = 0x7007, returned as negative + if (read_len_or_error == -ESP_ERR_HTTP_EAGAIN) { + return 0; // No data available yet, consistent with Arduino behavior + } + + // Real error - return the actual error code for debugging + return read_len_or_error; } void HttpContainerIDF::end() { diff --git a/esphome/components/http_request/ota/ota_http_request.cpp b/esphome/components/http_request/ota/ota_http_request.cpp index 2a7db9137f9..fa6860237fc 100644 --- a/esphome/components/http_request/ota/ota_http_request.cpp +++ b/esphome/components/http_request/ota/ota_http_request.cpp @@ -115,39 +115,45 @@ uint8_t OtaHttpRequestComponent::do_ota_() { return error_code; } + uint32_t last_data_time = millis(); + const uint32_t read_timeout = this->parent_->get_timeout(); + while (container->get_bytes_read() < container->content_length) { - // read a maximum of chunk_size bytes into buf. (real read size returned) - int bufsize = container->read(buf, OtaHttpRequestComponent::HTTP_RECV_BUFFER); - ESP_LOGVV(TAG, "bytes_read_ = %u, body_length_ = %u, bufsize = %i", container->get_bytes_read(), - container->content_length, bufsize); + // read a maximum of chunk_size bytes into buf. (real read size returned, or negative error code) + int bufsize_or_error = container->read(buf, OtaHttpRequestComponent::HTTP_RECV_BUFFER); + ESP_LOGVV(TAG, "bytes_read_ = %u, body_length_ = %u, bufsize_or_error = %i", container->get_bytes_read(), + container->content_length, bufsize_or_error); // feed watchdog and give other tasks a chance to run App.feed_wdt(); yield(); - // Exit loop if no data available (stream closed or end of data) - if (bufsize <= 0) { - if (bufsize < 0) { - ESP_LOGE(TAG, "Stream closed with error"); - this->cleanup_(std::move(backend), container); - return OTA_CONNECTION_ERROR; + auto result = http_read_loop_result(bufsize_or_error, last_data_time, read_timeout); + if (result == HttpReadLoopResult::RETRY) + continue; + if (result != HttpReadLoopResult::DATA) { + if (result == HttpReadLoopResult::TIMEOUT) { + ESP_LOGE(TAG, "Timeout reading data"); + } else { + ESP_LOGE(TAG, "Error reading data: %d", bufsize_or_error); } - // bufsize == 0: no more data available, exit loop - break; + this->cleanup_(std::move(backend), container); + return OTA_CONNECTION_ERROR; } - if (bufsize <= OtaHttpRequestComponent::HTTP_RECV_BUFFER) { + // At this point bufsize_or_error > 0, so it's a valid size + if (bufsize_or_error <= OtaHttpRequestComponent::HTTP_RECV_BUFFER) { // add read bytes to MD5 - md5_receive.add(buf, bufsize); + md5_receive.add(buf, bufsize_or_error); // write bytes to OTA backend this->update_started_ = true; - error_code = backend->write(buf, bufsize); + error_code = backend->write(buf, bufsize_or_error); if (error_code != ota::OTA_RESPONSE_OK) { // error code explanation available at // https://github.com/esphome/esphome/blob/dev/esphome/components/ota/ota_backend.h ESP_LOGE(TAG, "Error code (%02X) writing binary data to flash at offset %d and size %d", error_code, - container->get_bytes_read() - bufsize, container->content_length); + container->get_bytes_read() - bufsize_or_error, container->content_length); this->cleanup_(std::move(backend), container); return error_code; } @@ -244,19 +250,19 @@ bool OtaHttpRequestComponent::http_get_md5_() { } this->md5_expected_.resize(MD5_SIZE); - int read_len = 0; - while (container->get_bytes_read() < MD5_SIZE) { - read_len = container->read((uint8_t *) this->md5_expected_.data(), MD5_SIZE); - if (read_len <= 0) { - break; - } - App.feed_wdt(); - yield(); - } + auto result = http_read_fully(container.get(), (uint8_t *) this->md5_expected_.data(), MD5_SIZE, MD5_SIZE, + this->parent_->get_timeout()); container->end(); - ESP_LOGV(TAG, "Read len: %u, MD5 expected: %u", read_len, MD5_SIZE); - return read_len == MD5_SIZE; + if (result.status != HttpReadStatus::OK) { + if (result.status == HttpReadStatus::TIMEOUT) { + ESP_LOGE(TAG, "Timeout reading MD5"); + } else { + ESP_LOGE(TAG, "Error reading MD5: %d", result.error_code); + } + return false; + } + return true; } bool OtaHttpRequestComponent::validate_url_(const std::string &url) { diff --git a/esphome/components/http_request/update/http_request_update.cpp b/esphome/components/http_request/update/http_request_update.cpp index 82b391e01fc..b4e9a156db9 100644 --- a/esphome/components/http_request/update/http_request_update.cpp +++ b/esphome/components/http_request/update/http_request_update.cpp @@ -70,19 +70,16 @@ void HttpRequestUpdate::update_task(void *params) { UPDATE_RETURN; } - size_t read_index = 0; - while (container->get_bytes_read() < container->content_length) { - int read_bytes = container->read(data + read_index, MAX_READ_SIZE); - - yield(); - - if (read_bytes <= 0) { - // Network error or connection closed - break to avoid infinite loop - break; + auto read_result = http_read_fully(container.get(), data, container->content_length, MAX_READ_SIZE, + this_update->request_parent_->get_timeout()); + if (read_result.status != HttpReadStatus::OK) { + if (read_result.status == HttpReadStatus::TIMEOUT) { + ESP_LOGE(TAG, "Timeout reading manifest"); + } else { + ESP_LOGE(TAG, "Error reading manifest: %d", read_result.error_code); } - - read_index += read_bytes; } + size_t read_index = container->get_bytes_read(); bool valid = false; { // Ensures the response string falls out of scope and deallocates before the task ends From 81df19dd4b8df5b058ea5127ecba794f1c6f766c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 21 Jan 2026 12:18:52 -1000 Subject: [PATCH 02/16] handle failure --- .../components/http_request/update/http_request_update.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/esphome/components/http_request/update/http_request_update.cpp b/esphome/components/http_request/update/http_request_update.cpp index b4e9a156db9..bf6cc3448b8 100644 --- a/esphome/components/http_request/update/http_request_update.cpp +++ b/esphome/components/http_request/update/http_request_update.cpp @@ -78,6 +78,11 @@ void HttpRequestUpdate::update_task(void *params) { } else { ESP_LOGE(TAG, "Error reading manifest: %d", read_result.error_code); } + // Defer to main loop to avoid race condition on component_state_ read-modify-write + this_update->defer([this_update]() { this_update->status_set_error(LOG_STR("Failed to read manifest")); }); + allocator.deallocate(data, container->content_length); + container->end(); + UPDATE_RETURN; } size_t read_index = container->get_bytes_read(); From 68b328c019b3f967f4df501a57ceb2e5a999ec46 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 21 Jan 2026 12:25:28 -1000 Subject: [PATCH 03/16] match difficult ard behavior --- .../components/http_request/http_request_idf.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/esphome/components/http_request/http_request_idf.cpp b/esphome/components/http_request/http_request_idf.cpp index b19d236c6b6..7e0422c6c72 100644 --- a/esphome/components/http_request/http_request_idf.cpp +++ b/esphome/components/http_request/http_request_idf.cpp @@ -231,18 +231,19 @@ int HttpContainerIDF::read(uint8_t *buf, size_t max_len) { return read_len_or_error; } - if (read_len_or_error == 0) { - // Connection closed gracefully - return 0; - } - // read_len_or_error < 0: check for EAGAIN (no data available in non-blocking mode) // ESP_ERR_HTTP_EAGAIN = 0x7007, returned as negative if (read_len_or_error == -ESP_ERR_HTTP_EAGAIN) { - return 0; // No data available yet, consistent with Arduino behavior + return 0; // No data available yet, caller should retry } - // Real error - return the actual error code for debugging + if (read_len_or_error == 0) { + // Connection closed, but we haven't read all content yet (early check handles success case) + // This is a premature close - return error + return -1; + } + + // Other negative value - real error, return the actual error code for debugging return read_len_or_error; } From dffc9257dde3692e361ef3b5bfde54880d336714 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 21 Jan 2026 12:25:54 -1000 Subject: [PATCH 04/16] Update esphome/components/http_request/http_request_idf.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- esphome/components/http_request/http_request_idf.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esphome/components/http_request/http_request_idf.cpp b/esphome/components/http_request/http_request_idf.cpp index 7e0422c6c72..b0a2d264d95 100644 --- a/esphome/components/http_request/http_request_idf.cpp +++ b/esphome/components/http_request/http_request_idf.cpp @@ -232,7 +232,7 @@ int HttpContainerIDF::read(uint8_t *buf, size_t max_len) { } // read_len_or_error < 0: check for EAGAIN (no data available in non-blocking mode) - // ESP_ERR_HTTP_EAGAIN = 0x7007, returned as negative + // ESP_ERR_HTTP_EAGAIN is returned as a negative error code if (read_len_or_error == -ESP_ERR_HTTP_EAGAIN) { return 0; // No data available yet, caller should retry } From 6a8bae5b1c76bbb9f3ba35a88522c3e4bd730b27 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 21 Jan 2026 12:30:42 -1000 Subject: [PATCH 05/16] unify, make consistant --- .../components/http_request/http_request.h | 24 ++++++++++++++++ .../http_request/http_request_arduino.cpp | 25 ++++++++++++++++- .../http_request/http_request_idf.cpp | 28 ++++++++++++++----- 3 files changed, 69 insertions(+), 8 deletions(-) diff --git a/esphome/components/http_request/http_request.h b/esphome/components/http_request/http_request.h index ca7dcaa6b81..4d345412b53 100644 --- a/esphome/components/http_request/http_request.h +++ b/esphome/components/http_request/http_request.h @@ -79,6 +79,9 @@ inline bool is_redirect(int const status) { */ inline bool is_success(int const status) { return status >= HTTP_STATUS_OK && status < HTTP_STATUS_MULTIPLE_CHOICES; } +/// Error code returned by HttpContainer::read() when connection closed prematurely +static constexpr int HTTP_ERROR_CONNECTION_CLOSED = -1; + /// Status of a read operation enum class HttpReadStatus : uint8_t { OK, ///< Read completed successfully @@ -131,6 +134,27 @@ class HttpContainer : public Parented { int status_code; uint32_t duration_ms; + /** + * @brief Read data from the HTTP response body. + * + * This is a non-blocking read operation. The semantics are consistent across + * all platforms (Arduino and ESP-IDF): + * + * @param buf Buffer to read data into + * @param max_len Maximum number of bytes to read + * @return + * - > 0: Number of bytes read successfully + * - 0: No data available yet, caller should retry (data may still be arriving) + * - HTTP_ERROR_CONNECTION_CLOSED (-1): Connection closed prematurely + * - < -1: Other error (platform-specific error code) + * + * The caller should use get_bytes_read() and content_length to track progress. + * When get_bytes_read() >= content_length, all expected data has been received. + * + * For non-blocking read loops, use http_read_loop_result() helper which handles + * timeout tracking and converts return values to HttpReadLoopResult enum. + * For simple buffer reads, use http_read_fully() helper. + */ virtual int read(uint8_t *buf, size_t max_len) = 0; virtual void end() = 0; diff --git a/esphome/components/http_request/http_request_arduino.cpp b/esphome/components/http_request/http_request_arduino.cpp index a653942b186..d45d623b55f 100644 --- a/esphome/components/http_request/http_request_arduino.cpp +++ b/esphome/components/http_request/http_request_arduino.cpp @@ -139,6 +139,21 @@ std::shared_ptr HttpRequestArduino::perform(const std::string &ur return container; } +// Arduino HTTP read implementation +// +// Arduino's WiFiClient is inherently non-blocking - available() returns 0 when +// no data is ready. We use connected() to distinguish "no data yet" from +// "connection closed". +// +// WiFiClient behavior: +// available() > 0: data ready to read +// available() == 0 && connected(): no data yet, still connected +// available() == 0 && !connected(): connection closed +// +// We normalize these to the HttpContainer::read() contract: +// > 0: bytes read +// 0: no data yet, retry +// < 0: error (connection closed prematurely, or stream vanished) int HttpContainerArduino::read(uint8_t *buf, size_t max_len) { const uint32_t start = millis(); watchdog::WatchdogManager wdm(this->parent_->get_watchdog_timeout()); @@ -154,7 +169,15 @@ int HttpContainerArduino::read(uint8_t *buf, size_t max_len) { if (bufsize == 0) { this->duration_ms += (millis() - start); - return 0; + // Check if we've read all expected content + if (this->bytes_read_ >= this->content_length) { + return 0; // All content read successfully + } + // No data available - check if connection is still open + if (!stream_ptr->connected()) { + return HTTP_ERROR_CONNECTION_CLOSED; // Connection closed prematurely + } + return 0; // No data yet, caller should retry } App.feed_wdt(); diff --git a/esphome/components/http_request/http_request_idf.cpp b/esphome/components/http_request/http_request_idf.cpp index b0a2d264d95..dfbb33c8a10 100644 --- a/esphome/components/http_request/http_request_idf.cpp +++ b/esphome/components/http_request/http_request_idf.cpp @@ -210,6 +210,19 @@ std::shared_ptr HttpRequestIDF::perform(const std::string &url, c return container; } +// ESP-IDF HTTP read implementation +// +// Uses non-blocking mode (config.is_async = true) for consistent behavior with Arduino. +// esp_http_client_read() in async mode returns: +// > 0: bytes read +// 0: connection closed (end of stream) +// -ESP_ERR_HTTP_EAGAIN (0x7007): no data available yet (would block) +// other negative: error +// +// We normalize these to the HttpContainer::read() contract: +// > 0: bytes read +// 0: no data yet, retry +// < 0: error (connection closed prematurely, or other error) int HttpContainerIDF::read(uint8_t *buf, size_t max_len) { const uint32_t start = millis(); watchdog::WatchdogManager wdm(this->parent_->get_watchdog_timeout()); @@ -217,7 +230,7 @@ int HttpContainerIDF::read(uint8_t *buf, size_t max_len) { // Check if we've already read all expected content if (this->bytes_read_ >= this->content_length) { this->duration_ms += (millis() - start); - return 0; // All content read + return 0; // All content read successfully } this->feed_wdt(); @@ -231,16 +244,17 @@ int HttpContainerIDF::read(uint8_t *buf, size_t max_len) { return read_len_or_error; } - // read_len_or_error < 0: check for EAGAIN (no data available in non-blocking mode) - // ESP_ERR_HTTP_EAGAIN is returned as a negative error code + // No data available yet in non-blocking mode + // ESP_ERR_HTTP_EAGAIN (0x7007) is returned as negative if (read_len_or_error == -ESP_ERR_HTTP_EAGAIN) { - return 0; // No data available yet, caller should retry + return 0; // No data yet, caller should retry } + // Connection closed by server if (read_len_or_error == 0) { - // Connection closed, but we haven't read all content yet (early check handles success case) - // This is a premature close - return error - return -1; + // We haven't read all content yet (early check handles success case) + // Return error so caller exits immediately instead of waiting for timeout + return HTTP_ERROR_CONNECTION_CLOSED; } // Other negative value - real error, return the actual error code for debugging From af76ddeda4b11cd979e54ee6ec48582f0db06606 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 21 Jan 2026 12:31:18 -1000 Subject: [PATCH 06/16] unify, make consistant --- esphome/components/http_request/http_request_arduino.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esphome/components/http_request/http_request_arduino.cpp b/esphome/components/http_request/http_request_arduino.cpp index d45d623b55f..7eada01257a 100644 --- a/esphome/components/http_request/http_request_arduino.cpp +++ b/esphome/components/http_request/http_request_arduino.cpp @@ -161,7 +161,7 @@ int HttpContainerArduino::read(uint8_t *buf, size_t max_len) { WiFiClient *stream_ptr = this->client_.getStreamPtr(); if (stream_ptr == nullptr) { ESP_LOGE(TAG, "Stream pointer vanished!"); - return -1; + return HTTP_ERROR_CONNECTION_CLOSED; } int available_data = stream_ptr->available(); From 5efe5ff9fdefacf3e01553578a27b5063d288f82 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 21 Jan 2026 12:35:00 -1000 Subject: [PATCH 07/16] fix all the use --- .../update/esp32_hosted_update.cpp | 45 ++++++++++++------- .../components/http_request/http_request.h | 14 +++--- 2 files changed, 37 insertions(+), 22 deletions(-) diff --git a/esphome/components/esp32_hosted/update/esp32_hosted_update.cpp b/esphome/components/esp32_hosted/update/esp32_hosted_update.cpp index a82ee48718b..362151b322a 100644 --- a/esphome/components/esp32_hosted/update/esp32_hosted_update.cpp +++ b/esphome/components/esp32_hosted/update/esp32_hosted_update.cpp @@ -11,6 +11,7 @@ #include #ifdef USE_ESP32_HOSTED_HTTP_UPDATE +#include "esphome/components/http_request/http_request.h" #include "esphome/components/json/json_util.h" #include "esphome/components/network/util.h" #endif @@ -187,12 +188,18 @@ bool Esp32HostedUpdate::fetch_manifest_() { std::string json_str; json_str.reserve(container->content_length); uint8_t buf[256]; + uint32_t last_data_time = millis(); + const uint32_t read_timeout = this->http_request_parent_->get_timeout(); while (container->get_bytes_read() < container->content_length) { - int read = container->read(buf, sizeof(buf)); - if (read > 0) { - json_str.append(reinterpret_cast(buf), read); - } + int read_or_error = container->read(buf, sizeof(buf)); + App.feed_wdt(); yield(); + auto result = http_request::http_read_loop_result(read_or_error, last_data_time, read_timeout); + if (result == http_request::HttpReadLoopResult::RETRY) + continue; + if (result != http_request::HttpReadLoopResult::DATA) + break; // ERROR or TIMEOUT + json_str.append(reinterpret_cast(buf), read_or_error); } container->end(); @@ -301,28 +308,32 @@ bool Esp32HostedUpdate::stream_firmware_to_coprocessor_() { hasher.init(); uint8_t buffer[CHUNK_SIZE]; + uint32_t last_data_time = millis(); + const uint32_t read_timeout = this->http_request_parent_->get_timeout(); while (container->get_bytes_read() < total_size) { - int read = container->read(buffer, sizeof(buffer)); + int read_or_error = container->read(buffer, sizeof(buffer)); // Feed watchdog and give other tasks a chance to run App.feed_wdt(); yield(); - // Exit loop if no data available (stream closed or end of data) - if (read <= 0) { - if (read < 0) { - ESP_LOGE(TAG, "Stream closed with error"); - esp_hosted_slave_ota_end(); // NOLINT - container->end(); - this->status_set_error(LOG_STR("Download failed")); - return false; + auto result = http_request::http_read_loop_result(read_or_error, last_data_time, read_timeout); + if (result == http_request::HttpReadLoopResult::RETRY) + continue; + if (result != http_request::HttpReadLoopResult::DATA) { + if (result == http_request::HttpReadLoopResult::TIMEOUT) { + ESP_LOGE(TAG, "Timeout reading firmware data"); + } else { + ESP_LOGE(TAG, "Error reading firmware data: %d", read_or_error); } - // read == 0: no more data available, exit loop - break; + esp_hosted_slave_ota_end(); // NOLINT + container->end(); + this->status_set_error(LOG_STR("Download failed")); + return false; } - hasher.add(buffer, read); - err = esp_hosted_slave_ota_write(buffer, read); // NOLINT + hasher.add(buffer, read_or_error); + err = esp_hosted_slave_ota_write(buffer, read_or_error); // NOLINT if (err != ESP_OK) { ESP_LOGE(TAG, "Failed to write OTA data: %s", esp_err_to_name(err)); esp_hosted_slave_ota_end(); // NOLINT diff --git a/esphome/components/http_request/http_request.h b/esphome/components/http_request/http_request.h index 4d345412b53..57de00345e9 100644 --- a/esphome/components/http_request/http_request.h +++ b/esphome/components/http_request/http_request.h @@ -350,14 +350,18 @@ template class HttpRequestSendAction : public Action { uint8_t *buf = allocator.allocate(max_length); if (buf != nullptr) { size_t read_index = 0; + uint32_t last_data_time = millis(); + const uint32_t read_timeout = this->parent_->get_timeout(); while (container->get_bytes_read() < max_length) { - int read = container->read(buf + read_index, std::min(max_length - read_index, 512)); - if (read <= 0) { - break; - } + int read_or_error = container->read(buf + read_index, std::min(max_length - read_index, 512)); App.feed_wdt(); yield(); - read_index += read; + auto result = http_read_loop_result(read_or_error, last_data_time, read_timeout); + if (result == HttpReadLoopResult::RETRY) + continue; + if (result != HttpReadLoopResult::DATA) + break; // ERROR or TIMEOUT + read_index += read_or_error; } response_body.reserve(read_index); response_body.assign((char *) buf, read_index); From d56554100bcc2c4f7b39c104886f95abe5c5682f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 21 Jan 2026 12:50:06 -1000 Subject: [PATCH 08/16] document document document --- .../update/esp32_hosted_update.cpp | 2 + .../components/http_request/http_request.h | 47 +++++++++++++++---- .../http_request/http_request_arduino.cpp | 8 ++-- .../http_request/http_request_idf.cpp | 10 ++-- 4 files changed, 51 insertions(+), 16 deletions(-) diff --git a/esphome/components/esp32_hosted/update/esp32_hosted_update.cpp b/esphome/components/esp32_hosted/update/esp32_hosted_update.cpp index 362151b322a..93db9b7f029 100644 --- a/esphome/components/esp32_hosted/update/esp32_hosted_update.cpp +++ b/esphome/components/esp32_hosted/update/esp32_hosted_update.cpp @@ -185,6 +185,8 @@ bool Esp32HostedUpdate::fetch_manifest_() { } // Read manifest JSON into string (manifest is small, ~1KB max) + // NOTE: HttpContainer::read() has non-BSD socket semantics - see http_request.h + // Use http_read_loop_result() helper instead of checking return values directly std::string json_str; json_str.reserve(container->content_length); uint8_t buf[256]; diff --git a/esphome/components/http_request/http_request.h b/esphome/components/http_request/http_request.h index 57de00345e9..30e205bf106 100644 --- a/esphome/components/http_request/http_request.h +++ b/esphome/components/http_request/http_request.h @@ -79,7 +79,35 @@ inline bool is_redirect(int const status) { */ inline bool is_success(int const status) { return status >= HTTP_STATUS_OK && status < HTTP_STATUS_MULTIPLE_CHOICES; } +/* + * HTTP Container Read Semantics + * ============================= + * + * IMPORTANT: These semantics differ from standard BSD sockets! + * + * BSD socket read() returns: + * > 0: bytes read + * == 0: connection closed (EOF) + * < 0: error (check errno) + * + * HttpContainer::read() returns: + * > 0: bytes read successfully + * == 0: no data available yet (non-blocking, caller should RETRY) + * < 0: error or connection closed (caller should EXIT) + * HTTP_ERROR_CONNECTION_CLOSED (-1) = connection closed prematurely + * other negative values = platform-specific errors + * + * This non-blocking design allows consistent behavior across: + * - ESP-IDF (async mode with EAGAIN handling) + * - Arduino (available() + connected() checks) + * + * Use the helper functions below instead of checking return values directly: + * - http_read_loop_result(): for manual loops with per-chunk processing + * - http_read_fully(): for simple "read N bytes into buffer" operations + */ + /// Error code returned by HttpContainer::read() when connection closed prematurely +/// NOTE: Unlike BSD sockets where 0 means EOF, here 0 means "no data yet, retry" static constexpr int HTTP_ERROR_CONNECTION_CLOSED = -1; /// Status of a read operation @@ -135,25 +163,26 @@ class HttpContainer : public Parented { uint32_t duration_ms; /** - * @brief Read data from the HTTP response body. + * @brief Read data from the HTTP response body (non-blocking). * - * This is a non-blocking read operation. The semantics are consistent across - * all platforms (Arduino and ESP-IDF): + * WARNING: These semantics differ from BSD sockets! + * BSD sockets: 0 = EOF (connection closed) + * This method: 0 = no data yet (retry), negative = error/closed * * @param buf Buffer to read data into * @param max_len Maximum number of bytes to read * @return * - > 0: Number of bytes read successfully - * - 0: No data available yet, caller should retry (data may still be arriving) + * - 0: No data available yet (NOT EOF!), caller should retry * - HTTP_ERROR_CONNECTION_CLOSED (-1): Connection closed prematurely * - < -1: Other error (platform-specific error code) * - * The caller should use get_bytes_read() and content_length to track progress. - * When get_bytes_read() >= content_length, all expected data has been received. + * Use get_bytes_read() and content_length to track progress. + * When get_bytes_read() >= content_length, all data has been received. * - * For non-blocking read loops, use http_read_loop_result() helper which handles - * timeout tracking and converts return values to HttpReadLoopResult enum. - * For simple buffer reads, use http_read_fully() helper. + * IMPORTANT: Do not use raw return values directly. Use these helpers: + * - http_read_loop_result(): for loops with per-chunk processing + * - http_read_fully(): for simple "read N bytes" operations */ virtual int read(uint8_t *buf, size_t max_len) = 0; virtual void end() = 0; diff --git a/esphome/components/http_request/http_request_arduino.cpp b/esphome/components/http_request/http_request_arduino.cpp index 7eada01257a..8ec4d2bc4b5 100644 --- a/esphome/components/http_request/http_request_arduino.cpp +++ b/esphome/components/http_request/http_request_arduino.cpp @@ -141,6 +141,8 @@ std::shared_ptr HttpRequestArduino::perform(const std::string &ur // Arduino HTTP read implementation // +// WARNING: Return values differ from BSD sockets! See http_request.h for full documentation. +// // Arduino's WiFiClient is inherently non-blocking - available() returns 0 when // no data is ready. We use connected() to distinguish "no data yet" from // "connection closed". @@ -150,10 +152,10 @@ std::shared_ptr HttpRequestArduino::perform(const std::string &ur // available() == 0 && connected(): no data yet, still connected // available() == 0 && !connected(): connection closed // -// We normalize these to the HttpContainer::read() contract: +// We normalize to HttpContainer::read() contract (NOT BSD socket semantics!): // > 0: bytes read -// 0: no data yet, retry -// < 0: error (connection closed prematurely, or stream vanished) +// 0: no data yet, retry <-- NOTE: 0 means retry, NOT EOF! +// < 0: error/connection closed <-- connection closed returns -1, not 0 int HttpContainerArduino::read(uint8_t *buf, size_t max_len) { const uint32_t start = millis(); watchdog::WatchdogManager wdm(this->parent_->get_watchdog_timeout()); diff --git a/esphome/components/http_request/http_request_idf.cpp b/esphome/components/http_request/http_request_idf.cpp index dfbb33c8a10..e01b2ee35c8 100644 --- a/esphome/components/http_request/http_request_idf.cpp +++ b/esphome/components/http_request/http_request_idf.cpp @@ -212,17 +212,19 @@ std::shared_ptr HttpRequestIDF::perform(const std::string &url, c // ESP-IDF HTTP read implementation // +// WARNING: Return values differ from BSD sockets! See http_request.h for full documentation. +// // Uses non-blocking mode (config.is_async = true) for consistent behavior with Arduino. // esp_http_client_read() in async mode returns: // > 0: bytes read -// 0: connection closed (end of stream) +// 0: connection closed (end of stream) <-- BSD socket EOF semantics // -ESP_ERR_HTTP_EAGAIN (0x7007): no data available yet (would block) // other negative: error // -// We normalize these to the HttpContainer::read() contract: +// We normalize to HttpContainer::read() contract (NOT BSD socket semantics!): // > 0: bytes read -// 0: no data yet, retry -// < 0: error (connection closed prematurely, or other error) +// 0: no data yet, retry <-- NOTE: 0 means retry, NOT EOF! +// < 0: error/connection closed <-- connection closed returns -1, not 0 int HttpContainerIDF::read(uint8_t *buf, size_t max_len) { const uint32_t start = millis(); watchdog::WatchdogManager wdm(this->parent_->get_watchdog_timeout()); From 371a1f71a82b20a83d9c366c7af68fdc40e240b3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 21 Jan 2026 12:50:20 -1000 Subject: [PATCH 09/16] document document document --- esphome/components/esp32_hosted/update/esp32_hosted_update.cpp | 2 ++ esphome/components/http_request/ota/ota_http_request.cpp | 2 ++ 2 files changed, 4 insertions(+) diff --git a/esphome/components/esp32_hosted/update/esp32_hosted_update.cpp b/esphome/components/esp32_hosted/update/esp32_hosted_update.cpp index 93db9b7f029..ebcdd5f36ee 100644 --- a/esphome/components/esp32_hosted/update/esp32_hosted_update.cpp +++ b/esphome/components/esp32_hosted/update/esp32_hosted_update.cpp @@ -306,6 +306,8 @@ bool Esp32HostedUpdate::stream_firmware_to_coprocessor_() { } // Stream firmware to coprocessor while computing SHA256 + // NOTE: HttpContainer::read() has non-BSD socket semantics - see http_request.h + // Use http_read_loop_result() helper instead of checking return values directly sha256::SHA256 hasher; hasher.init(); diff --git a/esphome/components/http_request/ota/ota_http_request.cpp b/esphome/components/http_request/ota/ota_http_request.cpp index fa6860237fc..6c77e75d8c8 100644 --- a/esphome/components/http_request/ota/ota_http_request.cpp +++ b/esphome/components/http_request/ota/ota_http_request.cpp @@ -115,6 +115,8 @@ uint8_t OtaHttpRequestComponent::do_ota_() { return error_code; } + // NOTE: HttpContainer::read() has non-BSD socket semantics - see http_request.h + // Use http_read_loop_result() helper instead of checking return values directly uint32_t last_data_time = millis(); const uint32_t read_timeout = this->parent_->get_timeout(); From 9b155a3126ac2b4f3e57fb27d853581a15896f8f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 21 Jan 2026 12:50:39 -1000 Subject: [PATCH 10/16] document document document --- esphome/components/http_request/http_request.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/esphome/components/http_request/http_request.h b/esphome/components/http_request/http_request.h index 30e205bf106..cd683e8d759 100644 --- a/esphome/components/http_request/http_request.h +++ b/esphome/components/http_request/http_request.h @@ -378,6 +378,8 @@ template class HttpRequestSendAction : public Action { RAMAllocator allocator; uint8_t *buf = allocator.allocate(max_length); if (buf != nullptr) { + // NOTE: HttpContainer::read() has non-BSD socket semantics - see top of this file + // Use http_read_loop_result() helper instead of checking return values directly size_t read_index = 0; uint32_t last_data_time = millis(); const uint32_t read_timeout = this->parent_->get_timeout(); From 0d0899b10e38d3b2de1fb19c6b50ee3bd928880e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 21 Jan 2026 12:52:15 -1000 Subject: [PATCH 11/16] unify, make consistant --- esphome/components/http_request/http_request_idf.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esphome/components/http_request/http_request_idf.cpp b/esphome/components/http_request/http_request_idf.cpp index e01b2ee35c8..5bb08bf6265 100644 --- a/esphome/components/http_request/http_request_idf.cpp +++ b/esphome/components/http_request/http_request_idf.cpp @@ -247,7 +247,7 @@ int HttpContainerIDF::read(uint8_t *buf, size_t max_len) { } // No data available yet in non-blocking mode - // ESP_ERR_HTTP_EAGAIN (0x7007) is returned as negative + // ESP_ERR_HTTP_EAGAIN is returned as a negative error code if (read_len_or_error == -ESP_ERR_HTTP_EAGAIN) { return 0; // No data yet, caller should retry } From dd4bfc7b0b472f8b03dc1203b1e2e6f024296d0f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 21 Jan 2026 12:52:39 -1000 Subject: [PATCH 12/16] unify, make consistant --- esphome/components/http_request/http_request_idf.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esphome/components/http_request/http_request_idf.cpp b/esphome/components/http_request/http_request_idf.cpp index 5bb08bf6265..583c9b5e19c 100644 --- a/esphome/components/http_request/http_request_idf.cpp +++ b/esphome/components/http_request/http_request_idf.cpp @@ -218,7 +218,7 @@ std::shared_ptr HttpRequestIDF::perform(const std::string &url, c // esp_http_client_read() in async mode returns: // > 0: bytes read // 0: connection closed (end of stream) <-- BSD socket EOF semantics -// -ESP_ERR_HTTP_EAGAIN (0x7007): no data available yet (would block) +// -ESP_ERR_HTTP_EAGAIN: no data available yet (would block) // other negative: error // // We normalize to HttpContainer::read() contract (NOT BSD socket semantics!): From 802549362f6693b2f98d09214c0fc04672688d3e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 21 Jan 2026 12:57:59 -1000 Subject: [PATCH 13/16] help clang-tidy --- .../components/http_request/update/http_request_update.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/esphome/components/http_request/update/http_request_update.cpp b/esphome/components/http_request/update/http_request_update.cpp index bf6cc3448b8..c63e55d159c 100644 --- a/esphome/components/http_request/update/http_request_update.cpp +++ b/esphome/components/http_request/update/http_request_update.cpp @@ -11,7 +11,12 @@ namespace http_request { // The update function runs in a task only on ESP32s. #ifdef USE_ESP32 -#define UPDATE_RETURN vTaskDelete(nullptr) // Delete the current update task +// vTaskDelete doesn't return, but clang-tidy doesn't know that +#define UPDATE_RETURN \ + do { \ + vTaskDelete(nullptr); \ + __builtin_unreachable(); \ + } while (0) #else #define UPDATE_RETURN return #endif From d708dc648b17a9ac064a4ff5cfbed8c4537c22b3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 21 Jan 2026 13:56:37 -1000 Subject: [PATCH 14/16] fix cleanup crash (existing bug) [13:35:00.591][I][http_request.ota:175]: Done in 542 seconds [13:35:00.696][V][esp-idf:000]: E (669073) boot_comm: mismatch chip ID, expected 9, found 3424 [13:35:00.698][V][esp-idf:000]: E (669076) esp_ota_ops: New image failed verification [13:35:00.699][W][http_request.ota:198]: Error ending update! error_code: 132 [13:35:00.701][V][http_request.ota:073]: Aborting OTA backend [13:35:00.702][V][http_request.ota:076]: Aborting HTTP connection [13:35:00.703]Guru Meditation Error: Core 1 panic'ed (InstrFetchProhibited). Exception was unhandled. [13:35:00.703]Core 1 register dump: [13:35:00.703]PC : 0x11101080 PS : 0x00060630 A0 : 0x8203e461 A1 : 0x3fceded0 [13:35:00.703]A2 : 0x3fc9e45c A3 : 0x3fcee840 A4 : 0x0000003f A5 : 0x3fcee840 [13:35:00.703]A6 : 0x0000003e A7 : 0x3fcafccc A8 : 0x8203e43a A9 : 0x3fcede40 [13:35:00.703]A10 : 0x3fc9e45c A11 : 0x00000001 A12 : 0x0000003f A13 : 0x3fc9ee08 [13:35:00.704]A14 : 0x0000006d A15 : 0x3fcee674 SAR : 0x00000008 EXCCAUSE: 0x00000014 [13:35:00.704]EXCVADDR: 0x11101080 LBEG : 0x40056f08 LEND : 0x40056f12 LCOUNT : 0x00000000 [13:35:00.706]Backtrace: 0x1110107d:0x3fceded0 0x4203e45e:0x3fcedef0 0x4203e46e:0x3fcedf10 0x4201f561:0x3fcedf30 0x420078fc:0x3fcedf50 0x4200817f:0x3fcedf80 0x42008c89:0x3fcedfa0 0x42008da9:0x3fcee1d0 0x42014e45:0x3fcee1f0 0x4209e323:0x3fcee230 0x4209e337:0x3fcee250 0x4209e505:0x3fcee270 0x4201402e:0x3fcee290 0x42006555:0x3fcee2b0 0x4200376c:0x3fcee2d0 0x4209d809:0x3fcee2f0 0x42005b6d:0x3fcee310 0x42005cb9:0x3fcee350 0x420042b4:0x3fcee370 0x4200620f:0x3fcee3a0 0x4209e0ed:0x3fcee3f0 0x42012889:0x3fcee410 0x4201243e:0x3fcee430 0x420140d2:0x3fcee490 0x420070fe:0x3fcee4b0 WARNING Found stack trace! Trying to decode it WARNING Decoded 0x4203e45e: esp_transport_list_clean at /Users/bdraco/.platformio/packages/framework-espidf/components/tcp_transport/transport.c:85 WARNING Decoded 0x4203e46e: esp_transport_list_destroy at /Users/bdraco/.platformio/packages/framework-espidf/components/tcp_transport/transport.c:74 WARNING Decoded 0x4201f561: esp_http_client_cleanup at /Users/bdraco/.platformio/packages/framework-espidf/components/esp_http_client/esp_http_client.c:1027 WARNING Decoded 0x420078fc: esphome::http_request::HttpContainerIDF::end() at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/components/http_request/http_request_idf.cpp:270 WARNING Decoded 0x4200817f: esphome::http_request::OtaHttpRequestComponent::cleanup_(std::unique_ptr >, std::shared_ptr const&) at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/components/http_request/ota/ota_http_request.cpp:77 (discriminator 1) WARNING Decoded 0x42008c89: esphome::http_request::OtaHttpRequestComponent::do_ota_() at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/components/http_request/ota/ota_http_request.cpp:199 (discriminator 1) WARNING Decoded 0x42008da9: esphome::http_request::OtaHttpRequestComponent::flash() at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/components/http_request/ota/ota_http_request.cpp:49 WARNING Decoded 0x42014e45: esphome::http_request::OtaHttpRequestComponentFlashAction<>::play() at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/components/http_request/ota/automation.h:33 WARNING Decoded 0x4209e323: esphome::Action<>::play_complex() at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/core/automation.h:268 WARNING Decoded 0x4209e337: esphome::Action<>::play_next_() at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/core/automation.h:299 (inlined by) esphome::Action<>::play_complex() at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/core/automation.h:269 WARNING Decoded 0x4209e505: esphome::ActionList<>::play() at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/core/automation.h:347 (inlined by) esphome::Automation<>::trigger() at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/core/automation.h:389 (inlined by) esphome::Trigger<>::trigger() at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/core/automation.h:241 WARNING Decoded 0x4201402e: esphome::button::ButtonPressTrigger::ButtonPressTrigger(esphome::button::Button*)::{lambda()#1}::operator()() const at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/components/button/automation.h:22 (inlined by) void std::__invoke_impl(std::__invoke_other, esphome::button::ButtonPressTrigger::ButtonPressTrigger(esphome::button::Button*)::{lambda()#1}&) at /Users/bdraco/.platformio/packages/toolchain-xtensa-esp-elf/xtensa-esp-elf/include/c++/14.2.0/bits/invoke.h:61 (inlined by) std::enable_if, void>::type std::__invoke_r(esphome::button::ButtonPressTrigger::ButtonPressTrigger(esphome::button::Button*)::{lambda()#1}&) at /Users/bdraco/.platformio/packages/toolchain-xtensa-esp-elf/xtensa-esp-elf/include/c++/14.2.0/bits/invoke.h:111 (inlined by) std::_Function_handler::_M_invoke(std::_Any_data const&) at /Users/bdraco/.platformio/packages/toolchain-xtensa-esp-elf/xtensa-esp-elf/include/c++/14.2.0/bits/std_function.h:290 WARNING Decoded 0x42006555: std::function::operator()() const at /Users/bdraco/.platformio/packages/toolchain-xtensa-esp-elf/xtensa-esp-elf/include/c++/14.2.0/bits/std_function.h:591 (inlined by) esphome::CallbackManager::call() at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/core/helpers.h:1335 (inlined by) esphome::LazyCallbackManager::call() at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/core/helpers.h:1387 (inlined by) esphome::button::Button::press() at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/components/button/button.cpp:24 WARNING Decoded 0x4200376c: esphome::api::APIConnection::button_command(esphome::api::ButtonCommandRequest const&) at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/components/api/api_connection.cpp:941 WARNING Decoded 0x4209d809: esphome::api::APIServerConnection::on_button_command_request(esphome::api::ButtonCommandRequest const&) at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/components/api/api_pb2_service.cpp:692 WARNING Decoded 0x42005b6d: esphome::api::APIServerConnectionBase::read_message(unsigned long, unsigned long, unsigned char const*) at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/components/api/api_pb2_service.cpp:526 WARNING Decoded 0x42005cb9: esphome::api::APIServerConnection::read_message(unsigned long, unsigned long, unsigned char const*) at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/components/api/api_pb2_service.cpp:864 WARNING Decoded 0x420042b4: esphome::api::APIConnection::loop() at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/components/api/api_connection.cpp:210 WARNING Decoded 0x4200620f: esphome::api::APIServer::loop() at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/components/api/api_server.cpp:183 (discriminator 1) WARNING Decoded 0x4209e0ed: esphome::Component::call_loop() at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/core/component.cpp:211 WARNING Decoded 0x42012889: esphome::Component::call() at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/core/component.cpp:266 WARNING Decoded 0x4201243e: esphome::Application::loop() at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/core/application.cpp:164 WARNING Decoded 0x420140d2: loop() at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/test_http_ota_esp32s3.yaml:162 WARNING Decoded 0x420070fe: esphome::loop_task(void*) at /Users/bdraco/esphome/.esphome/build/test-http-ota-s3/src/esphome/components/esp32/core.cpp:62 (discriminator 1) [13:35:00.706]ELF file SHA256: 84cefc24a [13:35:00.706]Rebooting... [13:35:02.193]ESP-ROM:esp32s3-20210327 [13:35:02.193]Build:Mar 27 2021 [13:35:02.193]rst:0xc (RTC_SW_CPU_RST),boot:0x8 (SPI_FAST_FLASH_BOOT) [13:35:02.193]Saved PC:0x40378c02 WARNING Decoded 0x40378c02: esp_cpu_wait_for_intr at /Users/bdraco/.platformio/packages/framework-espidf/components/esp_hw_support/cpu.c:64 [13:35:02.193]SPIWP:0xee [13:35:02.193]mode:DIO, clock div:1 [13:35:02.193]load:0x3fce2820,len:0x15c8 [13:35:02.193]load:0x403c8700,len:0xce4 [13:35:02.193]load:0x403cb700,len:0x2f98 --- esphome/components/http_request/http_request_idf.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/esphome/components/http_request/http_request_idf.cpp b/esphome/components/http_request/http_request_idf.cpp index 583c9b5e19c..c1de226fef8 100644 --- a/esphome/components/http_request/http_request_idf.cpp +++ b/esphome/components/http_request/http_request_idf.cpp @@ -264,10 +264,14 @@ int HttpContainerIDF::read(uint8_t *buf, size_t max_len) { } void HttpContainerIDF::end() { + if (this->client_ == nullptr) { + return; // Already cleaned up + } watchdog::WatchdogManager wdm(this->parent_->get_watchdog_timeout()); esp_http_client_close(this->client_); esp_http_client_cleanup(this->client_); + this->client_ = nullptr; } void HttpContainerIDF::feed_wdt() { From 133cf0be1eb2c8e8ef65bbce32a5a5fed8bd2398 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 21 Jan 2026 15:38:13 -1000 Subject: [PATCH 15/16] remove unnecessary duration_ms update on early return --- esphome/components/http_request/http_request_idf.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/esphome/components/http_request/http_request_idf.cpp b/esphome/components/http_request/http_request_idf.cpp index c1de226fef8..d87a1e6b9bc 100644 --- a/esphome/components/http_request/http_request_idf.cpp +++ b/esphome/components/http_request/http_request_idf.cpp @@ -231,7 +231,6 @@ int HttpContainerIDF::read(uint8_t *buf, size_t max_len) { // Check if we've already read all expected content if (this->bytes_read_ >= this->content_length) { - this->duration_ms += (millis() - start); return 0; // All content read successfully } From d8b7097acc021cf7bf763c12a9396c9b3c82918e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 21 Jan 2026 15:53:09 -1000 Subject: [PATCH 16/16] idf http sync does not actually work --- .../components/http_request/http_request.h | 20 ++++++++----- .../http_request/http_request_idf.cpp | 29 ++++++------------- 2 files changed, 22 insertions(+), 27 deletions(-) diff --git a/esphome/components/http_request/http_request.h b/esphome/components/http_request/http_request.h index cd683e8d759..fb39ca504cd 100644 --- a/esphome/components/http_request/http_request.h +++ b/esphome/components/http_request/http_request.h @@ -92,14 +92,15 @@ inline bool is_success(int const status) { return status >= HTTP_STATUS_OK && st * * HttpContainer::read() returns: * > 0: bytes read successfully - * == 0: no data available yet (non-blocking, caller should RETRY) + * == 0: no data available yet OR all content read + * (caller should check bytes_read vs content_length) * < 0: error or connection closed (caller should EXIT) * HTTP_ERROR_CONNECTION_CLOSED (-1) = connection closed prematurely * other negative values = platform-specific errors * - * This non-blocking design allows consistent behavior across: - * - ESP-IDF (async mode with EAGAIN handling) - * - Arduino (available() + connected() checks) + * Platform behaviors: + * - ESP-IDF: blocking reads, 0 only returned when all content read + * - Arduino: non-blocking, 0 means "no data yet" or "all content read" * * Use the helper functions below instead of checking return values directly: * - http_read_loop_result(): for manual loops with per-chunk processing @@ -163,20 +164,25 @@ class HttpContainer : public Parented { uint32_t duration_ms; /** - * @brief Read data from the HTTP response body (non-blocking). + * @brief Read data from the HTTP response body. * * WARNING: These semantics differ from BSD sockets! * BSD sockets: 0 = EOF (connection closed) - * This method: 0 = no data yet (retry), negative = error/closed + * This method: 0 = no data yet OR all content read, negative = error/closed * * @param buf Buffer to read data into * @param max_len Maximum number of bytes to read * @return * - > 0: Number of bytes read successfully - * - 0: No data available yet (NOT EOF!), caller should retry + * - 0: No data available yet OR all content read + * (check get_bytes_read() >= content_length to distinguish) * - HTTP_ERROR_CONNECTION_CLOSED (-1): Connection closed prematurely * - < -1: Other error (platform-specific error code) * + * Platform notes: + * - ESP-IDF: blocking read, 0 only when all content read + * - Arduino: non-blocking, 0 can mean "no data yet" or "all content read" + * * Use get_bytes_read() and content_length to track progress. * When get_bytes_read() >= content_length, all data has been received. * diff --git a/esphome/components/http_request/http_request_idf.cpp b/esphome/components/http_request/http_request_idf.cpp index d87a1e6b9bc..b6fb7f7ea9b 100644 --- a/esphome/components/http_request/http_request_idf.cpp +++ b/esphome/components/http_request/http_request_idf.cpp @@ -100,7 +100,6 @@ std::shared_ptr HttpRequestIDF::perform(const std::string &url, c config.buffer_size = this->buffer_size_rx_; config.buffer_size_tx = this->buffer_size_tx_; - config.is_async = true; // Enable non-blocking mode const uint32_t start = millis(); watchdog::WatchdogManager wdm(this->get_watchdog_timeout()); @@ -210,21 +209,19 @@ std::shared_ptr HttpRequestIDF::perform(const std::string &url, c return container; } -// ESP-IDF HTTP read implementation +// ESP-IDF HTTP read implementation (blocking mode) // // WARNING: Return values differ from BSD sockets! See http_request.h for full documentation. // -// Uses non-blocking mode (config.is_async = true) for consistent behavior with Arduino. -// esp_http_client_read() in async mode returns: +// esp_http_client_read() in blocking mode returns: // > 0: bytes read -// 0: connection closed (end of stream) <-- BSD socket EOF semantics -// -ESP_ERR_HTTP_EAGAIN: no data available yet (would block) -// other negative: error +// 0: connection closed (end of stream) +// < 0: error // -// We normalize to HttpContainer::read() contract (NOT BSD socket semantics!): +// We normalize to HttpContainer::read() contract: // > 0: bytes read -// 0: no data yet, retry <-- NOTE: 0 means retry, NOT EOF! -// < 0: error/connection closed <-- connection closed returns -1, not 0 +// 0: no data yet / all content read (caller should check bytes_read vs content_length) +// < 0: error/connection closed int HttpContainerIDF::read(uint8_t *buf, size_t max_len) { const uint32_t start = millis(); watchdog::WatchdogManager wdm(this->parent_->get_watchdog_timeout()); @@ -245,20 +242,12 @@ int HttpContainerIDF::read(uint8_t *buf, size_t max_len) { return read_len_or_error; } - // No data available yet in non-blocking mode - // ESP_ERR_HTTP_EAGAIN is returned as a negative error code - if (read_len_or_error == -ESP_ERR_HTTP_EAGAIN) { - return 0; // No data yet, caller should retry - } - - // Connection closed by server + // Connection closed by server before all content received if (read_len_or_error == 0) { - // We haven't read all content yet (early check handles success case) - // Return error so caller exits immediately instead of waiting for timeout return HTTP_ERROR_CONNECTION_CLOSED; } - // Other negative value - real error, return the actual error code for debugging + // Negative value - error, return the actual error code for debugging return read_len_or_error; }