refactor(#271): E4 band 3b -- gps, imu, env and battery leave src.ino as units

docs/god-object-inventory.md

@@ -321,6 +321,50 @@ Flash     1,856,026 -> 1,856,146 (+120)   RAM 103,232 -> 103,248 (+16)
 src.ino   7,986 -> 7,483 lines           arduino-cli SUCCESS   58 tests pass
 ```
 
+## 11. Band 3b — gps, imu, env and battery; the graph could not see read-only data
+
+**What moved** — the four sensor domains, one unit each:
+
+| unit | owns | functions |
+|---|---|---|
+| `gps` | `gpsData`, the NMEA buffer, the twelve timing/debug flags, `rtcSyncedFromGPS`; `GpsData`; the GPS serial and debug defines, `NMEA_MAX_FIELDS` | `initGPS`, `readGPS`, `nmeaParse`, `parseNMEA`, `getGpsAccuracyMeters` |
+| `imu` | `imuData`, `imuAvailable`, `magAvailable`, `lis`, `lsm`, the twelve magnetometer calibration variables; `ImuData`; `MAG_CAL_DURATION_MS`, `MAG_MIN_MAGNITUDE` | `initIMU`, `readIMU`, `loadMagCal`, `saveMagCal`, `magLogPrintln` |
+| `env` | `envData`, `envSensor`, `bmeAvailable`, `sht4`, `shtData`, `shtAvailable`, the four BSEC state variables, the BSEC config blob; `EnvData`, `ShtData`; the BSEC defines | `initBME688`, `initSHT41`, `bsecDataCallback`, `readBME688`, `readSHT41`, `getIaqAccuracyText`, `getIaqQualityText`, `hPaToInHg`, `loadBsecState`, `saveBsecState`, `saveBsecToFRAM`, `loadBsecFromFRAM` |
+| `battery` | `battery`, `batteryAvailable`; `BATT_LOG_INTERVAL`, `BATT_LOG_FILE` | `initBattery`, `logBatteryToSD`, `logBatteryToFRAM`, `isBatteryConnected` |
+
+Ten cut-set variables left `src.ino` (7,483 → 6,598 lines): `gpsData`, `imuData`, `imuAvailable`, `magAvailable`, `envData`, `bmeAvailable`, `shtData`, `shtAvailable`, `battery`, `batteryAvailable`. The E4-1 structs went to the unit headers, without their instances.
+
+**Found by the build, not by the graph** — four things, two of which correct the diagnosis:
+
+1. **The relocation graph never counted read-only data.** `reference-graph.py` took `nm` classes B/b/D/d as variables and skipped R/r, so every `const` table — `tzPresets`, the timeout tables, the 1,943-byte BSEC config blob `bsec2_config` — was invisible to the fan-in tables, the cut set and the reference sets the band-2 and 3a proofs compared. It surfaced when `initBME688` failed to compile without the blob. Fixed: R/r count, vtables and typeinfo do not. On the pre-3b object the fixed graph reports 79 external-linkage variables (read-only ones now included) and the same 22-variable cut set (34 after band 2, less 3a's 12), so no ordering decision changes. This band's proof was re-run against a rebuilt pre-move baseline with the fixed graph, so it covers const references.
+2. **`gnrmcFixThisCycle` is write-only.** Set in `parseNMEA` and cleared by `handleWebGpsReset`, read by nothing since #115. As a `static` the compiler eliminated it — absent from `nm`, so the graph listed no users and the first plan kept it private; the build found the write in the web handler. It is published now, at the cost of one byte and its stores (`parseNMEA` +20). **Dead state; deleting it is the owner's call**, and the verifier now names this case.
+3. `NMEA_MAX_FIELDS` sat between `nmeaParse`'s doc comment and its body, so the comment-scan stopped at it; carried into `gps.h`, the stranded comment and two empty section banners removed by hand.
+4. The MAX17048 library header is `Adafruit_MAX1704X.h`; the harness cannot know library header names, the build can.
+
+**Harness, this band:** comma declarator lists (`float a = 0, b = 0, c = 0;` moves whole, one `extern float a, b, c;`, and every name on the line must be asked for); an instance declared with its struct (`struct GpsData {…} gpsData;` — the struct goes to the header, `GpsData gpsData;` to the unit); declarations matched at column 0 only (an indented `else gprmcFixThisCycle = true;` had matched as a declaration); the orphan sweep walks bottom-up so chained comments are caught; the verifier tolerates, and names, a published variable that did not exist before.
+
+**Measured** — `scripts/verify_extraction.py`, PASS:
+
+```
+symbols   585 before -> 589 after, union of 12 objects
+          76 relocated: gps 18, imu 27, env 24, battery 7 (function-local statics and the library
+             destructor inlines go with their functions)
+          11 linkage changes b -> B, all declared (the GPS timing flags, gpsDebugEnabled, the BSEC flags)
+           3 added initialisers: _GLOBAL__sub_I_lsm 53, _envSensor 38, _battery 29 (120);
+             src.ino's shrank 272 -> 166 (-106)
+           1 added variable: gnrmcFixThisCycle (item 2 above)
+          removed: none
+deltas    25 symbols, net -88 (the -106 included): 7 touch published state, 3 call a moved function,
+          3 moved functions, 12 identical references
+          unexplained: none
+sections  .flash.text  +164: our input sections +147 (.text +54, literal pools +76, startup +17), +17 alignment
+          .flash.rodata +96: input +228 (bsec2_config 1,943 bytes relocated unchanged; string
+                             literals now split across four objects), merged by the linker to +96
+          RAM unchanged at 103,248 (.dram0.bss input +1, the flag, absorbed by alignment)
+Flash     1,856,146 -> 1,856,406 (+260)
+src.ino   7,483 -> 6,598 lines           arduino-cli SUCCESS   58 tests pass
+```
+
 ---
 
 ## Appendix — the full tables

scripts/extract_unit.py

@@ -126,22 +126,64 @@ def is_static(lines: list[str], start: int) -> bool:
 
 
 # Type may be several words (`unsigned long`, `const char*`); each word may end
-# in `*`. The name follows, optionally an array suffix, then `=` or `;`.
-VAR_RE_TMPL = r"^\s*(?:static\s+)?(?:volatile\s+)?(?:const\s+)?(?:[A-Za-z_][\w:<>]*\*?\s+)+\**{name}\s*(?:\[[^\]]*\])*\s*(?:=|;)"
-
-
-def find_var(lines: list[str], name: str) -> int:
+# 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*(?:=|;|,)"
+
+
+# A name that is not the first declarator on its line: `float a = 0, b = 0, c;`
+# (band 3b: the magnetometer offsets). Column 0 only, and never a prototype.
+COMMA_RE_TMPL = r"^(?:static\s+)?(?:volatile\s+)?(?:const\s+)?(?:[A-Za-z_][\w:<>]*\*?\s+)+[^;{{()]*?,\s*\**{name}\s*(?:\[[^\]]*\])*\s*(?:=|;|,)"
+# An instance declared with its struct: `struct GpsData { ... } gpsData;`
+INST_RE_TMPL = r"^\}}\s*{name}\s*;"
+STRUCT_START_RE = re.compile(r"^(?:typedef\s+)?struct\s+(\w+)\s*\{")
+
+
+def declarators(line: str) -> list[str]:
+    """Names declared on one line: `static float a = 0, b = 0, c;` -> [a, b, c]."""
+    body = re.sub(r"//.*$", "", line.rstrip("\r\n")).strip().rstrip(";")
+    body = re.sub(r"^(?:static\s+)?(?:volatile\s+)?(?:const\s+)?(?:[A-Za-z_][\w:<>]*\*?\s+)+", "", body, count=1)
+    out = []
+    for part in body.split(","):
+        m = re.match(r"\s*\**(\w+)", part)
+        if m:
+            out.append(m.group(1))
+    return out
+
+
+def locate_var(lines: list[str], name: str) -> tuple[int, int, str | None]:
+    """(first line, last line, struct type or None) of a file-scope variable."""
     pat = re.compile(VAR_RE_TMPL.format(name=re.escape(name)))
-    return one([i for i, l in enumerate(lines)
-                if pat.match(l) and not l.lstrip().startswith("extern ")], f"variable {name}")
+    cpat = re.compile(COMMA_RE_TMPL.format(name=re.escape(name)))
+    hits = [i for i, l in enumerate(lines)
+            if (pat.match(l) or cpat.match(l)) and not l.lstrip().startswith("extern ")]
+    if hits:
+        i = one(hits, f"variable {name}")
+        # a table initialiser (`= {` ... `};`) spans lines: carry the whole block
+        end = block_end(lines, i) if "{" in lines[i] and "}" not in lines[i] else i
+        return i, end, None
+    ipat = re.compile(INST_RE_TMPL.format(name=re.escape(name)))
+    e = one([i for i, l in enumerate(lines) if ipat.match(l)], f"variable {name}")
+    s = e
+    while s >= 0 and not STRUCT_START_RE.match(lines[s]):
+        s -= 1
+    if s < 0:
+        sys.exit(f"refusing: no `struct T {{` above `}} {name};` at line {e + 1}")
+    return s, e, STRUCT_START_RE.match(lines[s]).group(1)
 
 
 def extern_decl(line: str) -> str:
-    """`static volatile uint16_t x = 0;  // c` -> `extern volatile uint16_t x;`"""
+    """`static volatile uint16_t x = 0;  // c` -> `extern volatile uint16_t x;`
+    `float a = 0, b = 0, c = 0;` -> `extern float a, b, c;`"""
     body = line.rstrip("\r\n")
     body = re.sub(r"//.*$", "", body).strip()
     body = re.sub(r"^static\s+", "", body)
-    body = body.split("=", 1)[0].rstrip().rstrip(";").rstrip()
+    if "{" in body:
+        body = body.split("=", 1)[0]
+    else:
+        body = re.sub(r"\s*=\s*[^,;]+", "", body)   # each declarator's initialiser
+    body = body.rstrip().rstrip(";").rstrip()
     return "extern " + body + ";"
 
 
@@ -185,11 +227,16 @@ def main() -> int:
         if y[0] <= x[3]:
             sys.exit(f"refusing: {x[4]} and {y[4]} overlap")
 
-    var_lines = {n: find_var(lines, n) for n in mvars + pvars}
-    # a table initialiser (`static const TZPreset tzPresets[] = {` ... `};`) spans
-    # lines: carry the whole block, not just its first line (band 3a)
-    var_ends = {n: (block_end(lines, i) if "{" in lines[i] and "}" not in lines[i] else i)
-                for n, i in var_lines.items()}
+    var_loc = {n: locate_var(lines, n) for n in mvars + pvars}
+    var_lines = {n: v[0] for n, v in var_loc.items()}
+    var_ends = {n: v[1] for n, v in var_loc.items()}
+    var_inst = {n: v[2] for n, v in var_loc.items() if v[2]}   # name -> struct type
+    # a comma line moves whole, so every declarator on it must have been asked for
+    for n, i in var_lines.items():
+        if n not in var_inst and "{" not in lines[i]:
+            others = [d for d in declarators(lines[i]) if d not in mvars + pvars]
+            if others:
+                sys.exit(f"refusing: line {i + 1} also declares {others}; name them too")
     def_lines = {n: one([i for i, l in enumerate(lines)
                         if re.match(r"^\s*#define\s+" + re.escape(n) + r"\b", l)], f"#define {n}")
                  for n in defines}
@@ -243,14 +290,25 @@ def inc_line(inc: str) -> str:
             s, e = decl_ranges[n]
             h.append(nl)
             h.extend(lines[s:e + 1])
+    for n, t in var_inst.items():
+        # the struct goes to the header without its instance; the .cpp defines `T n;`
+        h.append(nl)
+        h.extend(lines[var_lines[n]:var_ends[n]])
+        h.append("};" + nl)
     if hdr_range:
         # verbatim block goes BEFORE the externs and prototypes: it is types and
         # macros, and an extern of a struct type needs the struct first (band 3a)
         h.append(nl)
         h.extend(lines[hdr_range[0]:hdr_range[1] + 1])
     if pvars:
         h.append(nl)
-        h.extend(extern_decl(lines[var_lines[n]]) + nl for n in pvars)
+        seen = set()
+        for n in pvars:
+            if var_lines[n] in seen:
+                continue                     # a comma line: one extern names them all
+            seen.add(var_lines[n])
+            h.append((f"extern {var_inst[n]} {n};" if n in var_inst
+                      else extern_decl(lines[var_lines[n]])) + nl)
     if not a.header_only and moves:
         h.append(nl)
         existing = {n: t for _, (n, t) in protos.items()}
@@ -266,10 +324,18 @@ def inc_line(inc: str) -> str:
             cpp.append(inc_line(inc))
         if mvars or pvars:
             cpp.append(nl)
+            done = set()
             for n in sorted(mvars + pvars, key=lambda x: var_lines[x]):
-                l = lines[var_lines[n]]
+                i = var_lines[n]
+                if i in done:
+                    continue                 # a comma line carries several names; once
+                done.add(i)
+                if n in var_inst:
+                    cpp.append(f"{var_inst[n]} {n};" + nl)
+                    continue
+                l = lines[i]
                 cpp.append(re.sub(r"^(\s*)static\s+", r"\1", l) if n in pvars else l)
-                cpp.extend(lines[var_lines[n] + 1:var_ends[n] + 1])
+                cpp.extend(lines[i + 1:var_ends[n] + 1])
         for c, d, b, e, n, st in moves:
             cpp.append(nl)
             cpp.extend(lines[c:e + 1])
@@ -281,7 +347,8 @@ def inc_line(inc: str) -> str:
     for n in mvars:
         print(f"  var   {n:<28} line {var_lines[n] + 1} -> .cpp (private)")
     for n in pvars:
-        print(f"  var   {n:<28} line {var_lines[n] + 1} -> .cpp, published: {extern_decl(lines[var_lines[n]])}")
+        ext = f"extern {var_inst[n]} {n};" if n in var_inst else extern_decl(lines[var_lines[n]])
+        print(f"  var   {n:<28} line {var_lines[n] + 1} -> .cpp, published: {ext}")
     for n in defines:
         print(f"  define {n:<27} line {def_lines[n] + 1} -> header")
     for n in decls:
@@ -323,7 +390,7 @@ def is_comment(i: int) -> bool:
         t = lines[i].rstrip("\r\n")
         return t.lstrip().startswith("//") and not BANNER_RE.match(t)
 
-    for i in sorted(remove):
+    for i in sorted(remove, reverse=True):     # bottom-up: a swept comment can orphan the one above it
         if i == 0 or (i - 1) in remove or not is_comment(i - 1):
             continue
         j = i

scripts/reference-graph.py

@@ -85,9 +85,11 @@ def defined_symbols(obj: pathlib.Path):
         cls, name = parts[-2], parts[-1]
         if cls in ("T", "t"):
             funcs.add(name)
-        elif cls in ("B", "b", "D", "d"):
-            if name.startswith("_ZZ"):
-                continue                      # function-local static, not file scope
+        elif cls in ("B", "b", "D", "d", "R", "r"):
+            # R/r is read-only data: const tables (tzPresets, the BSEC config blob)
+            # are variables too, and were invisible until band 3b missed one.
+            if name.startswith(("_ZZ", "_ZTV", "_ZTI", "_ZTS")):
+                continue                      # function-local static, vtable, typeinfo
             (var_static if name.startswith("_ZL") else var_ext).add(name)
     return funcs, var_ext, var_static
 

scripts/verify_extraction.py

@@ -192,6 +192,11 @@ def norm(s: str) -> str:
     for k in added:
         if k.startswith("_GLOBAL__sub_I"):
             print(f"      tolerated: {k} (static initialiser of the new TU)")
+        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.
+            print(f"      tolerated: {k} (declared published; absent before because the compiler "
+                  f"eliminated an unread static -- dead state, worth deleting)")
         else:
             print(f"      UNEXPECTED addition: {k}")
             ok = False

src/battery.cpp

@@ -0,0 +1,100 @@
+// battery.cpp -- extracted from src.ino by scripts/extract_unit.py (E4).
+#include "battery.h"
+#include "fram.h"
+#include "logging.h"
+
+Adafruit_MAX17048 battery;
+bool batteryAvailable = false;
+
+void initBattery() {
+  logPrint("Initializing MAX17048... ");
+
+  if (!battery.begin()) {
+    logPrintln("NOT FOUND");
+    return;
+  }
+
+  batteryAvailable = true;
+  logPrintln("OK");
+}
+
+// Log battery data to SD card for analysis
+void logBatteryToSD() {
+  if (!sdHealth.available || !batteryAvailable) return;
+
+  File f = sdOpenSafe(BATT_LOG_FILE, "a", true);  // silent fail OK
+  if (!f) return;
+
+  // Write header if new file
+  if (f.size() == 0) {
+    f.println("millis,voltage,percent,rate");
+  }
+
+  float v = battery.cellVoltage();
+  float p = battery.cellPercent();
+  float r = battery.chargeRate();
+
+  f.printf("%lu,%.3f,%.2f,%.2f\n", millis(), v, p, r);
+  f.close();
+  recordSDSuccess();
+}
+
+// Write battery entry to FRAM ring buffer instead of SD
+void logBatteryToFRAM() {
+  if (!framAvailable || !batteryAvailable) return;
+
+  float v = battery.cellVoltage();
+  float p = battery.cellPercent();
+  float r = battery.chargeRate();
+
+  FRAMBatteryEntry entry;
+  entry.timestamp = millis();
+  entry.voltage = v;
+  entry.percent = p;
+  entry.rate = r;
+  entry.flags = 0;
+  // Simple XOR checksum over first 18 bytes (before checksum field)
+  uint16_t ck = 0;
+  uint8_t* bytes = (uint8_t*)&entry;
+  for (int i = 0; i < 18; i += 2) {
+    ck ^= (bytes[i] | (bytes[i+1] << 8));
+  }
+  entry.checksum = ck;
+
+  // Write entry to FRAM at head position
+  uint32_t addr = FRAM_BATT_ADDR + (framHeader.battHead * FRAM_BATT_ENTRY);
+  uint8_t* data = (uint8_t*)&entry;
+  for (int i = 0; i < FRAM_BATT_ENTRY; i++) {
+    fram.write8(addr + i, data[i]);
+  }
+
+  // Advance head (circular)
+  framHeader.battHead = (framHeader.battHead + 1) % FRAM_BATT_COUNT;
+  if (framHeader.battCount < FRAM_BATT_COUNT) {
+    framHeader.battCount++;
+  } else {
+    // Ring full — tail advances too (oldest data lost)
+    framHeader.battTail = (framHeader.battTail + 1) % FRAM_BATT_COUNT;
+    logPrintln("[FRAM] WARN: Battery ring overflow");
+  }
+  framHeader.flags |= 0x01;  // Set dirty flag
+  framWriteHeader();
+}
+
+// Check if a real LiPo battery is connected (not just USB power)
+// NOTE: This is a simplified version for data collection.
+// Will be enhanced after analyzing battery log data.
+bool isBatteryConnected() {
+  if (!batteryAvailable) return false;
+
+  float pct = battery.cellPercent();
+
+  // Check for invalid reading
+  if (isnan(pct)) return false;
+
+  // >100% is impossible for real LiPo - indicates USB-only power
+  // MAX17048 reports 100-101% when connected to USB without battery
+  if (pct > 100.0) return false;
+
+  return true;
+}

src/battery.h

@@ -0,0 +1,14 @@
+#pragma once
+// battery.h -- extracted from src.ino by scripts/extract_unit.py (E4).
+#include <Adafruit_MAX1704X.h>
+
+#define BATT_LOG_INTERVAL 10000  // Log every 10 seconds
+#define BATT_LOG_FILE "/battlog.csv"
+
+extern Adafruit_MAX17048 battery;
+extern bool batteryAvailable;
+
+void initBattery();
+void logBatteryToSD();
+void logBatteryToFRAM();
+bool isBatteryConnected();