From d8b7097acc021cf7bf763c12a9396c9b3c82918e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 21 Jan 2026 15:53:09 -1000 Subject: [PATCH] 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; }