Add TODO.md handoff notes for the remaining review items

Records the design-level suggestions from the 2026-08-03 review that were
not part of dd2d428 (UI freeze during fetches, stale-data indication,
debug readout removal, dead utils.c, OTA partitions, small cleanups),
plus the facts already verified and questions already answered so a
future agent does not re-derive or re-ask them.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

README.md

@@ -130,7 +130,10 @@ Two 800×480×2 framebuffers (750 KB each) live in PSRAM. The 8 MB **octal**
 PSRAM at 80 MHz is required — QSPI cannot sustain the RGB refresh.
 
 Do **not** enable `CONFIG_SPIRAM_FETCH_INSTRUCTIONS` / `CONFIG_SPIRAM_RODATA`:
-they contend with the panel for PSRAM bandwidth. Do **not** enable the RGB
-bounce buffer either — its refill ISR reads PSRAM, which shares the cache with
-flash on the S3, so any NVS write faults it with *"Cache disabled but cached
-memory region accessed"*.
+they contend with the panel for PSRAM bandwidth. The bounce buffer **is**
+enabled (the panel cannot stream straight from PSRAM), which is exactly why
+flash writes are dangerous while the panel runs: the refill ISR reads PSRAM,
+which shares the cache with flash on the S3, so an NVS write faults it with
+*"Cache disabled but cached memory region accessed"*. All flash writes happen
+before `display_init()` and `CONFIG_ESP_WIFI_NVS_ENABLED` is off to keep the
+WiFi stack from writing later.

TODO.md

@@ -0,0 +1,134 @@
+# TODO — remaining items from the 2026-08-03 code review
+
+Context for whoever (human or AI agent) picks this up. A full review of this
+firmware was done on 2026-08-03. The outright bugs it found were fixed in
+commit `dd2d428` ("Fix WiFi reconnect bugs, negative-temp rounding, and stale
+docs"). This file holds the items that were deliberately **not** done in that
+commit — design-level changes and enhancements — plus the facts the review
+already established so you don't re-derive or re-ask them.
+
+Read `README.md` first. It is accurate as of `dd2d428` and documents the
+hardware constraints that make this board unusual (PSRAM bandwidth, the
+STC8H1K28 board controller, init-order requirements). Do not violate the
+init order in `main.c`: WiFi/NVS before `display_init()`, `backlight_init()`
+before `touch_init()`.
+
+## Already verified — do not re-investigate
+
+- The build compiles clean on ESP-IDF v5.3 (`. ~/esp/esp-idf/export.sh &&
+  idf.py build`). Binary is ~1.1 MB, 74% of the 4 MB factory partition free.
+- `main/app_config.h` exists locally (gitignored, copied from the example
+  with empty credentials, done only to make the build compile). Real WiFi +
+  Ambient Weather credentials still need to be filled in before flashing.
+- `resp_buf` bounds/NUL-termination in `ambient_weather.c` is correct — the
+  guard leaves room for the terminator. Not a bug.
+- GT911 read-then-clear-status ordering in `touch_gt911.c` is correct.
+- Backlight duty inversion in `backlight.c` maps correctly at both endpoints
+  (255→0x00 full, 0→0xF5 off).
+- The `°` glyph exists in LVGL's built-in Montserrat fonts; the `·` separator
+  that does NOT exist in them only appears in log strings, never on screen.
+- The LVGL recursive-mutex locking discipline is consistent across all UI
+  entry points.
+- WiFi reconnect, `wifi_is_connected()` staleness, boot-timeout handling, and
+  negative-temperature rounding were all fixed in `dd2d428`. Don't redo them.
+
+## Tasks, in priority order
+
+### 1. Stop freezing the UI during weather fetches
+
+**Problem:** `weather_update_task` in `main.c` wraps the whole HTTP fetch in
+`lvgl_port_pause()`/`resume()`. Pause stops `lv_timer_handler()` entirely, so
+rendering AND touch input are dead for the whole fetch — worst case
+`WEATHER_TIMEOUT_MS` (30 s), every 60 s. Harmless while the screen is
+passive; a showstopper once thermostat controls (setpoint buttons) exist,
+which is the stated next step in the README.
+
+**Why the pause exists:** the TLS session, the LVGL redraw, and the flush all
+contend with the LCD's own DMA out of PSRAM; overlapping them produces the
+left-edge artefacts documented in the README. Do not simply delete the pause
+— the artefacts will come back. Options, roughly in order of preference:
+
+- Keep the HTTP client alive between polls (`keep_alive_enable` in
+  `esp_http_client_config_t`, don't `cleanup` between calls). The expensive
+  part is the TLS handshake; a reused session makes each poll a short
+  request/response, so the paused window shrinks from seconds to ~100s of ms.
+- After the first successful fetch, drop the timeout from 30 s to ~10 s (the
+  30 s exists only because a *first* handshake on this board is slow).
+- Alternatively pause only around LVGL's flush (the actual PSRAM writes)
+  rather than the whole network transaction — more invasive, only if the
+  above is insufficient.
+
+**Verify on hardware**: watch for the left-edge artefacts after weather
+fetches (they appear near the moment the status line updates). This cannot be
+verified by compiling alone.
+
+### 2. Indicate stale weather data
+
+`weather_data_t.last_update` is set in `ambient_weather.c` but never read.
+After fetch failures the old temperature stays on screen at full styling
+indefinitely. Track the last successful update (the weather task is the
+natural owner); after ~2 missed polls, dim the temperature label
+(e.g. change its text color) or append the age to the status line. Keep it in
+the weather task / `ui_manager` — do not add timekeeping to the API client.
+
+### 3. Remove the debug touch readout from the production UI
+
+`touch_label` + `touch_readout_timer_cb` + the 100 ms `lv_timer` in
+`ui_manager.c` are bring-up diagnostics baked into the shipped screen. Remove
+them or gate behind a compile-time flag (e.g. `#ifdef UI_DEBUG_TOUCH`,
+default off). Keep the explanatory comment about LVGL not bubbling events to
+parents somewhere — it documents a real trap that cost debugging time.
+
+### 4. Delete dead code: `main/utils/utils.c`
+
+`utils_delay_ms`, `utils_get_tick_ms`, `utils_trim` have zero callers;
+nothing outside utils.c includes `utils.h`. Delete `main/utils/` and remove
+its entries from `main/CMakeLists.txt` (SRCS and INCLUDE_DIRS), and drop the
+row from README's Components table and from `project_structure.txt`.
+`touch_get_indev()`, `display_get_panel()`, `lvgl_port_is_paused()` are also
+currently uncalled but were deliberately KEPT as driver API surface — leave
+them.
+
+### 5. Add OTA partitions while the fleet is size zero
+
+16 MB flash, but `partitions.csv` has a single 4 MB factory app and a ~12 MB
+SPIFFS partition nothing mounts. Re-partitioning fielded devices later means
+reflashing over USB, so lay out `otadata` + `ota_0` + `ota_1` now (two 4 MB
+slots fit easily; shrink or drop the unused SPIFFS partition). Note
+`CONFIG_ESP_WIFI_NVS_ENABLED=n` and the flash-write-while-panel-running
+constraint (README "Memory notes"): an OTA *write* happens while the panel is
+scanning, which will fault the bounce-buffer ISR. Any future OTA
+implementation must pause/stop the panel during the download+write, or
+restart into a dedicated updater mode. Adding the partition layout now is
+safe; implementing OTA download is a separate, harder task.
+
+### 6. Small cleanups (batch into one commit)
+
+- `ambient_weather.c` truncation path: the "Response exceeds N bytes" warning
+  logs once per remaining chunk and then JSON parse fails anyway. Log once
+  (set a flag) and consider aborting the transfer.
+- Weather task stack is 6144 bytes with the TLS handshake running in-task.
+  Works (mbedTLS heap goes to PSRAM via `CONFIG_MBEDTLS_EXTERNAL_MEM_ALLOC`)
+  but tight — log `uxTaskGetStackHighWaterMark(NULL)` once after a successful
+  fetch and right-size before more features land in that task.
+- `backlight_set()` is only ever called once with 255. A night-dim schedule
+  or touch-to-wake is a natural small feature; note the STC8H1K28 shares the
+  I2C bus with the GT911, so calls must stay in task context (never ISRs) and
+  brightness writes contend with touch polls on the same bus.
+
+## Questions already asked and answered (don't re-ask the user)
+
+- **"Should the bugs be fixed or just reported?"** — Answered: fix and
+  commit. That happened in `dd2d428`. The items in this file were explicitly
+  left for later; the user has seen this list and asked for it to be recorded
+  here. Proceeding with these tasks does not need re-confirmation, but
+  hardware-verification steps (task 1) need a human with the board.
+- **"Is the bounce buffer supposed to be enabled?"** — Yes. The README
+  previously contradicted the code; fixed in `dd2d428`. It is enabled and
+  load-bearing.
+- **"Is the board N4R8 or N16R8?"** — N16R8 (16 MB flash / 8 MB octal
+  PSRAM). The old sdkconfig header comment saying N4R8 was stale; fixed.
+- **"Why is WiFi brought up before the display?"** — Deliberate and
+  required; see README "Gotchas". Never reorder.
+- **"Why is rendering paused during fetches?"** — Deliberate mitigation for
+  PSRAM bus contention (see task 1). Improve it, don't just remove it.

main/drivers/display/display_rgb.c

@@ -87,14 +87,6 @@ static const char *TAG = "display_rgb";
 static esp_lcd_panel_handle_t panel = NULL;
 static lv_display_t *disp = NULL;
 
-/*
- * Partial-mode flush: blit the dirty rectangle from a small internal-RAM
- * draw buffer into the single PSRAM framebuffer.
- *
- * Page-flipping two framebuffers instead would be cheaper, but the swap
- * lands mid-scanout on this panel and tears visibly. Copying into one
- * framebuffer that the LCD scans continuously is what the vendor driver does.
- */
 /*
  * Double-buffered flush.
  *

main/main.c

@@ -1,3 +1,4 @@
+#include <math.h>
 #include <stdio.h>
 #include <string.h>
 
@@ -54,7 +55,9 @@ static void weather_update_task(void *arg)
             snprintf(detail, sizeof(detail), "%.0f%% humidity",
                      weather.has_humidity ? weather.humidity : 0.0f);
 
-            ui_update_weather((int16_t)(weather.temp_f + 0.5f),
+            /* lroundf, not +0.5f-and-truncate: the latter rounds toward zero
+             * below 0°F, turning -10.6 into -10. */
+            ui_update_weather((int16_t)lroundf(weather.temp_f),
                               weather.has_humidity ? detail : "Outdoor");
 
             wifi_get_status(line, sizeof(line));
@@ -83,7 +86,7 @@ void app_main(void)
      */
     const esp_err_t wifi_err = wifi_init_sta();
     if (wifi_err != ESP_OK) {
-        ESP_LOGE(TAG, "WiFi failed -- continuing, weather will be disabled");
+        ESP_LOGE(TAG, "WiFi not up yet -- weather starts when it connects");
     }
 
     /* LVGL core must exist before the display and touch drivers register
@@ -114,14 +117,21 @@ void app_main(void)
     vTaskDelay(pdMS_TO_TICKS(100));
     ESP_ERROR_CHECK(backlight_set(255));
 
-    if (wifi_err != ESP_OK) {
-        ui_set_status("WiFi: failed");
-        return;
-    }
-
+    if (wifi_err == ESP_OK) {
         char status[96];
         wifi_get_status(status, sizeof(status));
         ui_set_status(status);
+    } else if (wifi_err == ESP_FAIL || wifi_err == ESP_ERR_TIMEOUT) {
+        /* The event handler keeps retrying in the background; the weather
+         * task polls wifi_is_connected(), so start it anyway rather than
+         * permanently disabling weather because boot raced a slow AP. */
+        ui_set_status("WiFi: connecting...");
+    } else {
+        /* No SSID configured (or setup failed outright) -- nothing will ever
+         * connect, so there is no point starting the weather task. */
+        ui_set_status("WiFi: not configured");
+        return;
+    }
 
     if (weather_init() != ESP_OK) {
         ui_set_status("Weather: no API credentials");

main/wifi/wifi_manager.c

@@ -4,6 +4,7 @@
 #include "esp_log.h"
 #include "esp_netif.h"
 #include "esp_system.h"
+#include "esp_timer.h"
 #include "esp_wifi.h"
 #include "freertos/FreeRTOS.h"
 #include "freertos/event_groups.h"
@@ -18,10 +19,20 @@ static const char *TAG = "wifi_manager";
 #define WIFI_FAIL_BIT      BIT1
 #define WIFI_MAX_RETRIES   8
 #define WIFI_CONNECT_TIMEOUT_MS 30000
+/* After the fast retries are spent, keep trying at this interval forever --
+ * this is a wall appliance, so an AP outage must not require a power cycle. */
+#define WIFI_RECONNECT_DELAY_MS 30000
 
 static EventGroupHandle_t s_wifi_event_group = NULL;
+static esp_timer_handle_t s_reconnect_timer = NULL;
 static int s_retry_count = 0;
 
+static void reconnect_timer_cb(void *arg)
+{
+    ESP_LOGI(TAG, "Slow reconnect attempt");
+    esp_wifi_connect();
+}
+
 static void wifi_event_handler(void *arg, esp_event_base_t event_base,
                                int32_t event_id, void *event_data)
 {
@@ -31,18 +42,28 @@ static void wifi_event_handler(void *arg, esp_event_base_t event_base,
     }
 
     if (event_base == WIFI_EVENT && event_id == WIFI_EVENT_STA_DISCONNECTED) {
+        /* Without this, wifi_is_connected() keeps reporting true after a
+         * drop and callers never notice the link is gone. */
+        xEventGroupClearBits(s_wifi_event_group, WIFI_CONNECTED_BIT);
+
         /*
-         * Only give up after a bounded number of retries. Signalling failure
-         * on the first disconnect (as this previously did) meant a single
-         * transient drop was reported as a hard failure.
+         * A bounded burst of immediate retries covers transient drops;
+         * signalling failure on the first disconnect (as this previously did)
+         * meant a single blip was reported as a hard failure. Once the burst
+         * is spent, fall back to a slow periodic retry instead of giving up:
+         * FAIL_BIT only unblocks the boot-time wait in wifi_init_sta().
          */
         if (s_retry_count < WIFI_MAX_RETRIES) {
             s_retry_count++;
             ESP_LOGW(TAG, "Disconnected, retry %d/%d", s_retry_count, WIFI_MAX_RETRIES);
             esp_wifi_connect();
         } else {
-            ESP_LOGE(TAG, "Giving up after %d retries", WIFI_MAX_RETRIES);
+            ESP_LOGW(TAG, "Still down after %d fast retries; retrying every %d s",
+                     WIFI_MAX_RETRIES, WIFI_RECONNECT_DELAY_MS / 1000);
             xEventGroupSetBits(s_wifi_event_group, WIFI_FAIL_BIT);
+            esp_timer_stop(s_reconnect_timer);
+            esp_timer_start_once(s_reconnect_timer,
+                                 (uint64_t)WIFI_RECONNECT_DELAY_MS * 1000);
         }
         return;
     }
@@ -51,6 +72,7 @@ static void wifi_event_handler(void *arg, esp_event_base_t event_base,
         const ip_event_got_ip_t *event = (const ip_event_got_ip_t *)event_data;
         ESP_LOGI(TAG, "Got IP: " IPSTR, IP2STR(&event->ip_info.ip));
         s_retry_count = 0;
+        xEventGroupClearBits(s_wifi_event_group, WIFI_FAIL_BIT);
         xEventGroupSetBits(s_wifi_event_group, WIFI_CONNECTED_BIT);
     }
 }
@@ -67,6 +89,12 @@ esp_err_t wifi_init_sta(void)
         return ESP_ERR_NO_MEM;
     }
 
+    const esp_timer_create_args_t timer_args = {
+        .callback = reconnect_timer_cb,
+        .name = "wifi_reconnect",
+    };
+    ESP_ERROR_CHECK(esp_timer_create(&timer_args, &s_reconnect_timer));
+
     esp_err_t ret = nvs_flash_init();
     if (ret == ESP_ERR_NVS_NO_FREE_PAGES || ret == ESP_ERR_NVS_NEW_VERSION_FOUND) {
         ESP_ERROR_CHECK(nvs_flash_erase());

sdkconfig.defaults

@@ -1,5 +1,5 @@
-# Elecrow CrowPanel 7.0" HMI -- ESP32-S3-WROOM-1-N4R8
-# 4MB flash, 8MB octal PSRAM, 800x480 RGB panel.
+# Elecrow CrowPanel 7.0" Advance HMI -- ESP32-S3-WROOM-1-N16R8
+# 16MB flash, 8MB octal PSRAM, 800x480 RGB panel.
 
 CONFIG_IDF_TARGET="esp32s3"