Refine OTA error handling and documentation for experimental APIs

Author: fwaibel

CHANGELOG.md

@@ -15,9 +15,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 
 - **OTA MVP** — three new crates for end-to-end firmware update on ESP32-C3 (and ESP32-C6 / ESP32 for the bare-metal stack), aligned with `docs/adr/011-ota-crate-hosting-and-transport.md` and `docs/features/ota-mvp-v1.md`. All public APIs are explicitly experimental for MVP; stabilization is owned by the future `ota-library` feature.
-  - `ota-pure` (new crate) — platform-independent, `no_std`, host-tested. Surface: `Version` semver parser (`u16` components, `Display`, `Ord`), `StreamingVerifier` (chunk-fed SHA-256 over `sha2 = { default-features = false }`), `bytes_to_hex` / `hex_to_bytes` fixed-size helpers (returns `heapless::String<64>`), `ImageMetadata` sidecar parser (`.bin.sha256` + `.bin.version`), backend-neutral `OtaState` enum (`Idle → Downloading → Verifying → Writing → SwapPending → Booted`) with `next_state()`, and `OtaError` with the 8 MVP variants (`ServerUnreachable`, `DownloadFailed { status: u16 }`, `DownloadTimeout`, `ChecksumMismatch`, `VersionInvalid`, `FlashWriteFailed`, `PartitionNotFound`, `InsufficientSpace`). 37 host unit tests.
-  - `rustyfarian-esp-idf-ota` (new crate) — ESP-IDF std, blocking, lifted from `rustyfarian-beekeeper/src/ota/`. Surface: `OtaSession::new(config)`, `fetch_and_apply(url, &expected_sha256)`, `mark_valid()`, `rollback()`. Wraps `EspOta` / `EspOtaUpdate` and `EspHttpConnection`; streams download → SHA-256 verify (`StreamingVerifier`) → flash → swap in one pass without holding the full image in RAM. Strips RFC 3986 userinfo from URLs before logging (no credential leakage to `espflash monitor`). HTTPS rejected at MVP scope per ADR 011 (`ota-hardened` will revisit).
-  - `rustyfarian-esp-hal-ota` (new crate) — bare-metal `no_std`, async-only, built fresh against `esp_bootloader_esp_idf::OtaUpdater` over `esp-storage` and `embassy-net::TcpSocket`. Surface: `EspHalOtaManager::new(config, FLASH<'d>)`, `async fetch_and_apply(socket, url, &expected_sha256)`, `mark_valid()`, `rollback()`. Carries an internal hand-rolled HTTP/1.1 GET parser (per ADR 011 §2): accepts only `HTTP/1.1 200 OK` with exactly one valid `Content-Length`; rejects redirects, `Transfer-Encoding: chunked`/`identity`, missing or duplicate `Content-Length`, non-`1*DIGIT` numeric values (incl. leading `+`/`-`), whitespace before colon, oversized bodies, and short reads. Chip features `esp32c3` (MVP), `esp32c6`, `esp32`; stack features `unstable`, `rt`, `embassy`. Host stub mirrors the wifi-crate pattern (typecheck-only). 29 parser unit tests.
+  - `ota-pure` (new crate, **experimental API**) — platform-independent, `no_std`, host-tested. Surface: `Version` semver parser (`u16` components, `Display`, `Ord`), `StreamingVerifier` (chunk-fed SHA-256 over `sha2 = { default-features = false }`), `bytes_to_hex` / `hex_to_bytes` fixed-size helpers (returns `heapless::String<64>`), `ImageMetadata` sidecar parser (`.bin.sha256` + `.bin.version`), backend-neutral `OtaState` enum (`Idle → Downloading → Verifying → Writing → SwapPending → Booted`) with `next_state()`, and `OtaError` with the 8 MVP variants (`ServerUnreachable`, `DownloadFailed { status: u16 }`, `DownloadTimeout`, `ChecksumMismatch`, `VersionInvalid`, `FlashWriteFailed`, `PartitionNotFound`, `InsufficientSpace`). `DownloadFailed { status: 0 }` is reserved as a sentinel for protocol-shape rejections from the bare-metal HTTP client. 37 host unit tests.
+  - `rustyfarian-esp-idf-ota` (new crate, **experimental API**, blocking) — ESP-IDF std, lifted from `rustyfarian-beekeeper/src/ota/`. Surface: `OtaSession::new(config)`, `fetch_and_apply(url, &expected_sha256)`, `mark_valid()`, `rollback()`. Wraps `EspOta` / `EspOtaUpdate` and `EspHttpConnection`; streams download → SHA-256 verify (`StreamingVerifier`) → flash → swap in one pass without holding the full image in RAM. Strips RFC 3986 userinfo from URLs before logging (no credential leakage to `espflash monitor`). HTTPS rejected at MVP scope per ADR 011 (`ota-hardened` will revisit).
+  - `rustyfarian-esp-hal-ota` (new crate, **experimental API**, async-only) — bare-metal `no_std`, built fresh against `esp_bootloader_esp_idf::OtaUpdater` over `esp-storage` and `embassy-net::TcpSocket`. Surface: `EspHalOtaManager::new(config, FLASH<'d>)`, `async fetch_and_apply(socket, url, &expected_sha256)`, `mark_valid()`, `rollback()`. Carries an internal hand-rolled HTTP/1.1 GET parser (per ADR 011 §2): accepts only `HTTP/1.1 200 OK` with exactly one valid `Content-Length`; rejects redirects, `Transfer-Encoding: chunked`/`identity`, missing or duplicate `Content-Length`, non-`1*DIGIT` numeric values (incl. leading `+`/`-`), whitespace before colon, oversized bodies, and short reads. Chip features `esp32c3` (MVP), `esp32c6`, `esp32`; stack features `unstable`, `rt`, `embassy`. Host stub mirrors the wifi-crate pattern (typecheck-only). 29 parser unit tests.
 - Workspace: `sha2 = { version = "0.10", default-features = false }`, `esp-storage = "=0.9.0"`, `embedded-storage = "0.3"` added to `[workspace.dependencies]`; `embedded-svc = "0.29"` declared (aligning with `esp-idf-svc 0.52`).
 - Justfile: `check-ota-pure`, `test-ota`, `check-ota-idf`, `check-ota-hal`, `check-ota-hal-embassy`, `test-ota-hal` recipes; `test` aggregate extended.
 - CI (`.github/workflows/rust.yml`): host tests for `ota-pure` and `rustyfarian-esp-hal-ota --no-default-features` added to the "Test pure crates" block.

README.md

@@ -55,8 +55,8 @@ a pattern common in application development but rare in embedded Rust.
 | [`espnow-pure`](crates/espnow-pure)                               | Platform-independent ESP-NOW types, traits, and validation; `no_std`; unit-testable on host |
 | [`rustyfarian-esp-idf-espnow`](crates/rustyfarian-esp-idf-espnow) | ESP-NOW driver for ESP-IDF projects, implementing the `EspNowDriver` trait                  |
 | [`ota-pure`](crates/ota-pure)                                     | Platform-independent OTA primitives — `Version`, streaming SHA-256, sidecar metadata        |
-| [`rustyfarian-esp-idf-ota`](crates/rustyfarian-esp-idf-ota)       | ESP-IDF OTA driver — streaming download, SHA-256 verify, partition swap, rollback           |
-| [`rustyfarian-esp-hal-ota`](crates/rustyfarian-esp-hal-ota)       | Bare-metal OTA driver — async strict HTTP/1.1 over `embassy-net` + `OtaUpdater` (MVP)       |
+| [`rustyfarian-esp-idf-ota`](crates/rustyfarian-esp-idf-ota)       | ESP-IDF OTA driver — **blocking**; streaming download, SHA-256 verify, partition swap, rollback |
+| [`rustyfarian-esp-hal-ota`](crates/rustyfarian-esp-hal-ota)       | Bare-metal OTA driver — **async-only**; strict HTTP/1.1 over `embassy-net` + `OtaUpdater` (MVP) |
 
 ## Usage
 

crates/ota-pure/src/error.rs

@@ -9,9 +9,17 @@ use core::fmt;
 pub enum OtaError {
     /// The update server could not be reached.
     ServerUnreachable,
-    /// The download request returned a non-200 HTTP status.
+    /// The download request returned a non-200 HTTP status, or the server
+    /// response failed a strict-protocol check.
+    ///
+    /// `status == 0` is a sentinel for "protocol-shape rejection" — used by
+    /// the bare-metal HTTP client when a response is syntactically rejected
+    /// before a status code is meaningful (e.g. `Transfer-Encoding: chunked`
+    /// or another unsupported response shape). Any non-zero value is the
+    /// HTTP status code returned by the server.
     DownloadFailed {
-        /// HTTP status code returned by the server.
+        /// HTTP status code returned by the server, or `0` for a
+        /// protocol-shape rejection (see variant docs).
         status: u16,
     },
     /// The download did not complete within the allowed time.

crates/ota-pure/src/verifier.rs

@@ -90,8 +90,8 @@ pub fn bytes_to_hex(bytes: &[u8; 32]) -> heapless::String<64> {
     let mut s = heapless::String::<64>::new();
     for &b in bytes {
         // Capacity is exactly 64 and we push exactly 64 chars — these cannot fail.
-        let _ = s.push(HEX_CHARS[(b >> 4) as usize] as char);
-        let _ = s.push(HEX_CHARS[(b & 0x0f) as usize] as char);
+        s.push(HEX_CHARS[(b >> 4) as usize] as char).unwrap();
+        s.push(HEX_CHARS[(b & 0x0f) as usize] as char).unwrap();
     }
     s
 }

crates/rustyfarian-esp-hal-ota/src/manager.rs

@@ -194,9 +194,12 @@ impl<'d> EspHalOtaManager<'d> {
                 })?
                 .map_err(|_| OtaError::DownloadTimeout)?;
             if n == 0 {
-                // EOF before Content-Length bytes received.
+                // EOF before Content-Length bytes received — peer closed the
+                // socket mid-body. This is a protocol-shape failure (server
+                // declared a length it did not deliver), not a stall, so use
+                // the `status: 0` sentinel rather than `DownloadTimeout`.
                 log::error!("OTA: short read — EOF before {} bytes remaining", remaining);
-                return Err(OtaError::DownloadTimeout);
+                return Err(OtaError::DownloadFailed { status: 0 });
             }
             let chunk = &chunk_buf[..n];
             verifier.update(chunk);

crates/rustyfarian-esp-idf-ota/src/downloader.rs

@@ -86,9 +86,14 @@ impl FirmwareDownloader {
         let mut buffer = [0u8; DOWNLOAD_BUFFER_SIZE];
 
         loop {
+            // The embedded-svc `read()` error type collapses connection-reset,
+            // DNS-mid-stream, and read-timeout into one `IOError`. Map all of
+            // them to `ServerUnreachable` — most production failures are
+            // connection-shaped, and a true read-timeout is also a server that
+            // stopped answering. A future hardened build can differentiate.
             let bytes_read = client.read(&mut buffer).map_err(|e| {
                 log::error!("Read error during firmware download: {:?}", e);
-                OtaError::DownloadTimeout
+                OtaError::ServerUnreachable
             })?;
 
             if bytes_read == 0 {

crates/rustyfarian-esp-idf-ota/src/lib.rs

@@ -153,10 +153,13 @@ impl OtaSession {
         })?;
 
         // `mark_running_slot_invalid_and_reboot` never returns on success (device reboots).
-        // It only returns on failure — treat the returned error as FlashWriteFailed.
+        // It only returns on failure — surface as `FlashWriteFailed` so callers
+        // can distinguish "no rollback target" (would surface as
+        // `PartitionNotFound` from `EspOta::new` above) from "rollback path
+        // failed for some other reason".
         let err = ota.mark_running_slot_invalid_and_reboot();
         log::error!("Rollback failed (no valid previous slot?): {:?}", err);
-        Err(OtaError::PartitionNotFound)
+        Err(OtaError::FlashWriteFailed)
     }
 }