Merge pull request #149 from jgoguen/json-stream-parse

refactor(network): Stream parse JSON response from GitHub

src/network/HttpClientStream.cpp

@@ -0,0 +1,48 @@
+#include "HttpClientStream.h"
+
+#include <climits>
+
+HttpClientStream::HttpClientStream(esp_http_client_handle_t client, int64_t contentLength)
+    : client(client), contentLength(contentLength) {}
+
+int HttpClientStream::available() {
+  if (hasError() || endOfStream) {
+    return 0;
+  }
+  if (contentLength < 0) {
+    return 1;
+  }
+  const int64_t remaining = contentLength - bytesRead;
+  if (remaining <= 0) {
+    return 0;
+  }
+  return remaining > INT_MAX ? INT_MAX : static_cast<int>(remaining);
+}
+
+int HttpClientStream::read() {
+  uint8_t byte = 0;
+  const size_t readCount = readBytes(reinterpret_cast<char*>(&byte), 1);
+  return readCount == 1 ? byte : -1;
+}
+
+int HttpClientStream::peek() { return -1; }
+
+size_t HttpClientStream::write(uint8_t) { return 0; }
+
+size_t HttpClientStream::readBytes(char* buffer, size_t length) {
+  if (buffer == nullptr || length == 0) {
+    return 0;
+  }
+  const int readLen = esp_http_client_read(client, buffer, static_cast<int>(length));
+  if (readLen == 0) {
+    endOfStream = true;
+    return 0;
+  }
+  if (readLen < 0) {
+    lastReadError = readLen;
+    endOfStream = true;
+    return 0;
+  }
+  bytesRead += readLen;
+  return static_cast<size_t>(readLen);
+}

src/network/HttpClientStream.h

@@ -0,0 +1,29 @@
+#pragma once
+
+#include <Stream.h>
+#include <esp_http_client.h>
+
+#include <cstddef>
+#include <cstdint>
+
+class HttpClientStream final : public Stream {
+ public:
+  explicit HttpClientStream(esp_http_client_handle_t client, int64_t contentLength);
+
+  int available() override;
+  int read() override;
+  int peek() override;
+  size_t write(uint8_t) override;
+  size_t readBytes(char* buffer, size_t length) override;
+
+  size_t bytesReadCount() const { return bytesRead; }
+  bool hasError() const { return lastReadError < 0; }
+  int lastError() const { return lastReadError; }
+
+ private:
+  esp_http_client_handle_t client;
+  int64_t contentLength;
+  size_t bytesRead = 0;
+  int lastReadError = 0;
+  bool endOfStream = false;
+};

src/network/OtaUpdater.cpp

@@ -3,8 +3,12 @@
 #include <ArduinoJson.h>
 #include <Logging.h>
 
+#include <cstring>
+
+#include "HttpClientStream.h"
 #include "bootloader_common.h"
 #include "esp_flash_partitions.h"
+#include "esp_heap_caps.h"
 #include "esp_http_client.h"
 #include "esp_https_ota.h"
 #include "esp_ota_ops.h"
@@ -13,10 +17,8 @@
 
 namespace {
 constexpr char latestReleaseUrl[] = "https://api.github.com/repos/jpirnay/crosspoint-reader/releases/latest";
-
+constexpr int httpRxBufferSize = 2048;
-/* This is buffer and size holder to keep upcoming data from latestReleaseUrl */
+constexpr int httpTxBufferSize = 512;
-char* local_buf;
-int output_len;
 
 /*
  * When esp_crt_bundle.h included, it is pointing wrong header file
@@ -31,95 +33,96 @@ 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);
 }
 
-esp_err_t event_handler(esp_http_client_event_t* event) {
+struct HttpClientCleaner {
-  /* We are only interested in HTTP_EVENT_ON_DATA event */
+  esp_http_client_handle_t client;
-  if (event->event_id != HTTP_EVENT_ON_DATA) return ESP_OK;
+  ~HttpClientCleaner() {
-
+    if (client) {
-  if (event->data == nullptr || event->data_len == 0) {
+      esp_http_client_cleanup(client);
-    return ESP_OK;
+    }
   }
-
+};
-  const int newSize = output_len + event->data_len + 1;
-  char* newBuf = static_cast<char*>(realloc(local_buf, static_cast<size_t>(newSize)));
-  if (newBuf == nullptr) {
-    LOG_ERR("OTA", "HTTP Client Out of Memory Failed, Allocation %d", newSize);
-    return ESP_ERR_NO_MEM;
-  }
-
-  local_buf = newBuf;
-  memcpy(local_buf + output_len, event->data, event->data_len);
-  output_len += event->data_len;
-  local_buf[output_len] = '\0';
-
-  return ESP_OK;
-} /* event_handler */
 } /* namespace */
 
 OtaUpdater::OtaUpdaterError OtaUpdater::checkForUpdate() {
-  // Reset globals so retries start clean regardless of previous outcome
-  local_buf = nullptr;
-  output_len = 0;
-
   JsonDocument filter;
   esp_err_t esp_err;
   JsonDocument doc;
 
+  updateAvailable = false;
+  latestVersion.clear();
+  otaUrl.clear();
+  otaSize = 0;
+  processedSize = 0;
+  totalSize = 0;
+  render = false;
+
   esp_http_client_config_t client_config = {
       .url = latestReleaseUrl,
       .timeout_ms = 10000,
-      .event_handler = event_handler,
       /* Default HTTP client buffer size 512 byte only */
-      .buffer_size = 8192,
+      .buffer_size = httpRxBufferSize,
-      .buffer_size_tx = 8192,
+      .buffer_size_tx = httpTxBufferSize,
       .crt_bundle_attach = esp_crt_bundle_attach,
       .keep_alive_enable = true,
   };
 
-  /* To track life time of local_buf, dtor will be called on exit from that function */
-  struct localBufCleaner {
-    char** bufPtr;
-    ~localBufCleaner() {
-      if (*bufPtr) {
-        free(*bufPtr);
-        *bufPtr = NULL;
-      }
-    }
-  } localBufCleaner = {&local_buf};
-
   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;
   }
+  HttpClientCleaner clientCleaner = {client_handle};
 
   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);
+  esp_err = esp_http_client_set_header(client_handle, "Accept", "application/vnd.github+json");
   if (esp_err != ESP_OK) {
-    LOG_ERR("OTA", "esp_http_client_perform Failed : %s", esp_err_to_name(esp_err));
+    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_open(client_handle, 0);
+  if (esp_err != ESP_OK) {
+    LOG_ERR("OTA", "esp_http_client_open Failed : %s", esp_err_to_name(esp_err));
     return HTTP_ERROR;
   }
 
-  /* esp_http_client_close will be called inside cleanup as well*/
+  const int64_t headerContentLength = esp_http_client_fetch_headers(client_handle);
-  esp_err = esp_http_client_cleanup(client_handle);
+  if (headerContentLength < 0) {
-  if (esp_err != ESP_OK) {
+    LOG_ERR("OTA", "esp_http_client_fetch_headers Failed : %lld", headerContentLength);
-    LOG_ERR("OTA", "esp_http_client_cleanup Failed : %s", esp_err_to_name(esp_err));
+    return HTTP_ERROR;
-    return INTERNAL_UPDATE_ERROR;
   }
 
+  const int statusCode = esp_http_client_get_status_code(client_handle);
+  if (statusCode != 200) {
+    LOG_ERR("OTA", "Release metadata request failed: HTTP %d", statusCode);
+    return HTTP_ERROR;
+  }
+
+  const bool chunked = esp_http_client_is_chunked_response(client_handle);
+  const int64_t contentLength = chunked ? -1 : esp_http_client_get_content_length(client_handle);
+  LOG_DBG("OTA", "Release metadata headers: content_length=%lld chunked=%s heap=%u largest=%u", contentLength,
+          chunked ? "yes" : "no", heap_caps_get_free_size(MALLOC_CAP_8BIT),
+          heap_caps_get_largest_free_block(MALLOC_CAP_8BIT));
+
   filter["tag_name"] = true;
   filter["assets"][0]["name"] = true;
   filter["assets"][0]["browser_download_url"] = true;
   filter["assets"][0]["size"] = true;
-  const DeserializationError error = deserializeJson(doc, local_buf, DeserializationOption::Filter(filter));
+
+  HttpClientStream responseStream(client_handle, contentLength);
+  const DeserializationError error = deserializeJson(doc, responseStream, DeserializationOption::Filter(filter));
   if (error) {
-    LOG_ERR("OTA", "JSON parse failed: %s", error.c_str());
+    if (responseStream.hasError()) {
+      LOG_ERR("OTA", "HTTP stream read failed after %zu bytes: %d", responseStream.bytesReadCount(),
+              responseStream.lastError());
+      return HTTP_ERROR;
+    }
+    LOG_ERR("OTA", "JSON parse failed after %zu bytes: %s", responseStream.bytesReadCount(), error.c_str());
     return JSON_PARSE_ERROR;
   }
 
@@ -135,10 +138,11 @@ OtaUpdater::OtaUpdaterError OtaUpdater::checkForUpdate() {
 
   latestVersion = doc["tag_name"].as<std::string>();
 
-  for (int i = 0; i < doc["assets"].size(); i++) {
+  for (JsonObjectConst asset : doc["assets"].as<JsonArrayConst>()) {
-    if (doc["assets"][i]["name"] == "firmware.bin") {
+    const char* name = asset["name"] | "";
-      otaUrl = doc["assets"][i]["browser_download_url"].as<std::string>();
+    if (strcmp(name, "firmware.bin") == 0) {
-      otaSize = doc["assets"][i]["size"].as<size_t>();
+      otaUrl = asset["browser_download_url"].as<std::string>();
+      otaSize = asset["size"].as<size_t>();
       totalSize = otaSize;
       updateAvailable = true;
       break;