refactor(#265): E4 band 4 -- the screens leave src.ino; hardware verification before merge

docs/god-object-inventory.md

@@ -471,6 +471,44 @@ src.ino   5,581 -> 4,387 lines            arduino-cli SUCCESS   58 tests pass
 
 **Band 3 in total** (#270–#274, five PRs): 99 functions and 122 variables left `src.ino` in 14 units (fram, settings, rtc, gps, imu, env, battery, weather, geocache, oled, display, ui_state, touch, web), plus `fc_version.h`; `src.ino` went from 7,986 lines before 3a to 4,387; Flash from 1,856,026 to 1,856,690 (+664, every byte traced: seven static initialisers, function relaxation and literal pools, string-literal splitting, alignment); RAM from 103,232 to 103,248 (+16, 3a's `.bss` packing). Every extraction was proven behaviour-neutral by symbol set and, from 3c on, by every moved function's reference set. Nothing was flashed: hardware verification of the whole sequence is Epic #212's integration test.
 
+## 15. Band 4 — the screens; the god object is a residue
+
+**One branch, three proven commits, one PR left open.** #265's acceptance says *full hardware verification, every screen and every peripheral, before merge*, and nothing in E4 has been flashed. So band 4 is one branch on #265, `fc/212-e4-screens`, with a proven commit per unit and one PR that waits for the owner's hardware verification. It is not merged under the grant.
+
+**Composition, from the graph.** Clustering the 78 remaining functions by the static state they share gave one 34-function component of 163 statics — the LVGL widgets — with the shared statics few and explicable: each screen's screen object and header (navigation's `mainScreens` table and `updateSDIndicators` need them), the touch statistics, the settings scroll areas. Three commits:
+
+| commit | unit(s) | functions | what it owns |
+|---|---|---|---|
+| 1 | `lvgl_port` | the five LVGL callbacks, `initFCTheme` | the display, input-device and group objects, the draw-buffer pointers, the touch statistics (published), the seven named styles (private: only the theme touches them) |
+| 2 | `oled_screens` | `updateOLED`, five `drawOLED*` | nothing of its own; renders published state |
+| 3 | `screen_compass`, `screen_env`, `screen_telemetry`, `screen_geocache`, `screen_settings`, `navigation` | 58 | every widget of every screen (152 statics), each screen's object and header published; `mainScreens`, the list picker |
+
+Commit 3 is six units in one step because the screens and the navigation reference each other through their headers — a screen registers `screenGestureCb` and builds its header with `fcHeaderCreate`, navigation loads the screens — so any one of them alone would have reached back into `src.ino`. `initLVGL` stays: it builds every screen and wires the port, which is `setup()`'s kind of work, the composition root.
+
+**What is left** — `src/src.ino`, 750 lines, measured: 8 functions (`setup`, `loop`, `initLVGL`, `initSD`, `scanI2C`, `handleButtons` and its two helpers), 6 file-scope variables (three button-debounce values, three timers), **cut set 0, largest cluster 1**; the graph reports 7 of 8 functions touching no shared state of the file and the eighth (`handleButtons`) touching only its own three. #265 estimated "roughly 567 lines"; 750 is the number. 28 headers and 25 translation units hold the rest.
+
+**Found by the build, seven things, all recorded in the commits:** twelve static callbacks referenced before their definitions (a builder registers a callback defined after it — `src.ino` declared them at file scope; the harness now forward-declares every static function at the top of its unit and drops the file-scope prototypes); `screenGestureCb` published (every screen registers it); six statics the graph could not see (two write-only headers, two const-folded counts, an inlined colour helper, the settings menu buttons array) moved by hand and marked; `web.h` restoring the `fs::FS` alias `WebServer.h` relies on — TFT_eSPI defines `FS_NO_GLOBALS`, and `src.ino`'s include order used to hide it; the `LV_USE_LOG` guard around `lvglLogCb`, which the harness does not carry, restored by hand; `lvglTickCb` losing `static` because `initLVGL` registers it (`--publish-functions`); the button pins into `fc_config.h`.
+
+**Two harness faults caught before they wrote anything.** The existing-prototype matcher accepted any prefix and matched `case SCREEN_COMPASS: updateCompassData(); …; break;` inside `updateDisplay` — it would have deleted the switch case as a redundant prototype. The dry run showed it; it now requires type words at column 0 and nothing after the semicolon. And the baseline for band 3e (§14) had raced the extraction; band 4 captured every baseline before touching the tree.
+
+**Measured**, per commit — `scripts/verify_extraction.py`, PASS each time:
+
+```
+unit 1  lvgl_port      592 -> 592 symbols, 27 relocated, 13 linkage changes declared (12 published
+                       variables, the tick callback), 5 deltas net +2 all layout, 5 moved checked
+                       Flash 1,856,690 -> 1,856,670 (-20)         src.ino 4,387 -> 4,209
+unit 2  oled_screens   592 -> 592, 6 relocated, 1 delta (-1), 6 moved checked
+                       .flash.text +60 (six calls now cross-TU)   Flash -> 1,856,722 (+52)   src.ino -> 3,993
+unit 3  screens + nav  592 -> 593 (a constprop clone of an LVGL inline, tolerated by pattern),
+                       213 relocated, 12 linkage changes declared, 6 deltas net +104 all attributed,
+                       61 moved functions checked by reference set (27 references now cross-TU)
+                       .flash.text +184 (inputs +182), .flash.rodata -96
+                       Flash -> 1,856,810 (+88)                   src.ino -> 804, then 750 after comments
+RAM     103,248 throughout                                arduino-cli SUCCESS   58 tests pass
+```
+
+**E4 in total.** `src/src.ino` went from 8,742 lines (band 1's start) to 750 — 70 functions and about 170 statics left in band 4, 99 functions and 122 variables in band 3, 33 functions in bands 1 and 2 — into 25 translation units and 28 headers. Flash from 1,855,962 to 1,856,810 (+848 bytes, 0.05%, every byte traced to static initialisers, cross-TU calls, literal pools, string splitting and alignment); RAM from 103,232 to 103,248 (+16). Every move was proven behaviour-neutral by symbol set and, from band 3c on, by every moved function's reference set. Nothing was flashed: hardware verification is #265's gate and Epic #212's integration test.
+
 ---
 
 ## Appendix — the full tables

scripts/extract_unit.py

@@ -129,7 +129,7 @@ def is_static(lines: list[str], start: int) -> bool:
 # in `*`. The name follows, optionally an array suffix, then `=`, `;` or `,`.
 # Column 0 only: file-scope declarations are, and an indented
 # `else gprmcFixThisCycle = true;` reads as a declaration otherwise (band 3b).
-VAR_RE_TMPL = r"^(?:static\s+)?(?:volatile\s+)?(?:const\s+)?(?:[A-Za-z_][\w:<>]*\*?\s+)+\**{name}\s*(?:\[[^\]]*\])*\s*(?:=|;|,|\()"
+VAR_RE_TMPL = r"^(?:static\s+)?(?:volatile\s+)?(?:const\s+)?(?:[A-Za-z_][\w:<>]*\**\s+)+\**{name}\s*(?:\[[^\]]*\])*\s*(?:=|;|,|\()"
 # the trailing `(` is a constructor-call declaration, `WebServer webServer(WEB_SERVER_PORT);`
 # (band 3e); a prototype never collides because variables are looked up by name
 
@@ -251,6 +251,8 @@ def main() -> int:
     ap = argparse.ArgumentParser()
     ap.add_argument("--unit", required=True)
     ap.add_argument("--functions", default="")
+    ap.add_argument("--publish-functions", default="",
+                    help="moved functions that lose `static` and gain a prototype: a caller stays behind")
     ap.add_argument("--move-vars", default="", help="variables moved into the .cpp, kept private")
     ap.add_argument("--publish-vars", default="", help="variables moved into the .cpp and declared extern in the header")
     ap.add_argument("--move-defines", default="")
@@ -273,6 +275,8 @@ def main() -> int:
 
     split = lambda s: [n for n in s.split(",") if n]
     names, mvars, pvars = split(a.functions), split(a.move_vars), split(a.publish_vars)
+    pfuncs = split(a.publish_functions)
+    names += [n for n in pfuncs if n not in names]
     defines, decls = split(a.move_defines), split(a.move_decls)
 
     # ---- locate everything first; only then mutate --------------------------
@@ -281,7 +285,7 @@ def main() -> int:
         d = find_definition(lines, n)
         b = signature_end(lines, d)
         e = block_end(lines, b)
-        moves.append((comment_start(lines, d), d, b, e, n, is_static(lines, d)))
+        moves.append((comment_start(lines, d), d, b, e, n, is_static(lines, d) and n not in pfuncs))
     moves.sort()
     for x, y in zip(moves, moves[1:]):
         if y[0] <= x[3]:
@@ -316,9 +320,13 @@ def main() -> int:
     # existing standalone prototypes of moved functions: reuse text, drop line
     protos = {}   # line index -> (name, text)
     for c, d, b, e, n, st in moves:
-        if st:
-            continue
-        pat = re.compile(r"^\s*(?:static\s+)?[A-Za-z_][\w\s\*&:<>,]*\s\**" + re.escape(n) + r"\s*\([^;{]*\)\s*;")
+        # static functions included: their file-scope prototypes leave src.ino
+        # too, and the unit declares them up front (see the cpp emission)
+        # a prototype: type words at column 0, the name, a parameter list, `;` and
+        # nothing else. The first version accepted any prefix and matched
+        # `case SCREEN_COMPASS: updateCompassData(); ...; break;` (band 4)
+        pat = re.compile(r"^(?:static\s+)?(?:[A-Za-z_][\w:<>]*[\s*&]+)+" + re.escape(n)
+                         + r"\s*\([^;{]*\)\s*;\s*(?://.*)?$")
         for i, l in enumerate(lines):
             if i != d and pat.match(l):
                 protos[i] = (n, re.sub(r"\s*//.*$", "", l.rstrip("\r\n")).strip())
@@ -374,7 +382,10 @@ def inc_line(inc: str) -> str:
         existing = {n: t for _, (n, t) in protos.items()}
         for c, d, b, e, n, st in moves:
             if not st:
-                h.append(existing.get(n, prototype(lines, d, b)) + nl)
+                p = existing.get(n, prototype(lines, d, b))
+                if n in pfuncs:
+                    p = re.sub(r"^static\s+", "", p)
+                h.append(p + nl)
 
     cpp = []
     if not a.header_only:
@@ -396,10 +407,18 @@ def inc_line(inc: str) -> str:
                 l = lines[i]
                 cpp.append(re.sub(r"^(\s*)static\s+", r"\1", l) if n in pvars else l)
                 cpp.extend(lines[i + 1:var_ends[n] + 1])
+        statics = [prototype(lines, d, b) for c, d, b, e, n, st in moves if st]
+        if statics:
+            # a builder registers callbacks defined after it (band 4): declare
+            # every static function of the unit up front, as src.ino did at file scope
+            cpp.append(nl)
+            cpp.extend(p + nl for p in statics)
         for c, d, b, e, n, st in moves:
             cpp.append(nl)
             for i in range(c, e + 1):
                 l = lines[i]
+                if i == d and n in pfuncs:
+                    l = re.sub(r"^static\s+", "", l)     # published: a caller stays behind
                 if d <= i <= b and not st:
                     # a default argument may appear once: the header's prototype
                     # carries it, so the definition must not (parseGPXFromString)

scripts/verify_extraction.py

@@ -51,6 +51,7 @@
 import importlib.util
 import json
 import pathlib
+import re
 import subprocess
 import sys
 
@@ -144,9 +145,14 @@ def main() -> int:
     def dm(s: str) -> str:
         return names.get(s, s)
 
+    CLONE = re.compile(r"\$(?:constprop|isra|part)\$\d+$")
+
     def norm(s: str) -> str:
-        """Demangled name, collapsed to the base name for re-signed functions."""
-        d = dm(s)
+        """Demangled name, collapsed to the base name for re-signed functions,
+        and to the original function for a compiler clone: a smaller TU makes
+        GCC specialise a library inline for one caller (band 4), which is the
+        same reference with a different name."""
+        d = CLONE.sub("", dm(s))
         b = base(d)
         return b if b in resigned else d
 
@@ -186,12 +192,18 @@ def norm(s: str) -> str:
         ok = ok and base(r) in resigned
     print(f"   removed: {removed or 'none'}")
     for k in removed:
+        if CLONE.search(k):
+            print(f"      tolerated: {k} (a compiler clone of a library inline, no longer worth making)")
+            continue
         print(f"      UNEXPECTED removal: {k}")
         ok = False
     print(f"   added:   {added or 'none'}")
     for k in added:
         if k.startswith("_GLOBAL__sub_I"):
             print(f"      tolerated: {k} (static initialiser of the new TU)")
+        elif CLONE.search(k):
+            print(f"      tolerated: {k} (a compiler clone of a library inline: the smaller TU "
+                  f"specialised it for one caller; reference sets compare it as the original)")
         elif base(k) in published:
             # a static nobody read was eliminated by the compiler; published, it
             # exists (band 3b: write-only gnrmcFixThisCycle). Report it: dead state.

src/fc_config.h

@@ -8,3 +8,8 @@
 #define FRAM_CS   15  // A3 -> FRAM CS
 #define TFT_BL     8  // A5 -> LED pin on display module (PWM-dimmable backlight)
 #define CTP_INT   14  // A4 -> Touch interrupt (active-low, CHANGE: fires on touch + release)
+
+// Button pins (directly wired, active LOW)
+#define BUTTON_A 9
+#define BUTTON_B 6
+#define BUTTON_C 5

src/lvgl_port.cpp

@@ -0,0 +1,164 @@
+// lvgl_port.cpp -- extracted from src.ino by scripts/extract_unit.py (E4).
+#include "lvgl_port.h"
+#include "fc_config.h"
+#include "display.h"
+#include "touch.h"
+#include "ui_state.h"
+#include "fc_theme.h"
+#include "logging.h"
+
+lv_display_t* lvglDisplay = NULL;
+uint8_t* lvglBuf1 = NULL;
+uint8_t* lvglBuf2 = NULL;
+bool lvglAvailable = false;
+lv_indev_t* lvglTouchIndev = NULL;   // FT6336U → LV_INDEV_TYPE_POINTER
+lv_indev_t* lvglEncoderIndev = NULL; // Buttons → LV_INDEV_TYPE_ENCODER
+lv_group_t* lvglGroup = NULL;        // Focus group for encoder navigation
+static lv_style_t fcStyleHeader;   // Cyan, XL (24) — screen titles
+static lv_style_t fcStyleValue;    // Green, LG (20) — sensor values
+static lv_style_t fcStyleHero;     // Green, HERO (32) — large numbers
+static lv_style_t fcStyleBody;     // White, MD (18) — body text
+static lv_style_t fcStyleLabel;    // Gray, SM (16) — secondary labels
+static lv_style_t fcStyleWarn;     // Orange, MD (18) — warnings
+static lv_style_t fcStyleError;    // Red, MD (18) — errors
+uint32_t touchPressCount = 0;
+uint32_t touchReleaseCount = 0;
+bool     touchWasPressed = false;
+int32_t  lastTouchX = -1;          // Last LVGL-space touch X (for diagnostics)
+int32_t  lastTouchY = -1;          // Last LVGL-space touch Y (for diagnostics)
+
+// LVGL needs a tick source to track elapsed time for animations/timers.
+// On ESP32-S3, esp_timer_get_time() returns microseconds since boot.
+uint32_t lvglTickCb(void) {
+  return (uint32_t)(esp_timer_get_time() / 1000ULL);  // Convert µs to ms
+}
+
+// Called by LVGL when a rendered region is ready to be sent to the display.
+// Uses TFT_eSPI's SPI transaction-safe pushColors with byte swap.
+void lvglFlushCb(lv_display_t* disp, const lv_area_t* area, uint8_t* px_map) {
+  uint32_t w = (area->x2 - area->x1 + 1);
+  uint32_t h = (area->y2 - area->y1 + 1);
+
+  tft.startWrite();
+  tft.setAddrWindow(area->x1, area->y1, w, h);
+  // swap=true: byte-swaps from LVGL native little-endian to ST7796U big-endian
+  tft.pushColors((uint16_t*)px_map, w * h, true);
+  tft.endWrite();
+
+  lv_display_flush_ready(disp);
+}
+
+#if LV_USE_LOG != 0   // as in src.ino: compiled only when LVGL logging is on (initLVGL registers it under the same guard)
+void lvglLogCb(lv_log_level_t level, const char* buf) {
+  LV_UNUSED(level);
+  Serial.println(buf);
+  Serial.flush();
+}
+#endif
+
+void lvglTouchReadCb(lv_indev_t* indev, lv_indev_data_t* data) {
+  (void)indev;
+
+  if (!touchAvailable) {
+    data->state = LV_INDEV_STATE_RELEASED;
+    return;
+  }
+
+  if (ctp.touched()) {
+    TS_Point p = ctp.getPoint();
+    // Same coordinate transform as legacy pipeline:
+    data->point.x = (int32_t)(480 - p.y);  // horizontal 0-479
+    data->point.y = (int32_t)(p.x);        // vertical   0-319
+    data->state   = LV_INDEV_STATE_PRESSED;
+    lastTouchX = data->point.x;
+    lastTouchY = data->point.y;
+    lastActivityTime = millis();  // DIAG: keep TFT awake on LVGL screens too
+    if (!touchWasPressed) {
+      touchPressCount++;
+      touchWasPressed = true;
+      logPrintf("[TOUCH] PRESS @(%ld,%ld) scr=%d sub=%d\n",
+                data->point.x, data->point.y, currentScreen, settingsSubScreen);
+    }
+  } else {
+    data->state = LV_INDEV_STATE_RELEASED;
+    if (touchWasPressed) {
+      touchReleaseCount++;
+      touchWasPressed = false;
+      // DIAG: log what LVGL thinks is pressed at touch coordinates
+      lv_obj_t* hit = lv_indev_search_obj(lv_screen_active(), &data->point);
+      logPrintf("[TOUCH] RELEASE #%lu → hit_obj=%p (scr=%d sub=%d)\n",
+                touchPressCount, (void*)hit, currentScreen, settingsSubScreen);
+    }
+  }
+}
+
+// Maps A/B/C buttons to LVGL encoder: A=prev(-1), B=next(+1), C=enter.
+// Edge detection emits a single enc_diff pulse per press.
+void lvglEncoderReadCb(lv_indev_t* indev, lv_indev_data_t* data) {
+  (void)indev;
+
+  static bool prevA = false, prevB = false, prevC = false;
+
+  bool curA = !digitalRead(BUTTON_A);  // active LOW
+  bool curB = !digitalRead(BUTTON_B);
+  bool curC = !digitalRead(BUTTON_C);
+
+  int16_t diff = 0;
+
+  // Edge detect: fire once on press-down
+  if (curA && !prevA) diff = -1;   // A = previous
+  if (curB && !prevB) diff = +1;   // B = next
+
+  data->enc_diff = diff;
+  data->state = curC ? LV_INDEV_STATE_PRESSED : LV_INDEV_STATE_RELEASED;
+
+  prevA = curA;
+  prevB = curB;
+  prevC = curC;
+}
+
+void initFCTheme() {
+  // Initialize default dark theme with cyan primary, green secondary
+  lv_theme_t* theme = lv_theme_default_init(
+      lvglDisplay,
+      FC_COLOR_HEADER,    // primary — focus rings, active elements
+      FC_COLOR_VALUE,     // secondary — accents, toggles
+      true,               // dark mode
+      FC_FONT_MD          // default app font = 18px Montserrat
+  );
+  lv_display_set_theme(lvglDisplay, theme);
+
+  // Screen background: black
+  lv_obj_set_style_bg_color(lv_screen_active(), FC_COLOR_BG, 0);
+
+  // Initialize named styles
+  lv_style_init(&fcStyleHeader);
+  lv_style_set_text_color(&fcStyleHeader, FC_COLOR_HEADER);
+  lv_style_set_text_font(&fcStyleHeader, FC_FONT_XL);
+
+  lv_style_init(&fcStyleValue);
+  lv_style_set_text_color(&fcStyleValue, FC_COLOR_VALUE);
+  lv_style_set_text_font(&fcStyleValue, FC_FONT_LG);
+
+  lv_style_init(&fcStyleHero);
+  lv_style_set_text_color(&fcStyleHero, FC_COLOR_VALUE);
+  lv_style_set_text_font(&fcStyleHero, FC_FONT_HERO);
+
+  lv_style_init(&fcStyleBody);
+  lv_style_set_text_color(&fcStyleBody, FC_COLOR_TEXT);
+  lv_style_set_text_font(&fcStyleBody, FC_FONT_MD);
+
+  lv_style_init(&fcStyleLabel);
+  lv_style_set_text_color(&fcStyleLabel, FC_COLOR_DIM);
+  lv_style_set_text_font(&fcStyleLabel, FC_FONT_SM);
+
+  lv_style_init(&fcStyleWarn);
+  lv_style_set_text_color(&fcStyleWarn, FC_COLOR_WARN);
+  lv_style_set_text_font(&fcStyleWarn, FC_FONT_MD);
+
+  lv_style_init(&fcStyleError);
+  lv_style_set_text_color(&fcStyleError, FC_COLOR_ERROR);
+  lv_style_set_text_font(&fcStyleError, FC_FONT_MD);
+
+  logPrintln("[LVGL] Field Compass theme initialized (7 styles)");
+}

src/lvgl_port.h

@@ -0,0 +1,26 @@
+#pragma once
+// lvgl_port.h -- extracted from src.ino by scripts/extract_unit.py (E4).
+#include <lvgl.h>
+#include <esp_timer.h>
+
+#define LVGL_TEST_MODE 0
+
+extern lv_display_t* lvglDisplay;
+extern uint8_t* lvglBuf1;
+extern uint8_t* lvglBuf2;
+extern bool lvglAvailable;
+extern lv_indev_t* lvglTouchIndev;
+extern lv_indev_t* lvglEncoderIndev;
+extern lv_group_t* lvglGroup;
+extern int32_t  lastTouchX;
+extern int32_t  lastTouchY;
+extern uint32_t touchPressCount;
+extern uint32_t touchReleaseCount;
+extern bool     touchWasPressed;
+
+uint32_t lvglTickCb(void);
+void lvglFlushCb(lv_display_t* disp, const lv_area_t* area, uint8_t* px_map);
+void lvglLogCb(lv_log_level_t level, const char* buf);
+void lvglTouchReadCb(lv_indev_t* indev, lv_indev_data_t* data);
+void lvglEncoderReadCb(lv_indev_t* indev, lv_indev_data_t* data);
+void initFCTheme();