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

@@ -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) {

src/network/HttpDownloader.h

@@ -12,6 +12,9 @@
 class HttpDownloader {
  public:
   using ProgressCallback = std::function<void(size_t downloaded, size_t total)>;
+  // 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<bool(const uint8_t* data, size_t len)>;
 
   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.
    */

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 <Logging.h>
 #include <ReleaseJsonParser.h>
 #include <esp_crt_bundle.h>
 #include <esp_http_client.h>
 #include <esp_https_ota.h>
 #include <esp_wifi.h>
+// clang-format on
+
+#include <string>
 
 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<ReleaseJsonParser*>(event->user_data);
-  parser->feed(static_cast<const char*>(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);
+  // Stream the ~32KB release JSON straight into the parser as it arrives.
-  if (!client_handle) {
+  // Buffering the whole body in a std::string would add a growing allocation
-    LOG_ERR("OTA", "HTTP Client Handle Failed");
+  // on top of the TLS session's heap during the fetch; with -fno-exceptions an
-    return INTERNAL_UPDATE_ERROR;
+  // OOM there aborts. fetchUrl handles the verified-https GET, redirects, and
-  }
+  // User-Agent (see HttpDownloader).
-
+  ReleaseJsonParser releaseParser;
-  esp_err = esp_http_client_set_header(client_handle, "User-Agent", "CrossPoint-ESP32-" CROSSPOINT_VERSION);
+  const bool ok = HttpDownloader::fetchUrl(latestReleaseUrl, [&releaseParser](const uint8_t* data, size_t len) {
-  if (esp_err != ESP_OK) {
+    releaseParser.feed(reinterpret_cast<const char*>(data), len);
-    LOG_ERR("OTA", "esp_http_client_set_header Failed : %s", esp_err_to_name(esp_err));
+    return true;
-    esp_http_client_cleanup(client_handle);
+  });
-    return INTERNAL_UPDATE_ERROR;
+  if (!ok) {
-  }
+    LOG_ERR("OTA", "Release check fetch failed");
-
-  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);
     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");