fix: preserve last image and overlay yellow border on sync failure

Previously a 5xx / timeout / malformed response fired epd_fill(COLOR_YELLOW),
which writes the yellow nibble across the entire 800×480 framebuffer and
destroys the last good image — exactly what FR38 forbids ("Last image
persists ... yellow border signals state"). The device then got stuck on a
blank yellow screen because the next 304 didn't redraw.

Changes:

- New epd_draw_image_with_border streams the cached .bin row-by-row,
  overwrites border-region pixels in the row buffer, and pushes a single
  composited framebuffer (same pattern as the existing setup-QR overlay).
- normal_operation_impl else-branch now redraws the cached image with a
  yellow border, falling back to epd_fill only when no cache exists
  (first-boot error). Sets a new NVS_KEY_ERR_BORDER flag.
- 200 and 304 paths clear NVS_KEY_ERR_BORDER. The 304 branch now
  triggers a clean repaint when the err flag is set, so the device
  recovers from the stuck-yellow state on the next healthy poll
  without waiting for rotation to advance.
- LittleFS read mock now returns invalid File when the file doesn't
  exist (matches real LittleFS), so the no-cache fallback path is
  actually exercisable in tests.

Tests:

- Replaces the old test_fw06_error_fills_yellow (which locked in the
  buggy fill behavior) with FW-06a..e covering: error+cache draws
  border (no fill), error+no-cache falls back to fill, 304 after
  error repaints clean, steady-state 304 touches nothing (the
  regression the user flagged), 200 after error clears the flag.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

src/config.h

@@ -30,6 +30,10 @@
 #define NVS_KEY_PASS        "pass"
 #define NVS_KEY_IMG_ID      "img_id"
 #define NVS_KEY_DRAW_NEEDED "draw"
+#define NVS_KEY_ERR_BORDER  "err"     // set when display is showing a sync-fail border; force a clean redraw on next 200/304
+
+// Width of the sync-fail / no-WiFi border, in pixels.
+#define BORDER_THICKNESS_PX 16
 
 // ── Network ──────────────────────────────────────────────────────────────────
 #define APP_BASE_URL      "https://pictureframe.edholm.me"

src/epd.cpp

@@ -84,6 +84,41 @@ void epd_draw_image_from_file(fs::File& f) {
     epd_refresh();
 }
 
+void epd_draw_image_with_border(fs::File& f, uint8_t color, int thickness) {
+    const size_t expected = (size_t)EPD_WIDTH * EPD_HEIGHT / 2;
+    if (f.size() != expected) {
+        epd_fill(color);
+        return;
+    }
+
+    const uint8_t pair = (color << 4) | color;
+
+    cmd(0x10);
+    for (int y = 0; y < EPD_HEIGHT; y++) {
+        f.read(s_row, sizeof(s_row));
+
+        if (y < thickness || y >= EPD_HEIGHT - thickness) {
+            // Top/bottom band — solid color across the row.
+            for (size_t i = 0; i < sizeof(s_row); i++) s_row[i] = pair;
+        } else {
+            // Middle band — overlay border on left/right edges.
+            for (int x = 0; x < thickness; x++) {
+                if (x & 1) s_row[x/2] = (s_row[x/2] & 0xF0) | color;
+                else       s_row[x/2] = (s_row[x/2] & 0x0F) | (color << 4);
+            }
+            for (int x = EPD_WIDTH - thickness; x < EPD_WIDTH; x++) {
+                if (x & 1) s_row[x/2] = (s_row[x/2] & 0xF0) | color;
+                else       s_row[x/2] = (s_row[x/2] & 0x0F) | (color << 4);
+            }
+        }
+
+        digitalWrite(PIN_DC, HIGH); digitalWrite(PIN_CS, LOW);
+        SPI.writeBytes(s_row, sizeof(s_row));
+        digitalWrite(PIN_CS, HIGH);
+    }
+    epd_refresh();
+}
+
 void epd_draw_qr(QRCode* qr, uint8_t cellPx, uint8_t bg, uint8_t fg) {
     int qrPx = qr->size * cellPx;
     int offX  = (EPD_WIDTH  - qrPx) / 2;

src/epd.h

@@ -8,6 +8,11 @@ void epd_sleep();
 void epd_fill(uint8_t color);
 void epd_draw_image_from_file(fs::File& f);
 
+// Draw the image streamed from `f` with a `thickness`-pixel border of `color`
+// overlaid. If `f` is not a full-frame .bin (size mismatch), falls back to
+// epd_fill(color) so callers don't have to pre-validate.
+void epd_draw_image_with_border(fs::File& f, uint8_t color, int thickness);
+
 // Draw a QR code centred on the display.
 // bg/fg are EPD color nibbles (COLOR_WHITE / COLOR_BLACK).
 struct QRCode;

src/operation.h

@@ -66,6 +66,7 @@ void normal_operation_impl(const String& mac, HTTP& http, const String& url, Pre
     prefs.begin(NVS_NAMESPACE, true);
     int32_t currentImgId = prefs.getInt(NVS_KEY_IMG_ID, -1);
     bool drawNeeded = prefs.getInt(NVS_KEY_DRAW_NEEDED, 0) != 0;
+    bool errBorder  = prefs.getInt(NVS_KEY_ERR_BORDER, 0) != 0;
     prefs.end();
 
     if (currentImgId >= 0) {
@@ -107,16 +108,19 @@ void normal_operation_impl(const String& mac, HTTP& http, const String& url, Pre
         File r = LittleFS.open(IMAGE_PATH, "r");
         if (r) { epd_draw_image_from_file(r); r.close(); }
 
-        // Draw complete — clear the pending flag.
+        // Draw complete — clear pending and error-border flags. The fresh
+        // image fully overwrites any prior border, so error state is gone.
         prefs.begin(NVS_NAMESPACE, false);
         prefs.putInt(NVS_KEY_DRAW_NEEDED, 0);
+        prefs.putInt(NVS_KEY_ERR_BORDER, 0);
         prefs.end();
 
     } else if (code == 304) {
         http.end();
-        // If a previous draw was interrupted (power loss mid-refresh), the image
+        // Redraw from LittleFS if either: a previous draw was interrupted
-        // file is in LittleFS and the ID is correct in NVS — just re-draw it.
+        // (drawNeeded), or a sync-fail border is currently on screen and the
-        if (drawNeeded) {
+        // server is healthy again (errBorder) — repaint clean to clear it.
+        if (drawNeeded || errBorder) {
             File r = LittleFS.open(IMAGE_PATH, "r");
             if (r) {
                 displayInitialized = true;
@@ -125,6 +129,7 @@ void normal_operation_impl(const String& mac, HTTP& http, const String& url, Pre
                 r.close();
                 prefs.begin(NVS_NAMESPACE, false);
                 prefs.putInt(NVS_KEY_DRAW_NEEDED, 0);
+                prefs.putInt(NVS_KEY_ERR_BORDER, 0);
                 prefs.end();
             }
         }
@@ -139,10 +144,23 @@ void normal_operation_impl(const String& mac, HTTP& http, const String& url, Pre
         epd_init();
         show_setup_qr(mac);
     } else {
+        // Sync failed (5xx, timeout, malformed). Per FR38, the last-good image
+        // must persist; only the border indicates the error. epd_draw_image_with_border
+        // falls back to a full fill if the cached file is missing or wrong size,
+        // so first-boot error still gets a visible signal.
         http.end();
         displayInitialized = true;
         epd_init();
-        epd_fill(COLOR_YELLOW);
+        File r = LittleFS.open(IMAGE_PATH, "r");
+        if (r) {
+            epd_draw_image_with_border(r, COLOR_YELLOW, BORDER_THICKNESS_PX);
+            r.close();
+        } else {
+            epd_fill(COLOR_YELLOW);
+        }
+        prefs.begin(NVS_NAMESPACE, false);
+        prefs.putInt(NVS_KEY_ERR_BORDER, 1);
+        prefs.end();
     }
 
     // Only power off the display if it was initialized this cycle. Calling

test/mocks/LittleFS.h

@@ -29,11 +29,24 @@ struct LittleFSClass {
 
     File open(const char* path, const char* mode, bool create = false) {
         File f;
-        f._valid = true;
         f._write = (mode[0] == 'w');
-        f._buf = &files[path];
-        if (f._write) f._buf->clear();
         f._pos = 0;
+        if (f._write) {
+            f._buf   = &files[path];
+            f._buf->clear();
+            f._valid = true;
+        } else {
+            // Read mode: behave like real LittleFS — return invalid when file
+            // doesn't exist (do NOT create an empty entry via operator[]).
+            auto it = files.find(path);
+            if (it == files.end()) {
+                f._valid = false;
+                f._buf   = nullptr;
+            } else {
+                f._buf   = &it->second;
+                f._valid = true;
+            }
+        }
         return f;
     }
 } LittleFS;

test/mocks/epd_mock.h

@@ -12,6 +12,9 @@ extern int g_epd_draw_image_count;
 extern int g_epd_fill_count;
 extern int g_epd_fill_last_color;
 extern int g_epd_draw_setup_count;
+extern int g_epd_draw_border_count;
+extern int g_epd_draw_border_last_color;
+extern int g_epd_draw_border_last_thickness;
 
 inline void epd_init()  { g_epd_init_count++; }
 inline void epd_sleep() { g_epd_sleep_count++; }
@@ -21,5 +24,10 @@ inline void epd_draw_image_from_file(File& f) {
     g_call_seq++;
 }
 inline void epd_fill(int color) { g_epd_fill_count++; g_epd_fill_last_color = color; }
+inline void epd_draw_image_with_border(File& f, int color, int thickness) {
+    g_epd_draw_border_count++;
+    g_epd_draw_border_last_color     = color;
+    g_epd_draw_border_last_thickness = thickness;
+}
 inline void epd_draw_ap_screen(void*) {}
 inline void epd_draw_setup_screen(void*) { g_epd_draw_setup_count++; }

test/test_normal_operation/test_main.cpp

@@ -30,6 +30,7 @@ std::string g_http_body;
 
 int g_epd_init_count, g_epd_sleep_count, g_epd_draw_image_count;
 int g_epd_fill_count, g_epd_fill_last_color, g_epd_draw_setup_count;
+int g_epd_draw_border_count, g_epd_draw_border_last_color, g_epd_draw_border_last_thickness;
 
 uint64_t g_sleep_us;
 bool g_deep_sleep_started;
@@ -62,6 +63,7 @@ void reset_state() {
     g_http_body = "TESTDATA";
     g_epd_init_count = g_epd_sleep_count = g_epd_draw_image_count = 0;
     g_epd_fill_count = g_epd_fill_last_color = g_epd_draw_setup_count = 0;
+    g_epd_draw_border_count = g_epd_draw_border_last_color = g_epd_draw_border_last_thickness = 0;
     g_sleep_us = 0;
     g_deep_sleep_started = false;
     g_show_setup_qr_count = 0;
@@ -133,12 +135,83 @@ void test_fw05_404_shows_setup_qr() {
     TEST_ASSERT_EQUAL(0, g_epd_draw_image_count);
 }
 
-// FW-06: other error — epd_fill yellow
+// FW-06a: 5xx error WITH a cached image → preserve last image and overlay a
-void test_fw06_error_fills_yellow() {
+// yellow BORDER (per FR38). MUST NOT fill the screen with yellow — that would
+// destroy the last good image. Sets the err_border NVS flag so the next
+// healthy response repaints clean.
+void test_fw06a_error_with_cache_draws_border_not_fill() {
     g_http_get_code = 500;
+    LittleFS.files[IMAGE_PATH] = "IMGDATA";
+
     normal_operation_impl(String("mac"), http, String("url"), prefs);
+
+    TEST_ASSERT_EQUAL_MESSAGE(0, g_epd_fill_count,
+        "epd_fill must NOT be called when a cached image exists — it would obliterate the photo");
+    TEST_ASSERT_EQUAL(1, g_epd_draw_border_count);
+    TEST_ASSERT_EQUAL(COLOR_YELLOW, g_epd_draw_border_last_color);
+    TEST_ASSERT_EQUAL(BORDER_THICKNESS_PX, g_epd_draw_border_last_thickness);
+    TEST_ASSERT_EQUAL(1, prefs.getInt(NVS_KEY_ERR_BORDER, -1));
+}
+
+// FW-06b: 5xx error with NO cached image → fall back to full yellow fill so
+// the user still sees a sync-fail signal on a fresh device.
+void test_fw06b_error_without_cache_falls_back_to_fill() {
+    g_http_get_code = 500;
+    // LittleFS has no IMAGE_PATH entry
+
+    normal_operation_impl(String("mac"), http, String("url"), prefs);
+
+    TEST_ASSERT_EQUAL(0, g_epd_draw_border_count);
     TEST_ASSERT_EQUAL(1, g_epd_fill_count);
     TEST_ASSERT_EQUAL(COLOR_YELLOW, g_epd_fill_last_color);
+    TEST_ASSERT_EQUAL(1, prefs.getInt(NVS_KEY_ERR_BORDER, -1));
+}
+
+// FW-06c: 304 with err_border flag set (sync recovered after a previous
+// failure) → repaint the cached image clean and clear the flag.
+void test_fw06c_304_after_error_repaints_clean() {
+    g_http_get_code = 304;
+    prefs.ints[NVS_KEY_ERR_BORDER] = 1;
+    prefs.ints[NVS_KEY_IMG_ID]     = 7;
+    LittleFS.files[IMAGE_PATH]     = "IMGDATA";
+
+    normal_operation_impl(String("mac"), http, String("url"), prefs);
+
+    TEST_ASSERT_EQUAL(1, g_epd_init_count);
+    TEST_ASSERT_EQUAL(1, g_epd_draw_image_count);
+    TEST_ASSERT_EQUAL_MESSAGE(0, g_epd_draw_border_count,
+        "304 with err flag must redraw clean — no border on the recovery frame");
+    TEST_ASSERT_EQUAL(0, prefs.getInt(NVS_KEY_ERR_BORDER, -1));
+}
+
+// FW-06d: 304 (same image) with NO error or pending state must NOT touch the
+// display. Specifically must not invoke any yellow path. Locks down the
+// regression the user reported: 304 was suspected of triggering yellow fill.
+void test_fw06d_304_steady_state_does_not_fill_yellow() {
+    g_http_get_code = 304;
+    prefs.ints[NVS_KEY_IMG_ID] = 7;
+    LittleFS.files[IMAGE_PATH] = "IMGDATA";
+    // err_border = 0, draw_needed = 0 (defaults)
+
+    normal_operation_impl(String("mac"), http, String("url"), prefs);
+
+    TEST_ASSERT_EQUAL(0, g_epd_fill_count);
+    TEST_ASSERT_EQUAL(0, g_epd_draw_border_count);
+    TEST_ASSERT_EQUAL(0, g_epd_draw_image_count);
+    TEST_ASSERT_EQUAL(0, g_epd_init_count);
+}
+
+// FW-06e: 200 response after a previous error border → fresh image fully
+// overwrites the framebuffer, err_border flag cleared.
+void test_fw06e_200_after_error_clears_flag() {
+    g_http_response_headers["X-Image-Id"] = "8";
+    g_http_body = "BINDATA";
+    prefs.ints[NVS_KEY_ERR_BORDER] = 1;
+
+    normal_operation_impl(String("mac"), http, String("url"), prefs);
+
+    TEST_ASSERT_EQUAL(1, g_epd_draw_image_count);
+    TEST_ASSERT_EQUAL(0, prefs.getInt(NVS_KEY_ERR_BORDER, -1));
 }
 
 // FW-07: NVS has saved img_id → X-Current-Image-Id header sent
@@ -233,7 +306,11 @@ int main(int argc, char** argv) {
     RUN_TEST(test_fw03_304_no_redraw);
     RUN_TEST(test_fw04_204_shows_setup_qr);
     RUN_TEST(test_fw05_404_shows_setup_qr);
-    RUN_TEST(test_fw06_error_fills_yellow);
+    RUN_TEST(test_fw06a_error_with_cache_draws_border_not_fill);
+    RUN_TEST(test_fw06b_error_without_cache_falls_back_to_fill);
+    RUN_TEST(test_fw06c_304_after_error_repaints_clean);
+    RUN_TEST(test_fw06d_304_steady_state_does_not_fill_yellow);
+    RUN_TEST(test_fw06e_200_after_error_clears_flag);
     RUN_TEST(test_fw07_current_image_id_sent_when_saved);
     RUN_TEST(test_fw08_no_current_image_id_when_default);
     RUN_TEST(test_fw09_server_interval_honored);