From 19954aa1b57845a51deec5e2a41ee37ed1950cc4 Mon Sep 17 00:00:00 2001 From: Jeremy Klein Date: Sun, 24 May 2026 18:44:30 -0700 Subject: [PATCH] refactor: route OTA version check through HttpDownloader (#2076) With both OtaUpdater and HttpDownloader on esp_http_client (https://github.com/crosspoint-reader/crosspoint-reader/pull/2074, https://github.com/crosspoint-reader/crosspoint-reader/pull/2075), checkForUpdate no longer needs its own client and event handler to fetch the release JSON. It streams the response straight into ReleaseJsonParser through a new HttpDownloader::fetchUrl(url, DataCallback) overload, dropping the duplicate esp_http_client setup, the HTTP_EVENT_ON_DATA handler, and the totalBytesReceived file global. DataCallback hands body chunks to a callback without buffering. The naive alternative, collecting the ~32KB JSON into a std::string, aborts under -fno-exceptions: the growing allocation collides with the TLS session's heap mid-fetch and operator new calls abort(). The OTA install path stays on esp_https_ota (flash-write streaming), which has no HttpDownloader equivalent. HttpDownloader.h must precede the lwip (esp_http_client) headers in OtaUpdater.cpp, or Arduino/SdFat macros collide with lwip. Did you use AI tools to help write this code? partial --- src/network/HttpDownloader.cpp | 8 ++++ src/network/HttpDownloader.h | 9 +++++ src/network/OtaUpdater.cpp | 74 ++++++++++------------------------ 3 files changed, 38 insertions(+), 53 deletions(-) diff --git a/src/network/HttpDownloader.cpp b/src/network/HttpDownloader.cpp index 31a791cd..958b5845 100644 --- a/src/network/HttpDownloader.cpp +++ b/src/network/HttpDownloader.cpp @@ -163,6 +163,14 @@ bool HttpDownloader::fetchUrl(const std::string& url, std::string& outContent, c return runGet(url, username, password, sink) == OK; } +bool HttpDownloader::fetchUrl(const std::string& url, const DataCallback& onData, const std::string& username, + const std::string& password) { + LOG_DBG("HTTP", "Fetching: %s", url.c_str()); + Sink sink; + sink.write = onData; + return runGet(url, username, password, sink) == OK; +} + HttpDownloader::DownloadError HttpDownloader::downloadToFile(const std::string& url, const std::string& destPath, ProgressCallback progress, bool* cancelFlag, const std::string& username, const std::string& password) { diff --git a/src/network/HttpDownloader.h b/src/network/HttpDownloader.h index 1c0a243f..c52b790d 100644 --- a/src/network/HttpDownloader.h +++ b/src/network/HttpDownloader.h @@ -12,6 +12,9 @@ class HttpDownloader { public: using ProgressCallback = std::function; + // Called with each body chunk as it arrives; return false to abort. Lets a + // streaming parser consume the response without buffering the whole body. + using DataCallback = std::function; enum DownloadError { OK = 0, @@ -29,6 +32,12 @@ class HttpDownloader { static bool fetchUrl(const std::string& url, Stream& stream, const std::string& username = "", const std::string& password = ""); + /** + * Stream the response body to onData as it arrives, without buffering it. + */ + static bool fetchUrl(const std::string& url, const DataCallback& onData, const std::string& username = "", + const std::string& password = ""); + /** * Download a file to the SD card with optional credentials. */ diff --git a/src/network/OtaUpdater.cpp b/src/network/OtaUpdater.cpp index 658b145a..63a1903f 100644 --- a/src/network/OtaUpdater.cpp +++ b/src/network/OtaUpdater.cpp @@ -1,11 +1,20 @@ #include "OtaUpdater.h" +// clang-format off +// HttpDownloader.h pulls Arduino/SdFat, whose macros collide with lwip's +// ip4_addr.h unless seen before esp_http_client (which includes lwip). Pin this +// order; clang-format would otherwise sort the local header last and break the +// build. +#include "HttpDownloader.h" #include #include #include #include #include #include +// clang-format on + +#include namespace { constexpr char latestReleaseUrl[] = "https://api.github.com/repos/crosspoint-reader/crosspoint-reader/releases/latest"; @@ -13,67 +22,26 @@ constexpr char latestReleaseUrl[] = "https://api.github.com/repos/crosspoint-rea esp_err_t http_client_set_header_cb(esp_http_client_handle_t http_client) { return esp_http_client_set_header(http_client, "User-Agent", "CrossPoint-ESP32-" CROSSPOINT_VERSION); } - -size_t totalBytesReceived = 0; - -esp_err_t event_handler(esp_http_client_event_t* event) { - if (event->event_id != HTTP_EVENT_ON_DATA) return ESP_OK; - totalBytesReceived += event->data_len; - LOG_DBG("OTA", "HTTP chunk: %d bytes (total: %zu)", event->data_len, totalBytesReceived); - auto* parser = static_cast(event->user_data); - parser->feed(static_cast(event->data), event->data_len); - return ESP_OK; -} } // namespace OtaUpdater::OtaUpdaterError OtaUpdater::checkForUpdate() { - esp_err_t esp_err; - ReleaseJsonParser releaseParser; - - esp_http_client_config_t client_config = { - .url = latestReleaseUrl, - .event_handler = event_handler, - // 4096 holds the API response headers; the 32KB body streams through the - // parser in chunks so RX needn't be larger. TX only carries our GET. - // Both free before installUpdate, so smaller leaves it less fragmentation. - .buffer_size = 4096, - .buffer_size_tx = 1024, - .user_data = &releaseParser, - .skip_cert_common_name_check = true, - .crt_bundle_attach = esp_crt_bundle_attach, - .keep_alive_enable = true, - }; - - totalBytesReceived = 0; LOG_DBG("OTA", "Checking for update (current: %s)", CROSSPOINT_VERSION); - esp_http_client_handle_t client_handle = esp_http_client_init(&client_config); - if (!client_handle) { - LOG_ERR("OTA", "HTTP Client Handle Failed"); - return INTERNAL_UPDATE_ERROR; - } - - esp_err = esp_http_client_set_header(client_handle, "User-Agent", "CrossPoint-ESP32-" CROSSPOINT_VERSION); - if (esp_err != ESP_OK) { - LOG_ERR("OTA", "esp_http_client_set_header Failed : %s", esp_err_to_name(esp_err)); - esp_http_client_cleanup(client_handle); - return INTERNAL_UPDATE_ERROR; - } - - esp_err = esp_http_client_perform(client_handle); - if (esp_err != ESP_OK) { - LOG_ERR("OTA", "esp_http_client_perform Failed : %s", esp_err_to_name(esp_err)); - esp_http_client_cleanup(client_handle); + // Stream the ~32KB release JSON straight into the parser as it arrives. + // Buffering the whole body in a std::string would add a growing allocation + // on top of the TLS session's heap during the fetch; with -fno-exceptions an + // OOM there aborts. fetchUrl handles the verified-https GET, redirects, and + // User-Agent (see HttpDownloader). + ReleaseJsonParser releaseParser; + const bool ok = HttpDownloader::fetchUrl(latestReleaseUrl, [&releaseParser](const uint8_t* data, size_t len) { + releaseParser.feed(reinterpret_cast(data), len); + return true; + }); + if (!ok) { + LOG_ERR("OTA", "Release check fetch failed"); return HTTP_ERROR; } - esp_err = esp_http_client_cleanup(client_handle); - if (esp_err != ESP_OK) { - LOG_ERR("OTA", "esp_http_client_cleanup Failed : %s", esp_err_to_name(esp_err)); - return INTERNAL_UPDATE_ERROR; - } - - LOG_DBG("OTA", "Response received: %zu bytes total", totalBytesReceived); LOG_DBG("OTA", "Parser results: tag=%s firmware=%s", releaseParser.foundTag() ? "yes" : "no", releaseParser.foundFirmware() ? "yes" : "no");