From 7da8b335ab973ce99a8fdae1d41b3af234921688 Mon Sep 17 00:00:00 2001
From: jpirnay <jens@pirnay.com>
Date: Tue, 21 Apr 2026 22:16:23 +0200
Subject: [PATCH] Review comments

---
 .github/workflows/release.yml                 | 21 +-----
 .github/workflows/release_candidate.yml       | 13 +---
 scripts/patch_min_chip_rev.py                 | 64 +++++++++++++++++++
 .../network/WifiSelectionActivity.cpp         |  5 ++
 src/main.cpp                                  |  9 ++-
 src/network/OtaUpdater.cpp                    | 22 ++-----
 src/network/OtaUpdater.h                      |  1 -
 7 files changed, 82 insertions(+), 53 deletions(-)
 create mode 100644 scripts/patch_min_chip_rev.py

diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
index 360e9076..bcd73969 100644
--- a/.github/workflows/release.yml
+++ b/.github/workflows/release.yml
@@ -29,26 +29,7 @@ jobs:
         run: pio run -e gh_release
 
       - name: Patch min_chip_rev_full to 0
-        run: |
-          python3 - <<'EOF'
-          import struct, sys
-          # esp_image_header_t (8 bytes) + extended header:
-          #   offset 0: wp_pin (B)
-          #   offset 1: clk/q drv (B)
-          #   offset 2: d/cs drv (B)
-          #   offset 3: hd/wp drv (B)
-          #   offset 4: chip_id (H, 2 bytes)
-          #   offset 6: min_rev (B)
-          #   offset 7: min_rev_full (H, 2 bytes)  <-- patch target
-          MIN_REV_FULL_OFFSET = 8 + 7
-          path = ".pio/build/gh_release/firmware.bin"
-          with open(path, "r+b") as f:
-              f.seek(MIN_REV_FULL_OFFSET)
-              old = struct.unpack("<H", f.read(2))[0]
-              print(f"min_chip_rev_full: {old // 100}.{old % 100} -> 0.0")
-              f.seek(MIN_REV_FULL_OFFSET)
-              f.write(struct.pack("<H", 0))
-          EOF
+        run: python3 scripts/patch_min_chip_rev.py .pio/build/gh_release/firmware.bin
 
       - name: Upload Artifacts
         uses: actions/upload-artifact@v4
diff --git a/.github/workflows/release_candidate.yml b/.github/workflows/release_candidate.yml
index 7b814aea..2b333524 100644
--- a/.github/workflows/release_candidate.yml
+++ b/.github/workflows/release_candidate.yml
@@ -36,18 +36,7 @@ jobs:
         run: pio run -e gh_release_rc
 
       - name: Patch min_chip_rev_full to 0
-        run: |
-          python3 - <<'EOF'
-          import struct, sys
-          MIN_REV_FULL_OFFSET = 8 + 7
-          path = ".pio/build/gh_release_rc/firmware.bin"
-          with open(path, "r+b") as f:
-              f.seek(MIN_REV_FULL_OFFSET)
-              old = struct.unpack("<H", f.read(2))[0]
-              print(f"min_chip_rev_full: {old // 100}.{old % 100} -> 0.0")
-              f.seek(MIN_REV_FULL_OFFSET)
-              f.write(struct.pack("<H", 0))
-          EOF
+        run: python3 scripts/patch_min_chip_rev.py .pio/build/gh_release_rc/firmware.bin
 
       - name: Upload Artifacts
         uses: actions/upload-artifact@v4
diff --git a/scripts/patch_min_chip_rev.py b/scripts/patch_min_chip_rev.py
new file mode 100644
index 00000000..2d8ab619
--- /dev/null
+++ b/scripts/patch_min_chip_rev.py
@@ -0,0 +1,64 @@
+"""
+Patch min_chip_rev_full to 0 in a firmware.bin and recompute the appended
+SHA-256 digest so esp_image_verify() still passes.
+
+esp_image_header_t layout (24 bytes, all packed):
+  offset  0      magic (0xE9)
+  offset  1      segment_count
+  offset  2      spi_mode
+  offset  3      spi_speed:4 | spi_size:4
+  offset  4-7    entry_addr  (uint32 LE)
+  offset  8      wp_pin
+  offset  9-11   spi_pin_drv[3]
+  offset  12-13  chip_id     (uint16 LE)
+  offset  14     min_chip_rev (uint8, legacy)
+  offset  15-16  min_chip_rev_full (uint16 LE)  <-- patch target
+  offset  17-18  max_chip_rev_full (uint16 LE)
+  offset  19-22  reserved[4]
+  offset  23     hash_appended (uint8)
+
+When hash_appended == 1, the last 32 bytes of the image are the SHA-256 over
+bytes [0 .. len-33] (everything except the digest itself).  We recompute and
+overwrite those 32 bytes after patching the header.
+"""
+import hashlib
+import struct
+import sys
+
+MIN_CHIP_REV_FULL_OFFSET = 15  # byte offset of min_chip_rev_full in esp_image_header_t
+HASH_LEN = 32
+HASH_APPENDED_OFFSET = 23  # byte offset of hash_appended flag
+
+
+def patch(path: str) -> None:
+    with open(path, "r+b") as f:
+        data = bytearray(f.read())
+
+    if data[0] != 0xE9:
+        print(f"ERROR: {path}: not a valid ESP image (magic=0x{data[0]:02x})", file=sys.stderr)
+        sys.exit(1)
+
+    old = struct.unpack_from("<H", data, MIN_CHIP_REV_FULL_OFFSET)[0]
+    print(f"min_chip_rev_full: v{old // 100}.{old % 100} -> v0.0")
+    struct.pack_into("<H", data, MIN_CHIP_REV_FULL_OFFSET, 0)
+
+    hash_appended = data[HASH_APPENDED_OFFSET]
+    if hash_appended == 1:
+        # Digest covers everything except the last 32 bytes.
+        digest = hashlib.sha256(data[:-HASH_LEN]).digest()
+        data[-HASH_LEN:] = digest
+        print(f"SHA-256 recomputed and written to last {HASH_LEN} bytes")
+    else:
+        print("hash_appended=0, no digest to update")
+
+    with open(path, "wb") as f:
+        f.write(data)
+
+    print(f"Patched: {path}")
+
+
+if __name__ == "__main__":
+    if len(sys.argv) != 2:
+        print(f"Usage: {sys.argv[0]} <firmware.bin>", file=sys.stderr)
+        sys.exit(1)
+    patch(sys.argv[1])
diff --git a/src/activities/network/WifiSelectionActivity.cpp b/src/activities/network/WifiSelectionActivity.cpp
index 5235ea4c..1c45441b 100644
--- a/src/activities/network/WifiSelectionActivity.cpp
+++ b/src/activities/network/WifiSelectionActivity.cpp
@@ -280,6 +280,11 @@ bool WifiSelectionActivity::checkCaptivePortal() {
   String location = http.getLocation();
   http.end();
 
+  if (code < 0) {
+    LOG_DBG("WIFI", "Captive portal probe failed (connection error %d)", code);
+    return false;
+  }
+
   if (code == 204) {
     return false;  // Open internet, no captive portal
   }
diff --git a/src/main.cpp b/src/main.cpp
index 456d2bb3..f1914ee5 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -31,7 +31,6 @@
 #include "util/ButtonNavigator.h"
 #include "util/ScreenshotUtil.h"
 
-
 MappedInputManager mappedInputManager(gpio);
 GfxRenderer renderer(display);
 ActivityManager activityManager(renderer, mappedInputManager);
@@ -182,7 +181,13 @@ void setupDisplayAndFonts() {
 }
 
 void setup() {
-  esp_ota_mark_app_valid_cancel_rollback();
+  {
+    esp_ota_img_states_t otaState;
+    const esp_partition_t* running = esp_ota_get_running_partition();
+    if (esp_ota_get_state_partition(running, &otaState) == ESP_OK && otaState == ESP_OTA_IMG_PENDING_VERIFY) {
+      esp_ota_mark_app_valid_cancel_rollback();
+    }
+  }
   HalSystem::begin();
   gpio.begin();
   powerManager.begin();
diff --git a/src/network/OtaUpdater.cpp b/src/network/OtaUpdater.cpp
index e08ecac4..732b2e3f 100644
--- a/src/network/OtaUpdater.cpp
+++ b/src/network/OtaUpdater.cpp
@@ -201,7 +201,10 @@ const std::string& OtaUpdater::getLatestVersion() const { return latestVersion;
 
 void OtaUpdater::cleanupUpdate() {
   if (otaHandle) {
-    esp_https_ota_finish(otaHandle);
+    const esp_err_t err = esp_https_ota_finish(otaHandle);
+    if (err != ESP_OK) {
+      LOG_ERR("OTA", "esp_https_ota_finish on cleanup: %s", esp_err_to_name(err));
+    }
     otaHandle = nullptr;
   }
   cancelRequested = false;
@@ -365,20 +368,3 @@ OtaUpdater::OtaUpdaterError OtaUpdater::performInstallUpdateStep() {
   LOG_INF("OTA", "Update completed");
   return OK;
 }
-
-OtaUpdater::OtaUpdaterError OtaUpdater::installUpdate() {
-  const auto beginResult = beginInstallUpdate();
-  if (beginResult != UPDATE_IN_PROGRESS) {
-    return beginResult;
-  }
-
-  OtaUpdaterError result;
-  do {
-    result = performInstallUpdateStep();
-    if (result == UPDATE_IN_PROGRESS) {
-      delay(100);
-    }
-  } while (result == UPDATE_IN_PROGRESS);
-
-  return result;
-}
diff --git a/src/network/OtaUpdater.h b/src/network/OtaUpdater.h
index 26812fa3..4ed2b1a9 100644
--- a/src/network/OtaUpdater.h
+++ b/src/network/OtaUpdater.h
@@ -51,7 +51,6 @@ class OtaUpdater {
   OtaUpdaterError performInstallUpdateStep();
   void cancelUpdate();
   void cleanupUpdate();
-  OtaUpdaterError installUpdate();
 
  private:
   static int forceSetOtaBootPartition();