OpenDisplay · cryptomilk · Jul 27, 2026 · Jul 27, 2026
@@ -1,43 +1,72 @@
 # CLAUDE.md — Firmware
 
-Guidance for Claude Code when working in the **OpenDisplay `Firmware`** repo (nRF52840 + ESP32-S3/C6/C3 BLE e-paper tags). This is one repo inside the larger OpenDisplay multi-repo workspace; the workspace-level `../CLAUDE.md` covers cross-repo layout and the end-to-end pipeline. Prefer this file for anything Firmware-specific.
+Guidance for Claude Code when working in the **OpenDisplay `Firmware`** repo
+(nRF52840 + ESP32-S3/C6/C3 BLE e-paper tags). This is one repo inside the
+larger OpenDisplay multi-repo workspace; the workspace-level `../CLAUDE.md`
+covers cross-repo layout and the end-to-end pipeline. Prefer this file for
+anything Firmware-specific.
 
 ## Build & flash
 
-PlatformIO (Arduino framework), using the `bb_epaper` display library. Environments are defined in `platformio.ini`.
+PlatformIO (Arduino framework), using the `bb_epaper` display library.
+Environments are defined in `platformio.ini`.
 
 ```bash
 pio run -e <env>                 # build one environment
 pio run -e <env> -t upload       # build + flash
 pio run                          # build every environment
 ```
 
-Common envs: `nrf52840custom`, `esp32-s3-N16R8`, `esp32-s3-N8R8`, `esp32-c3-N16`, `esp32-c6-N4`. CI (`.github/workflows/main.yaml`) builds **all eleven** environments on every push — keep them all green.
+Common envs: `nrf52840custom`, `esp32-s3-N16R8`, `esp32-s3-N8R8`,
+`esp32-c3-N16`, `esp32-c6-N4`. CI (`.github/workflows/main.yaml`) builds **all
+eleven** environments on every push — keep them all green.
 
-Factory provisioning: `OPENDISPLAY_FACTORY_CONFIG_HEX="..." pio run -e <env>` (or `tools/provision_firmware.py`). `scripts/factory_config_gen.py` runs as a pre-build step.
+Factory provisioning: `OPENDISPLAY_FACTORY_CONFIG_HEX="..." pio run -e <env>`
+(or `tools/provision_firmware.py`). `scripts/factory_config_gen.py` runs as a
+pre-build step.
 
 ### Do NOT bump the ESP platform past IDF 5.5.4 (C6 / NimBLE-Arduino)
 
-Every ESP env pins pioarduino **`55.03.39`** (Arduino 3.3.9 / **IDF 5.5.4**) by exact version, never the floating `stable` URL. **Do not move to `55.03.311` (IDF 5.5.5) or later until NimBLE-Arduino ships a release built against it.**
-
-**NimBLE-Arduino 2.5.0 depends on IDF ≤ 5.5.4.** IDF 5.5.5 dropped the `r_` prefix on the C6 BLE controller's mempool exports and moved them from `libble_app.a` to `libbt.a` (`r_os_mempool_init` → `os_mempool_init`, same for `os_memblock_get`/`_put`). NimBLE-Arduino 2.5.0 — the latest release, 2026-04-02 — still calls the `r_` names, so `esp32-c6-N4` fails to link with a wall of `undefined reference to r_os_mempool_init`. There is no library-side fix; it has to land upstream in NimBLE-Arduino.
-
-Only C6 is affected: it is the sole target whose BLE controller ships as a precompiled blob carrying its own NimBLE porting layer (`CONFIG_BT_LE_CONTROLLER_NPL_OS_PORTING_SUPPORT=y`). ESP32/S3/C3 compile that layer from source and build fine on IDF 5.5.5.
-
-`scripts/esp32c6_nimble_mempool_link.py` force-links the mempool object for C6 and **warns-and-skips on every failure path** — never abort the build from it, or a future platform change hides the real linker error behind a missing-member message. Full analysis, including a `--defsym` workaround that links but is unvalidated on hardware: `docs/FINDINGS_C6_NIMBLE_IDF555_MEMPOOL_ABI_2026-07-25.md`.
-
-Note for whenever the bump does happen: `55.03.311` also requires PlatformIO Core ≥ 6.1.19 (it silently uninstalls itself on older Core with an `IncompatiblePlatform` error). The pinned `.39` has no such requirement.
+Every ESP env pins pioarduino **`55.03.39`** (Arduino 3.3.9 / **IDF 5.5.4**) by
+exact version, never the floating `stable` URL. **Do not move to `55.03.311`
+(IDF 5.5.5) or later until NimBLE-Arduino ships a release built against it.**
+
+**NimBLE-Arduino 2.5.0 depends on IDF ≤ 5.5.4.** IDF 5.5.5 dropped the `r_`
+prefix on the C6 BLE controller's mempool exports and moved them from
+`libble_app.a` to `libbt.a` (`r_os_mempool_init` → `os_mempool_init`, same for
+`os_memblock_get`/`_put`). NimBLE-Arduino 2.5.0 — the latest release,
+2026-04-02 — still calls the `r_` names, so `esp32-c6-N4` fails to link with a
+wall of `undefined reference to r_os_mempool_init`. There is no library-side
+fix; it has to land upstream in NimBLE-Arduino.
+
+Only C6 is affected: it is the sole target whose BLE controller ships as a
+precompiled blob carrying its own NimBLE porting layer
+(`CONFIG_BT_LE_CONTROLLER_NPL_OS_PORTING_SUPPORT=y`). ESP32/S3/C3 compile that
+layer from source and build fine on IDF 5.5.5.
+
+`scripts/esp32c6_nimble_mempool_link.py` force-links the mempool object for C6
+and **warns-and-skips on every failure path** — never abort the build from it,
+or a future platform change hides the real linker error behind a missing-member
+message. Full analysis, including a `--defsym` workaround that links but is
+unvalidated on hardware:
+`docs/FINDINGS_C6_NIMBLE_IDF555_MEMPOOL_ABI_2026-07-25.md`.
+
+Note for whenever the bump does happen: `55.03.311` also requires PlatformIO
+Core ≥ 6.1.19 (it silently uninstalls itself on older Core with an
+`IncompatiblePlatform` error). The pinned `.39` has no such requirement.
 
 ## The vendored protocol header (critical invariant)
 
-`include/opendisplay_protocol.h` is a **byte-for-byte vendored copy** of the canonical wire-protocol header. The canonical source lives in a sibling repo:
+`include/opendisplay_protocol.h` is a **byte-for-byte vendored copy** of the
+canonical wire-protocol header. The canonical source lives in a sibling repo:
 
 ```
 ../opendisplay-protocol/src/opendisplay_protocol.h   ← single source of truth
 include/opendisplay_protocol.h                        ← vendored copy (this repo)
 ```
 
-**Never hand-edit `include/opendisplay_protocol.h`.** To change the protocol, edit the canonical file, then propagate and verify from the protocol repo:
+**Never hand-edit `include/opendisplay_protocol.h`.** To change the protocol,
+edit the canonical file, then propagate and verify from the protocol repo:
 
 ```bash
 cd ../opendisplay-protocol
@@ -47,32 +76,80 @@ tools/sync_protocol_header.py --check --only Firmware   # fail if the copy drift
 
 What the header contains and what it does **not**:
 
-- It is **pure `#define` constants only** — protocol version, command opcodes (`CMD_*`), response codes (`RESP_*`), auth/error/NFC/pipe/config-limit constants, encryption sizes.
-- It deliberately contains **no structs, enums, typedefs, or functions**. Those are per-repo and live in `src/structs.h` (e.g. `SystemConfig`, `PowerOption`, `PipeWriteState`). Do not move struct definitions into the header.
+- It is **pure `#define` constants only** — protocol version, command opcodes
+  (`CMD_*`), response codes (`RESP_*`), auth/error/NFC/pipe/config-limit
+  constants, encryption sizes.
+- It deliberately contains **no structs, enums, typedefs, or functions**. Those
+  are per-repo and live in `src/structs.h` (e.g. `SystemConfig`, `PowerOption`,
+  `PipeWriteState`). Do not move struct definitions into the header.
 
 ### Rules when touching protocol code
 
-- Include the constants via `src/structs.h`, which is the common include hub (`#include "opendisplay_protocol.h"`). Most translation units get them transitively; add a direct include only where `structs.h` isn't already pulled in.
-- **Do not redefine** any macro the canonical header provides. Firmware previously hand-defined some of these (`RESP_*`, `CONFIG_CHUNK_SIZE*`, `MAX_CONFIG_CHUNKS`, `MAX_RESPONSE_DATA_SIZE`, `PIPE_*`); they now come from the header only. A local `#define` of the same name is a redefinition error waiting to happen.
-- Prefer named constants over magic numbers: use `CMD_PIPE_WRITE_START`, `CMD_CONFIG_WRITE`, etc. instead of raw `0x0080` / `0x0041` in switch/case dispatch (see `src/communication.cpp`).
-- Opcode/response values must always match the canonical spec — if a value looks wrong, fix it in `../opendisplay-protocol` and re-`--push`, never locally.
+- Include the constants via `src/structs.h`, which is the common include hub
+  (`#include "opendisplay_protocol.h"`). Most translation units get them
+  transitively; add a direct include only where `structs.h` isn't already
+  pulled in.
+- **Do not redefine** any macro the canonical header provides. Firmware
+  previously hand-defined some of these (`RESP_*`, `CONFIG_CHUNK_SIZE*`,
+  `MAX_CONFIG_CHUNKS`, `MAX_RESPONSE_DATA_SIZE`, `PIPE_*`); they now come from
+  the header only. A local `#define` of the same name is a redefinition error
+  waiting to happen.
+- Prefer named constants over magic numbers: use `CMD_PIPE_WRITE_START`,
+  `CMD_CONFIG_WRITE`, etc. instead of raw `0x0080` / `0x0041` in switch/case
+  dispatch (see `src/communication.cpp`).
+- Opcode/response values must always match the canonical spec — if a value
+  looks wrong, fix it in `../opendisplay-protocol` and re-`--push`, never
+  locally.
 
 ### Keep `tools/od-device-cli.py` in sync
 
-`tools/od-device-cli.py`'s `BLOCKS` dict duplicates the config-packet field layout (names, sizes, enum values) from `include/opendisplay_structs.h` so it can decode/encode config packets offline. It is **not** auto-generated. Whenever a config struct's fields are renamed, resized, or added/removed in `opendisplay_structs.h`, update `BLOCKS` (and `VALID_ENUMS` for new enum values) in the same change. A stale field name that happens to start with `reserved` silently vanishes from decoded YAML when its value is 0 — treat drift here as a correctness bug, not a cosmetic one.
+`tools/od-device-cli.py`'s `BLOCKS` dict duplicates the config-packet field
+layout (names, sizes, enum values) from `include/opendisplay_structs.h` so it
+can decode/encode config packets offline. It is **not** auto-generated.
+Whenever a config struct's fields are renamed, resized, or added/removed in
+`opendisplay_structs.h`, update `BLOCKS` (and `VALID_ENUMS` for new enum
+values) in the same change. A stale field name that happens to start with
+`reserved` silently vanishes from decoded YAML when its value is 0 — treat
+drift here as a correctness bug, not a cosmetic one.
 
 ## Source layout (`src/`)
 
 - `main.cpp` / `main.h` — entry point, boot, main loop.
 - `communication.cpp` — BLE command dispatch (opcode switch on `CMD_*`).
-- `structs.h` — config-packet structs + Firmware-local constants; includes the vendored protocol header.
+- `structs.h` — config-packet structs + Firmware-local constants; includes the
+  vendored protocol header.
 - `config_parser.*` — parse the factory/BLE config blob into the structs.
-- `display_service.*`, `display_fastepd.*` — rendering + panel drive (PIPE/DIRECT/PARTIAL write paths).
+- `display_service.*`, `display_fastepd.*` — rendering + panel drive
+  (PIPE/DIRECT/PARTIAL write paths).
 - `encryption.*`, `encryption_state.h` — BLE session auth + CCM.
-- `ble_init.*`, `esp32_ble_callbacks.h` — NimBLE-Arduino setup and callbacks (ESP32 migrated Bluedroid → NimBLE).
-- Peripherals: `buzzer_*`, `sensor_*`, `touch_input.*`, `wake_button.*`, `power_latch.*`, `wifi_service.*`.
+- `ble_init.*`, `esp32_ble_callbacks.h` — NimBLE-Arduino setup and callbacks
+  (ESP32 migrated Bluedroid → NimBLE).
+- Peripherals: `buzzer_*`, `sensor_*`, `touch_input.*`, `wake_button.*`,
+  `power_latch.*`, `wifi_service.*`.
+
+## Coding rules
+
+- **No `extern` for cross-file global access.** Don't reach into another
+  file's global variable with `extern`. Expose a getter (and setter, if
+  mutation is needed) instead. This keeps ownership of the variable in one
+  translation unit and makes reads/writes greppable and interceptable.
+- **Avoid heap allocation where possible.** These are memory-constrained
+  MCUs (nRF52840 / ESP32-S3 / C6 / C3) — prefer stack allocation, static
+  buffers, or fixed-size structs over `malloc`/`new`/dynamic containers.
+  If a heap allocation is unavoidable, call it out explicitly and justify it.
+- **Performance is a first-class constraint.** Before proposing or starting
+  any implementation plan, explicitly consider its performance impact (CPU
+  cycles, RAM/flash footprint, BLE/display timing) as part of the plan, not
+  as an afterthought.
+
+These rules apply to new and changed code. Don't mass-refactor pre-existing
+`extern` globals or heap usage as a side effect of unrelated work — flag it
+instead.
 
 ## Notes
 
-- `FINDINGS.md` and `docs/` hold working design/investigation notes (BLE throughput, pipe-write protocol, deep-sleep). Read the relevant one before changing that subsystem.
-- Run git commands inside this repo, not the workspace root. Branch + PR per repo.
+- `FINDINGS.md` and `docs/` hold working design/investigation notes (BLE
+  throughput, pipe-write protocol, deep-sleep). Read the relevant one before
+  changing that subsystem.
+- Run git commands inside this repo, not the workspace root. Branch + PR per
+  repo.
@@ -69,7 +69,6 @@ extern bool displayPowerState;
 extern volatile uint8_t pwrmgmState;
 extern uint32_t pwrmgmOffDeadlineMs;
 extern volatile uint8_t pwrmgmLock;
-extern uint32_t directWriteStartTime;
 extern uint32_t directWriteCompressedReceived;
 extern uint8_t directWriteRefreshMode;
 extern uint32_t directWriteTotalBytes;
@@ -117,9 +116,19 @@ struct PartialStreamContext {
     uint32_t bytes_received;
     uint32_t bytes_written;
     uint8_t current_plane;
-    uint32_t start_time;
+    uint32_t last_activity_ms;
 };
 
+// Shared by the direct-write and partial-write watchdogs below: how long a
+// transfer may sit with no data chunk arriving before it's considered dead.
+// Measured from the LAST chunk, not from transfer start, so a slow-but-flowing
+// stream (weak BLE link) is never punished for its total duration.
+static const uint32_t TRANSFER_IDLE_TIMEOUT_MS = 60000UL;
+
+// Timestamp of the last direct-write (0x71/0x81) chunk accepted, or of START if
+// no chunk has arrived yet. Drives checkDirectWriteTimeout() below.
+static uint32_t directWriteLastChunkMs = 0;
+
 #ifdef TARGET_ESP32
 extern BLEAdvertisementData* advertisementData;
 extern BLEServer* pServer;
@@ -576,16 +585,27 @@ static PipeWriteState pipeState = {};
 static PipeReorderSlot pipeReorder[PIPE_REORDER_SLOTS];
 
 void checkPartialWriteTimeout(void) {
-    if (partialCtx.active && partialCtx.start_time > 0 &&
-        (millis() - partialCtx.start_time) > 900000UL) {
-        od_log_error("ERROR: Partial write timeout - cleaning up stuck state");
+    if (partialCtx.active && partialCtx.last_activity_ms > 0 &&
+        (millis() - partialCtx.last_activity_ms) > TRANSFER_IDLE_TIMEOUT_MS) {
+        od_log_error("ERROR: Partial write idle timeout - cleaning up stuck state");
         cleanup_partial_write_state();
         // A pipe-partial transfer shares partialCtx: also clear pipeState so a zombie
         // pipeState.active can't misroute later 0x0081 frames into the dead partialCtx.
         if (pipeState.partial) resetPipeWriteState();
     }
 }
 
+void checkDirectWriteTimeout(void) {
+    if (!directWriteActive || directWriteLastChunkMs == 0) {
+        return;
+    }
+    uint32_t idleMs = millis() - directWriteLastChunkMs;
+    if (idleMs > TRANSFER_IDLE_TIMEOUT_MS) {
+        od_log_error("ERROR: Direct write idle timeout (%u ms since last chunk) - cleaning up stuck state", (unsigned)idleMs);
+        cleanupDirectWriteState(true);
+    }
+}
+
 // Disconnect hook: a partial session (0x76 or pipe-partial) powers the panel via
 // partial_prepare_panel_ram but never sets directWriteActive, so the disconnect
 // handlers' cleanupDirectWriteState gate misses it and the panel would stay
@@ -2019,10 +2039,10 @@ void cleanupDirectWriteState(bool refreshDisplay) {
     directWriteHeight = 0;
     directWriteTotalBytes = 0;
     directWriteRefreshMode = 0;
-    directWriteStartTime = 0;
+    directWriteLastChunkMs = 0;
     // Panel power acts only while a transfer/refresh is actually in flight
     // (PWR_ACTIVE). refreshDisplay==true is a terminal teardown (disconnect,
-    // 15-min timeout, mid-stream error) -> power fully off. refreshDisplay==false
+    // idle timeout, mid-stream error) -> power fully off. refreshDisplay==false
     // is the post-refresh path from directWriteFinishAndRefresh -> release to WARM
     // so keep-alive holds the rail for the next push.
     if (pwrmgmState == PWR_ACTIVE) {
@@ -2074,7 +2094,7 @@ static void directWriteComputeGeometry(bool compressed) {
 static void directWriteActivatePanel(void) {
     directWriteActive = true;
     directWriteBytesWritten = 0;
-    directWriteStartTime = millis();
+    directWriteLastChunkMs = millis();
     imageWriteLogStart(directWriteTotalBytes);
     // Full-frame write: acquire the session with the FULL init sequence. A warm
     // re-acquire skips the ~900 ms rail bring-up + bbepInitIO (replaces the old
@@ -2246,7 +2266,7 @@ void handlePartialWriteStart(uint8_t* data, uint16_t len) {
     partialCtx.expected_stream_size = expectedLogicalSize;
     partialCtx.plane_size = planeBytes;
     partialCtx.current_plane = 0xFF;
-    partialCtx.start_time = millis();
+    partialCtx.last_activity_ms = millis();
     imageWriteLogStart(expectedLogicalSize);
 
     partial_prepare_panel_ram();
@@ -2286,6 +2306,7 @@ void handleDirectWriteData(uint8_t* data, uint16_t len) {
     }
     if (!directWriteActive || len == 0) return;
     if (!frameOwnsSession("0x0071")) return;
+    directWriteLastChunkMs = millis();
     imageWriteLogChunk(data, len);
     if (directWriteCompressed) {
         if (!handleDirectWriteCompressedData(data, len)) {
@@ -2372,7 +2393,7 @@ void handleDirectWriteEnd(uint8_t* data, uint16_t len) {
 // {0xFF,endOpcode} on compressed-flush/completeness failure, then refreshes the
 // panel and emits {0x00,0x73}/{0x00,0x74}. Caller guarantees directWriteActive.
 static void directWriteFinishAndRefresh(uint8_t* data, uint16_t len, uint8_t endOpcode) {
-    directWriteStartTime = 0;
+    directWriteLastChunkMs = 0;
     if (directWriteCompressed && !zlib_stream_to_direct_write(nullptr, 0, true)) {
         cleanupDirectWriteState(true);
         uint8_t errorResponse[] = {0xFF, endOpcode};
@@ -2759,7 +2780,7 @@ void handlePipeWriteStart(uint8_t* data, uint16_t len) {
         partialCtx.expected_stream_size = total_size;
         partialCtx.plane_size = planeBytes;
         partialCtx.current_plane = 0xFF;
-        partialCtx.start_time = millis();
+        partialCtx.last_activity_ms = millis();
     }
 
     // Respond BEFORE panel bring-up: slow panels (Spectra/ACeP-class init can take
@@ -2811,6 +2832,7 @@ void handlePipeWriteData(uint8_t* data, uint16_t len) {
     if (!pipeState.active || pipeState.error) return;   // silent discard
     if (len < 1) return;
     if (!frameOwnsSession("0x0081")) return;
+    directWriteLastChunkMs = millis();
     uint8_t  seq     = data[0];
     uint8_t* payload = data + 1;
     uint16_t plen    = (uint16_t)(len - 1);
@@ -3122,6 +3144,7 @@ static bool partial_consume_bytes(uint8_t* data, uint32_t len) {
         }
     }
     partialCtx.bytes_received += len;
+    partialCtx.last_activity_ms = millis();
     if (partialCtx.compressed) return zlib_stream_to_partial_write(data, len, false);
     return partial_write_stream_bytes(data, len);
 }

@@ -77,6 +77,7 @@ bool imageWriteLogQuietFrame(const uint8_t* data, uint16_t len);
 extern volatile bool epdRefreshInProgress;
 void handlePartialWriteStart(uint8_t* data, uint16_t len);
 void checkPartialWriteTimeout(void);
+void checkDirectWriteTimeout(void);
 void cleanupPartialWriteOnDisconnect(void);
 // Origin (see commandOrigin()) of the transport that opened the in-flight transfer.
 // A disconnect must only tear down a session its own transport owns.