From 3dc9eb0c63746fc88a8a2cb80ff6234968e20720 Mon Sep 17 00:00:00 2001 From: Cryptotomte Date: Wed, 12 Aug 2026 14:49:34 +0200 Subject: [PATCH 1/7] =?UTF-8?q?docs(spec):=20feature=20012=20=E2=80=94=20r?= =?UTF-8?q?ev2=20board=20profile=20aligned=20with=20frozen=20hardware?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spec, plan, research, data model, contract and tasks for closing the gap between the frozen rev2 schematic (2026-08-12) and the firmware board profile. Spec was rewritten same-day after a codebase survey showed the reservoir-pump divergence was already fixed in feature 006; the real remaining scope is the phantom buttons (BTN_CONFIG polled on EXP_SCK/IO18 at boot), three missing power signals, and stale SYNC1 markers. Analyze pass: 100% coverage, no critical findings; quickstart build commands corrected to CI's canonical forms. Co-Authored-By: Claude Fable 5 --- .specify/feature.json | 4 +- .../checklists/requirements.md | 49 ++++ .../contracts/board-profile-contract.md | 30 +++ specs/012-rev2-pin-map/data-model.md | 33 +++ specs/012-rev2-pin-map/plan.md | 140 ++++++++++ specs/012-rev2-pin-map/quickstart.md | 43 ++++ specs/012-rev2-pin-map/research.md | 86 +++++++ specs/012-rev2-pin-map/spec.md | 243 ++++++++++++++++++ specs/012-rev2-pin-map/tasks.md | 88 +++++++ 9 files changed, 715 insertions(+), 1 deletion(-) create mode 100644 specs/012-rev2-pin-map/checklists/requirements.md create mode 100644 specs/012-rev2-pin-map/contracts/board-profile-contract.md create mode 100644 specs/012-rev2-pin-map/data-model.md create mode 100644 specs/012-rev2-pin-map/plan.md create mode 100644 specs/012-rev2-pin-map/quickstart.md create mode 100644 specs/012-rev2-pin-map/research.md create mode 100644 specs/012-rev2-pin-map/spec.md create mode 100644 specs/012-rev2-pin-map/tasks.md diff --git a/.specify/feature.json b/.specify/feature.json index cb29409..3954931 100644 --- a/.specify/feature.json +++ b/.specify/feature.json @@ -1 +1,3 @@ -{"feature_directory":"specs/006-level-sensors-ina226"} \ No newline at end of file +{ + "feature_directory": "specs/012-rev2-pin-map" +} diff --git a/specs/012-rev2-pin-map/checklists/requirements.md b/specs/012-rev2-pin-map/checklists/requirements.md new file mode 100644 index 0000000..824589d --- /dev/null +++ b/specs/012-rev2-pin-map/checklists/requirements.md @@ -0,0 +1,49 @@ +# Specification Quality Checklist: rev2 board profile aligned with frozen hardware + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-08-12 +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- **2026-08-12 (same day): spec rewritten after codebase survey.** The first + draft assumed the reservoir-pump/IO27 divergence still existed; origin/main + already fixed it in phases 1–3. The survey found the actually-remaining + divergences (phantom buttons with BTN_CONFIG on EXP_SCK/IO18, three missing + signals, stale markers). All checklist items re-validated against the + rewritten spec — still passing. + +- GPIO numbers appear throughout the spec. They are retained deliberately: pin + assignments are frozen *hardware facts* (the contract this feature encodes), + not implementation choices. The spec would be untestable without them. +- The mechanism for board-conditional pump handling (count macro vs capability + flags) is intentionally left to the plan; FR-003 states only the required + property (derived from profile, not hard-coded). +- No [NEEDS CLARIFICATION] markers were needed: scope boundaries (definitions + in, drivers out), safety constraints (constitution I) and both boards' + expected behavior are fully determined by existing project documents. diff --git a/specs/012-rev2-pin-map/contracts/board-profile-contract.md b/specs/012-rev2-pin-map/contracts/board-profile-contract.md new file mode 100644 index 0000000..45fcef9 --- /dev/null +++ b/specs/012-rev2-pin-map/contracts/board-profile-contract.md @@ -0,0 +1,30 @@ +# Contract: board profile macro surface (feature 012) + +The board profile's consumers are other firmware components; the contract is +the macro surface below, pinned by +`test_apps/host/main/test_board_contract_rev{1,2}.cpp`. + +## Guaranteed by this feature + +### Both boards +- `BOARD_HAS_BTN_MANUAL`, `BOARD_HAS_BTN_CONFIG`, `BOARD_HAS_VBAT_SENSE`, + `BOARD_HAS_PWR_PG`, `BOARD_HAS_SENS_PWR_EN` are **always defined** (0 or 1). +- For every flag: value 1 ⟺ corresponding `BOARD_PIN_*` defined. + Unguarded reference on a flag-0 board = compile error. + +### rev1 (`CONFIG_BOARD_REV1_DEVKIT`) +- `BOARD_PIN_BTN_MANUAL == 5`, `BOARD_PIN_BTN_CONFIG == 18` (unchanged). +- New signal flags all 0; their pins undefined. +- Every pre-existing macro value unchanged (behavioral no-op). + +### rev2 (`CONFIG_BOARD_REV2`) +- Button flags 0; button pins undefined. +- `BOARD_PIN_VBAT_SENSE == 34`, `BOARD_PIN_PWR_PG == 35`, + `BOARD_PIN_SENS_PWR_EN == 25`; flags 1. +- No defined `BOARD_PIN_*` in {18, 19, 23, 4, 27} (expansion reservation). +- Zero `TODO(SYNC1)` markers; pin groups cite `02-mcu.md §2.2` frozen map. + +## Explicitly NOT in this contract (PR-14 scope) +- Drivers/consumers for VBAT_SENSE (ADC calibration, FW-1), PWR_PG + monitoring, SENS_PWR_EN sequencing (FW-3 settle), solar INA226 (0x41). +- Any physical re-provisioning trigger on rev2 (BOOT-button reuse idea). diff --git a/specs/012-rev2-pin-map/data-model.md b/specs/012-rev2-pin-map/data-model.md new file mode 100644 index 0000000..a9b0a94 --- /dev/null +++ b/specs/012-rev2-pin-map/data-model.md @@ -0,0 +1,33 @@ +# Data Model: board profile capability matrix + +**Feature**: 012-rev2-pin-map · the "data" is compile-time facts. + +## Capability flags and pins per board (target state) + +| Capability flag | rev1 | rev1 pin | rev2 | rev2 pin | Change | +|---|---|---|---|---|---| +| `BOARD_HAS_RS485_DE` | 1 | 25 | 0 | — | none (existing) | +| `BOARD_HAS_RESERVOIR_PUMP` | 1 | 27 | 0 | — | none (existing) | +| `BOARD_HAS_INA226` | 0 | — | 1 | I2C 0x40 | none (existing) | +| `BOARD_HAS_BTN_MANUAL` | **1** | 5 | **0** | — | **new flag; rev2 pin removed** | +| `BOARD_HAS_BTN_CONFIG` | **1** | 18 | **0** | — | **new flag; rev2 pin removed** | +| `BOARD_HAS_VBAT_SENSE` | **0** | — | **1** | 34 (ADC1, input-only) | **new** | +| `BOARD_HAS_PWR_PG` | **0** | — | **1** | 35 (input-only, ext. pull-up) | **new** | +| `BOARD_HAS_SENS_PWR_EN` | **0** | — | **1** | 25 (output, rail OFF default) | **new** | + +Unconditional pins (both boards, unchanged): I2C 21/22 · RS485 TX/RX 16/17 · +MAIN_PUMP 26 · LEVEL_LOW/HIGH 32/33 · STATUS_LED 2. rev2-only constants +(unchanged): `BOARD_LEVEL_ACTIVE_LOW 1`, debounce 300 / settle 500 ms, +`BOARD_RS485_UART_PORT 2`. + +## Invariants (compile-time enforced) + +1. Flag = 1 ⟺ pin macro defined; flag = 0 ⟺ pin macro undefined + (per flag, both boards — sanity `#error` pair each). +2. rev2: no defined `BOARD_PIN_*` ∈ expansion set {18, 19, 23, 4, 27}. +3. Note: rev1 legitimately violates (2) — BTN_CONFIG=18, RESERVOIR_PUMP=27 — + the expansion reservation is a **rev2-only** invariant; the check must be + inside the rev2 conditional. +4. All pin-distinctness checks already in the sanity section remain and must + still pass with the new definitions (25 appears on rev1 as RS485_DE and on + rev2 as SENS_PWR_EN — never both in one target). diff --git a/specs/012-rev2-pin-map/plan.md b/specs/012-rev2-pin-map/plan.md new file mode 100644 index 0000000..a8b5dc2 --- /dev/null +++ b/specs/012-rev2-pin-map/plan.md @@ -0,0 +1,140 @@ +# Implementation Plan: rev2 board profile aligned with frozen hardware + +**Branch**: `fix/rev2-pin-map` | **Date**: 2026-08-12 | **Spec**: [spec.md](spec.md) + +**Input**: Feature specification from `specs/012-rev2-pin-map/spec.md` + +## Summary + +Close the gap between the frozen rev2 hardware (schematic freeze 2026-08-12) +and the firmware's rev2 board profile: remove the phantom button pins (one of +which sits on the expansion header's SPI clock and is polled at boot), add the +three missing frozen signals behind capability flags, and retire the stale +SYNC1 markers — using the established `BOARD_HAS_*` + deliberate-undefine +enforcement pattern, extended contract tests, and zero behavioral change on +rev1. + +## Technical Context + +**Language/Version**: C/C++ (C17 / C++17), ESP-IDF v6.0.1 (pinned Docker image) + +**Primary Dependencies**: ESP-IDF GPIO/Kconfig only — no new components + +**Storage**: N/A + +**Testing**: host test app (`firmware/test_apps/host`, CMake + pinned +toolchain in CI), compile-time contract TUs per board target + +**Target Platform**: ESP32 (rev1 devkit / rev2 custom PCB via +`CONFIG_BOARD_REV1_DEVKIT` / `CONFIG_BOARD_REV2`) + +**Project Type**: embedded firmware, single repo + +**Performance Goals**: N/A (compile-time facts; no runtime additions) + +**Constraints**: pumps-OFF-at-boot invariant untouchable (constitution I); +rev1 behavior byte-equivalent; no hardware available for verification — +everything must be provable by build + host tests + +**Scale/Scope**: 1 header, 1 boot file, 2 contract-test TUs, docs citations. +No new files except tests if split is cleaner. + +## Constitution Check + +*GATE: evaluated pre-Phase 0 and re-checked post-design — PASS on both.* + +| Principle | Verdict | Note | +|---|---|---| +| I Safety First | PASS | Boot pump-OFF logic untouched on both boards; button-path guard removes a spurious-provisioning risk. No fail-safe weakened. | +| II Host-Testability | PASS | All new facts are compile-time; contract TUs extended; no hardware needed. | +| III Reproducible Builds | PASS | No dependency changes; CI already builds both targets. | +| IV Frozen Legacy | PASS | Arduino tree untouched. | +| V Checkpoint-Gated Workflow | PASS | CP1 passed silently; this plan stops at CP2 before implementation. | +| VI (remaining principles) | PASS | No scope beyond profile alignment. | + +## Project Structure + +### Documentation (this feature) + +```text +specs/012-rev2-pin-map/ +├── spec.md +├── plan.md # this file +├── research.md # R1–R6: survey, mechanism decision, consumers +├── data-model.md # capability-flag/pin matrix for both boards +├── quickstart.md # build/test validation commands +├── contracts/ +│ └── board-profile-contract.md +├── checklists/requirements.md +└── tasks.md # /speckit-tasks output (next step) +``` + +### Source Code (repository root) + +```text +firmware/ +├── components/board/include/board/board.h # the profile — main edit +├── main/app_main.cpp # guard config-button block +└── test_apps/host/main/ + ├── test_board_contract_rev1.cpp # extend: buttons present, new flags 0 + └── test_board_contract_rev2.cpp # extend: buttons absent, new pins, expansion disjointness +``` + +**Structure Decision**: existing layout; no new components. The board profile +stays a single header per the project rule "pin tables live in +`firmware/components/board/` only". + +## Design + +### D1 — board.h rev2 section (per research R1/R2) + +1. Remove `BOARD_PIN_BTN_MANUAL` / `BOARD_PIN_BTN_CONFIG` from rev2; add + `BOARD_HAS_BTN_MANUAL` / `BOARD_HAS_BTN_CONFIG` **to both sections** + (rev1 = 1 with existing pins 5/18 unchanged; rev2 = 0, pins undefined). +2. Add to rev2 (with `BOARD_HAS_* 1`), absent from rev1 (`BOARD_HAS_* 0`): + - `BOARD_PIN_VBAT_SENSE 34` — input-only, ADC1 (FW-1: nonlinearity note + stays in rev2-firmware-notes; the profile carries pin + ADC1-only fact) + - `BOARD_PIN_PWR_PG 35` — input-only, open-drain + external pull-up, no + internal pulls on IO34–39 (FW-6) + - `BOARD_PIN_SENS_PWR_EN 25` — output, sensor rail OFF default +3. Replace all 11 `TODO(SYNC1)` markers and the "provisionally mirror rev 1" + header with citations: `02-mcu.md §2.2 SYNC 1 map (frozen 2026-08-12)`. +4. Update the I²C address-map comment: 0x41 = solar INA226, populated on this + node (support lands PR-14). Address constant for it NOT added — PR-14 owns + the driver surface. +5. Sanity section: flag↔pin consistency checks for the five new flags (same + pattern as reservoir pump), plus rev2-only expansion-reservation check — + compile error if any defined `BOARD_PIN_*` ∈ {18, 19, 23, 4, 27}. + +### D2 — app_main.cpp (per research R3) + +Wrap the config-button provisioning block (`gpio_config` of BTN_CONFIG, the +poll loop, its LED feedback) in `#if BOARD_HAS_BTN_CONFIG`. The +credentials-absent provisioning path is unconditional and untouched. Pump +boot-safety code untouched. + +### D3 — contract tests (per research R4) + +- rev1 TU: `BOARD_HAS_BTN_CONFIG == 1`, `BOARD_PIN_BTN_CONFIG == 18`, + `BOARD_HAS_BTN_MANUAL == 1`, pin 5; new signal flags all 0 and pins + undefined (`#ifdef` + `#error`). +- rev2 TU: button flags 0, pins undefined; `BOARD_PIN_VBAT_SENSE == 34`, + `BOARD_PIN_PWR_PG == 35`, `BOARD_PIN_SENS_PWR_EN == 25`, flags 1; + static_assert that every defined core pin ∉ expansion set (belt to the + header's braces — catches accidental future edits even if the header check + is removed). +- Grep-style truth checks (zero `TODO(SYNC1)`) are a CI/test-script concern: + add a trivial host test or CI step ONLY if cheap; otherwise SC-004 is + verified at review. (Tasks phase decides; do not over-engineer.) + +### D4 — docs + +`firmware/CLAUDE.md` board section: note the five new capability flags and +the frozen-map citation convention, if that file documents the profile (check +at implementation; keep diff minimal). + +## Complexity Tracking + +No constitution violations. No new mechanisms — the feature deliberately +reuses the existing enforcement pattern (research R2 rejected the +`BOARD_PUMP_COUNT` alternative as a second parallel mechanism). diff --git a/specs/012-rev2-pin-map/quickstart.md b/specs/012-rev2-pin-map/quickstart.md new file mode 100644 index 0000000..d17ef1c --- /dev/null +++ b/specs/012-rev2-pin-map/quickstart.md @@ -0,0 +1,43 @@ +# Quickstart: validate feature 012 + +All validation is host-side + containerized builds — no hardware. Commands +below mirror `.github/workflows/firmware-build.yml` (the canonical forms). + +## 1. Host test suite (contract TUs compile the real header) + +```bash +cd firmware/test_apps/host +idf.py --preview set-target linux +idf.py build +./build/pump_host_tests.elf +``` + +Expected: build succeeds (the contract TUs are compile-time — a contract +violation IS a build failure) and the runtime suite passes with no +regressions. + +## 2. Both board targets, pinned container + +```bash +docker run --rm -v "$PWD/firmware":/project -w /project espressif/idf:v6.0.1 \ + idf.py -DSDKCONFIG_DEFAULTS="sdkconfig.defaults;sdkconfig.board.rev1_devkit" build +docker run --rm -v "$PWD/firmware":/project -w /project espressif/idf:v6.0.1 \ + idf.py -DSDKCONFIG_DEFAULTS="sdkconfig.defaults;sdkconfig.board.rev2" build +``` + +(CI additionally greps `firmware/sdkconfig` for `CONFIG_BOARD_REV1_DEVKIT=y` / +`CONFIG_BOARD_REV2=y` to prove the right profile was built — do the same if +building both locally back-to-back, and clean between runs so the second +build does not inherit the first's sdkconfig.) + +## 3. Truth checks (SC-004) + +```bash +grep -rn "TODO(SYNC1)\|provisionally" firmware/components/board/ && echo FAIL || echo OK +``` + +## 4. Negative proof (optional, high-value) + +Temporarily add an unguarded `BOARD_PIN_BTN_CONFIG` reference to the rev2 +contract TU — the host build must FAIL. Revert. (Demonstrates the enforcement +pattern actually bites; the rev1 TU documents this technique.) diff --git a/specs/012-rev2-pin-map/research.md b/specs/012-rev2-pin-map/research.md new file mode 100644 index 0000000..eb363d2 --- /dev/null +++ b/specs/012-rev2-pin-map/research.md @@ -0,0 +1,86 @@ +# Research: rev2 board profile aligned with frozen hardware + +**Date**: 2026-08-12 · **Feature**: 012-rev2-pin-map + +## R1 — What is actually divergent on origin/main (survey result) + +**Decision basis for the whole feature.** The initial assumption (reservoir +pump on IO27 still defined for rev2) was FALSE — fixed in feature 006 with +`BOARD_HAS_RESERVOIR_PUMP`, deliberate pin-undefine enforcement, guarded +app_main, and contract tests. Verified divergences that remain: + +| # | Divergence | Evidence | +|---|---|---| +| 1 | Phantom buttons: rev2 defines `BOARD_PIN_BTN_MANUAL 5`, `BOARD_PIN_BTN_CONFIG 18`; neither exists on frozen rev2 (only BOOT/RESET); IO18 = `EXP_SCK` | `board.h` rev2 section; `02-mcu.md` §2.2 SYNC 1 map; `08-expansion.md` | +| 2 | Boot path reads the phantom button: `app_main.cpp` configures IO18 as input and polls it (with STATUS_LED feedback) to enter provisioning | `app_main.cpp:148-217` | +| 3 | Missing signals: `VBAT_SENSE` (IO34), `PWR_PG` (IO35), `SENS_PWR_EN` (IO25) undefined | grep over `board.h`; `01-power.md` §1.0a; `rev2-firmware-notes.md` FW-1/FW-6 | +| 4 | 11 stale `TODO(SYNC1)` markers + "provisionally mirror rev 1" header | grep count | +| 5 | Stale comment: I²C map calls 0x41 "solar footprint, DNP"; solar group is populated per 2026-08-12 decision | `board.h` rev2 INA226 block; `01-power.md` §1.5 | + +All present rev2 pin VALUES verified correct against the frozen contract +(I2C 21/22, RS485 16/17, pump 26, level 32/33, LED 2). The fix is +subtractive/additive, not corrective, for existing values. + +## R2 — Mechanism for board-conditional peripherals + +**Decision**: per-peripheral capability flags (`BOARD_HAS_*`) with the +deliberate-undefine enforcement pattern. + +**Rationale**: the codebase already settled this twice — `BOARD_HAS_RS485_DE` +(feature 004) and `BOARD_HAS_RESERVOIR_PUMP` (feature 006), both with the +"flag 0 ⇒ pin macro undefined ⇒ unguarded reference is a compile error" +pattern, sanity-checked in the header's consistency section and pinned by the +contract-test TUs. Introducing `BOARD_PUMP_COUNT` (the alternative floated at +spec time) would add a second parallel mechanism for the same job. + +**Alternatives considered**: `BOARD_PUMP_COUNT` — rejected: pumps already use +the HAS-flag pattern on origin/main; a count adds nothing the flag doesn't +give and invites index-loop code over pins that don't form an array. + +**New flags this feature adds**: `BOARD_HAS_BTN_MANUAL`, `BOARD_HAS_BTN_CONFIG` +(rev1 = 1, rev2 = 0); `BOARD_HAS_VBAT_SENSE`, `BOARD_HAS_PWR_PG`, +`BOARD_HAS_SENS_PWR_EN` (rev1 = 0, rev2 = 1). Flags are defined 0/1 on BOTH +boards (never absent) so `#if` works uniformly; pin macros exist only where +flag = 1. + +## R3 — Consumers that must become conditional + +Grep survey of `BOARD_PIN_BTN_*` / `BOARD_PIN_STATUS_LED` outside board.h: + +- `app_main.cpp:148-217` — config-button provisioning entry (IO18 input cfg, + poll loop, LED feedback). Must be `#if BOARD_HAS_BTN_CONFIG`. The + credentials-absent provisioning path is separate and stays unconditional. +- `wifi_task.cpp/h` — STATUS_LED only; LED exists on both boards; no change. +- `BOARD_PIN_BTN_MANUAL` — **zero consumers** outside board.h. Removing the + rev2 definition breaks nothing. + +## R4 — Where the contract is enforced + +- `firmware/test_apps/host/main/test_board_contract_rev{1,2}.cpp`: each TU + defines `CONFIG_BOARD_REVx 1` and includes the REAL `board/board.h`, + pinning values/absences with `static_assert` / `#ifdef` + `#error`. The + natural home for every new assertion (buttons, new signals, expansion + disjointness). +- `board.h` sanity section: compile-time distinctness and flag/pin + consistency checks; gains (a) flag↔pin consistency for the five new flags, + (b) a rev2-only expansion-reservation check asserting no defined core pin + equals 18/19/23/4/27. +- CI already builds both targets + host suite (constitution III). + +## R5 — Expansion-set reservation as compile-time property (SC-002) + +**Decision**: express "boot-claimed ⊆ profile ∧ profile ∩ expansion = ∅" in +the header's sanity section: on rev2, `#if` chains erroring if any defined +`BOARD_PIN_*` equals an expansion GPIO. Boot code derives every claimed pin +from `BOARD_PIN_*` macros (it already does), so header-level disjointness + +guarded button path together give the SC-002 property without runtime cost. + +**Alternative considered**: runtime assertion table — rejected: adds code to +the safety-critical boot path for a property fully decidable at compile time. + +## R6 — Provisioning UX on buttonless rev2 + +**Decision**: accept credentials-absent as the only rev2 provisioning +trigger (spec assumption, flagged to Paul at CP2). BOOT-button (IO0) reuse +after boot is feasible ESP32 practice but is new function on a frozen-scope +fix — deferred to PR-14+. diff --git a/specs/012-rev2-pin-map/spec.md b/specs/012-rev2-pin-map/spec.md new file mode 100644 index 0000000..0ecc472 --- /dev/null +++ b/specs/012-rev2-pin-map/spec.md @@ -0,0 +1,243 @@ +# Feature Specification: rev2 board profile aligned with frozen hardware + +**Feature Branch**: `fix/rev2-pin-map` + +**Created**: 2026-08-12 + +**Status**: Draft (rewritten same day after codebase survey — see Revision note) + +**Input**: User description: "Align the rev2 board profile in firmware/components/board/include/board/board.h with the frozen rev2 hardware design. Close the stale SYNC1 markers, remove phantom peripherals, add the missing frozen signals, keep the pumps-OFF-at-boot invariant intact on both boards." + +## Revision note + +The first draft of this spec assumed the rev2 profile still defined two pump +channels with a reservoir pump on IO27. A survey of `origin/main` (the branch +this feature builds on) showed that fix already landed during phases 1–3: +`BOARD_HAS_RESERVOIR_PUMP == 0` on rev2, the pin macro deliberately undefined, +boot safety properly guarded, host contract tests in place +(`test_board_contract_rev1/rev2.cpp`). The earlier observation came from a +stale working tree. This spec covers what actually remains divergent. + +## Context + +The rev2 hardware design was frozen 2026-08-12 (all 8 schematic sheets drawn, +ERC clean, components ordered). Surveying the current rev2 board profile +against the frozen design leaves three real divergence classes: + +1. **A boot-active phantom peripheral (the serious one).** The profile defines + `BOARD_PIN_BTN_CONFIG = 18` and `BOARD_PIN_BTN_MANUAL = 5`, and the boot + sequence configures IO18 as an input and reads it to decide whether to + enter WiFi provisioning mode. On frozen rev2 hardware **neither button + exists** (the board has only BOOT/RESET switches on IO0/EN and a status + LED on IO2), and **IO18 is `EXP_SCK`** — the expansion header's SPI clock + (J7, reserved set IO18/19/23/4/27). Consequences on real rev2 hardware: + whatever an attached expansion device does with SCK can be misread as + "operator holds the config button" and drop the node into provisioning + mode at boot; and core firmware claims a pin the design reserves for + expansion. +2. **Missing frozen signals.** Three signals the frozen design provides have + no definition: battery-voltage sense (IO34, ADC1, input-only), buck + power-good (IO35, input-only, open-drain with external pull-up), and + sensor-rail enable (IO25, output, rail OFF by default). +3. **Stale annotations.** Eleven "TODO(SYNC1)" markers and a section header + saying the pin map "provisionally mirrors rev 1" — but SYNC 1 (the pin-map + freeze) has happened and every present value matches the frozen contract. + Also one stale hardware comment: the I²C address-map note calls the solar + INA226 (0x41) "footprint, DNP", but the populate decision of 2026-08-12 + made it a populated device on this node. + +Already correct on `origin/main` — to be regression-guarded, not re-done: +single-pump profile with compile-error enforcement, level-sensor polarity and +timing constants (active LOW, 300/500 ms), pump INA226 at 0x40, status LED on +IO2 (matches the frozen design), and the existing board contract host tests. + +Sources of truth: `hardware/rev2/design-notes/02-mcu.md` §2.2 (SYNC 1 GPIO +map) and `00-architecture.md` §0.5 pin contract (both on the +`docs/single-pump-node` branch), `docs/rev2-firmware-notes.md` FW-1..FW-6, +`docs/parity-checklist.md` for rev1. + +### Frozen rev2 pin facts (the contract this feature encodes) + +| Signal | GPIO | Direction / note | +|---|---|---| +| I2C SDA / SCL | IO21 / IO22 | bidirectional; 3 devices: BME280 0x77, INA226 0x40 (pump), INA226 0x41 (solar, populated) | +| RS485 TX / RX | IO16 / IO17 | auto-direction transceiver, no DE pin | +| Pump enable (single pump) | IO26 | active-HIGH output *(already correct)* | +| Reservoir level LOW / HIGH | IO32 / IO33 | inputs, active LOW *(already correct)* | +| Status LED | IO2 | output *(already correct)* | +| Sensor-rail enable | IO25 | output; switched 12 V sensor domain, OFF by default — **missing today** | +| Battery voltage sense | IO34 | input-only, ADC1; ADC2 unusable with WiFi — **missing today** | +| Buck power-good | IO35 | input-only, open-drain externally pulled up; IO34–39 have no internal pulls — **missing today** | +| Manual / config buttons | — | **do not exist on rev2** (BOOT/RESET only) — profile must say so | +| Expansion (reserved, untouchable) | IO18/19/23/4/27 | J7: VSPI SCK/MOSI/MISO + CS/IRQ; core firmware must not claim any of them | + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - rev2 boot touches only pins that exist on rev2 (Priority: P1) + +Paul boots the firmware on rev2 hardware (module prototype or final PCB) with +an expansion device attached to J7. The node never claims an expansion pin and +never mistakes expansion-bus activity for a pressed configuration button. + +**Why this priority**: This is the correctness- and hardware-protecting fix. +Today's boot path reads `EXP_SCK` as if it were a button — a spurious +provisioning entry at best, interference with attached expansion hardware at +worst, on a project whose failure history is exactly "pins used for something +the hardware didn't intend". + +**Independent Test**: Host contract tests for the rev2 target assert the +button macros are absent (unguarded reference fails to compile) and that the +boot path's claimed-pin set is disjoint from the expansion set. + +**Acceptance Scenarios**: + +1. **Given** the rev2 board target, **When** the firmware boots, **Then** no + GPIO in the expansion set IO18/19/23/4/27 is configured (input or output) + or read by core firmware. +2. **Given** the rev2 board target with no stored WiFi credentials, **When** + the firmware boots, **Then** provisioning mode is entered via the + credentials-absent path exactly as before — the button path simply does + not exist on this board. +3. **Given** the rev1 board target, **When** the firmware boots, **Then** the + config-button check on IO18, the manual button on IO5, and both pump + outputs behave exactly as today — rev1 has this hardware and must not + change. + +--- + +### User Story 2 - the profile states every frozen rev2 signal (Priority: P2) + +A developer (human or agent) implementing the rev2 bring-up drivers (PR-14) +reads the board profile and finds every signal the frozen hardware provides — +battery sense, power-good, sensor-rail enable — with direction and polarity +constraints, without opening the schematic. + +**Why this priority**: The board profile is the single place pin facts live +(project rule: pin tables are not duplicated elsewhere). Missing entries force +the next developer back to the schematic and invite guessed constants. + +**Independent Test**: rev2 contract test asserts the three new definitions +exist with the frozen values; rev1 contract test asserts referencing them +unguarded fails to compile. + +**Acceptance Scenarios**: + +1. **Given** the rev2 board target, **When** a driver references battery + sense, power-good, or sensor-rail enable, **Then** the definitions exist + with the frozen GPIO numbers (IO34, IO35, IO25) and capability flags. +2. **Given** the rev1 board target, **When** the same references are made + unguarded, **Then** compilation fails (rev1 lacks all three) — the + established enforcement pattern. + +--- + +### User Story 3 - annotations tell the truth (Priority: P3) + +A reader of the rev2 profile sees no stale "TODO(SYNC1)" markers, no +"provisionally mirrors rev 1" language, and no "solar INA226 is DNP" claim; +each section instead cites the frozen design source. + +**Why this priority**: Stale TODOs actively invite "helpful" future edits to +values that are frozen, and the DNP claim contradicts a purchasing decision +already made. + +**Independent Test**: Text search over the board component returns zero +occurrences of "TODO(SYNC1)" and "provisionally"; the I²C address-map comment +describes 0x41 as populated on this node. + +**Acceptance Scenarios**: + +1. **Given** the merged feature, **When** searching the board component for + SYNC1 markers or provisional wording, **Then** none remain in the rev2 + section and each pin group cites its design-note source. + +--- + +### Edge Cases + +- Watchdog/panic/OTA reset (not cold boot): the boot path runs the same + claiming logic — the expansion-untouched and single-pump guarantees hold on + every reset type. +- rev2 provisioning re-entry on a deployed node: with the button path absent, + re-provisioning relies on the credentials-absent path and whatever + API/reset mechanisms PR-07 provides. If a physical re-provisioning trigger + is wanted on rev2, the BOOT button (IO0) is free after boot — that is a + future feature (PR-14 or later), explicitly out of scope here. +- A future board with different button complement must be expressible via the + capability flags without touching application logic. + +## Requirements *(mandatory)* + +### Functional Requirements + +- **FR-001**: The rev2 board profile MUST NOT define manual- or config-button + pins; button presence MUST be a capability flag, and an unguarded reference + to an absent button pin MUST fail compilation (established enforcement + pattern). +- **FR-002**: The rev1 board profile MUST remain behaviorally identical: all + existing rev1 pin values, both buttons, both pumps, unchanged. +- **FR-003**: The boot provisioning-entry logic MUST compile the button path + only on boards that have a config button; on buttonless boards the + credentials-absent trigger (existing PR-07 behavior) is the sole entry and + MUST be preserved unchanged. +- **FR-004**: On rev2, core firmware MUST NOT configure, drive, or read any + GPIO in the expansion set (IO18, IO19, IO23, IO4, IO27). +- **FR-005**: The rev2 profile MUST define battery-voltage sense (IO34, + ADC1, input-only), buck power-good (IO35, input-only, externally pulled + up), and sensor-rail enable (IO25, output, rail OFF default), each behind a + capability flag with the compile-error enforcement pattern on boards that + lack the signal. +- **FR-006**: All "TODO(SYNC1)" markers and provisional wording MUST be + removed from the board component; each rev2 pin group MUST cite its frozen + design source; the solar INA226 comment MUST reflect the populate decision + (0x41 populated on this node; firmware support for reading it is PR-14 + scope). +- **FR-007**: Host contract tests MUST be extended to cover: button absence + on rev2 / presence on rev1, the three new rev2 signals, and the + expansion-set-disjointness of boot-claimed pins; existing pump-invariant + tests MUST remain green unchanged. +- **FR-008**: Both board targets MUST build green in CI from a clean + checkout; the host test suite MUST pass. + +### Key Entities + +- **Board profile**: per-target pin definitions and capability flags; the + single source of pin truth in firmware. +- **Boot-claimed pin set**: every GPIO the boot path configures or reads on a + target; must be derivable from the profile and disjoint from the expansion + set on rev2. +- **Expansion pin set**: IO18/19/23/4/27 — reserved for J7, unclaimed by core + firmware on rev2. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: Host contract tests pass asserting: rev2 defines no button + pins, defines the three new signals with frozen values, and rev1 values are + unchanged. +- **SC-002**: The rev2 boot path's claimed-pin set is provably disjoint from + IO18/19/23/4/27 (compile-time assertion or host test). +- **SC-003**: CI builds both board targets green on the feature branch; host + suite passes. +- **SC-004**: Zero "TODO(SYNC1)" and zero "provisionally" occurrences remain + in the board component. +- **SC-005**: The rev2 profile covers all rows of the frozen pin facts table + (present signals defined, absent peripherals capability-flagged off). + +## Assumptions + +- The frozen pin facts table is correct and final, extracted from + `02-mcu.md` §2.2 SYNC 1 map and the §0.5 pin contract after the 2026-08-12 + freeze. Future pin changes go schematic-first and re-open the profile + deliberately. +- Consuming the new signal definitions (ADC reads, PG monitoring, rail + switching, solar INA226 support) is PR-14 driver scope; this feature only + makes the facts available and safe. +- Losing the physical config-button provisioning trigger on rev2 is accepted: + the hardware has no such button, and the credentials-absent path covers + first provisioning. A BOOT-button-based trigger is a possible future + feature, out of scope. +- The legacy Arduino tree is frozen and untouched. +- No hardware is required for verification; host tests + CI builds suffice. + Bench verification lands with PR-14 bring-up. diff --git a/specs/012-rev2-pin-map/tasks.md b/specs/012-rev2-pin-map/tasks.md new file mode 100644 index 0000000..885b1b5 --- /dev/null +++ b/specs/012-rev2-pin-map/tasks.md @@ -0,0 +1,88 @@ +# Tasks: rev2 board profile aligned with frozen hardware + +**Input**: Design documents from `specs/012-rev2-pin-map/` +**Prerequisites**: plan.md (D1–D4), research.md (R1–R6), data-model.md (capability matrix), contracts/board-profile-contract.md + +**Tests**: Included — the contract tests ARE the feature's verification +(constitution II; spec FR-007). Ordering is test-first: contract assertions +are written before the header edits that make them pass; a red step is a +build failure, which is the intended TDD signal for compile-time contracts. + +**Organization**: By user story, in spec priority order. All stories touch +`board.h` and the contract TUs, so tasks run sequentially (same files — no +[P] markers except the truly independent docs task). + +## Phase 1: Setup + +- [ ] T001 Record the green baseline: run the host test suite and both board-target builds per `specs/012-rev2-pin-map/quickstart.md` §1–2 on the unmodified branch; note results in the task log. A pre-existing red here is a STOP — this feature must start from green. + +## Phase 2: Foundational + +*(No blocking infrastructure — single-header feature. Proceed to stories.)* + +## Phase 3: User Story 1 — rev2 boot touches only pins that exist (P1) + +**Goal**: remove the phantom buttons (BTN_CONFIG on EXP_SCK/IO18 polled at +boot), guard the boot provisioning button path, reserve the expansion set. + +**Independent test**: host build fails if rev2 defines a button pin or any +core pin collides with the expansion set; app_main compiles for rev2 without +the button block and for rev1 with it, unchanged. + +- [ ] T002 [US1] Extend `firmware/test_apps/host/main/test_board_contract_rev2.cpp`: assert `BOARD_HAS_BTN_MANUAL == 0` and `BOARD_HAS_BTN_CONFIG == 0`; `#ifdef BOARD_PIN_BTN_MANUAL/`BOARD_PIN_BTN_CONFIG` → `#error`; add static_asserts that every defined core `BOARD_PIN_*` (I2C, RS485, pump, levels, LED — and the US2 signals once they exist) differs from each of 18, 19, 23, 4, 27. Expect the build to go red (pins still defined). +- [ ] T003 [US1] Extend `firmware/test_apps/host/main/test_board_contract_rev1.cpp`: assert `BOARD_HAS_BTN_MANUAL == 1`, `BOARD_PIN_BTN_MANUAL == 5`, `BOARD_HAS_BTN_CONFIG == 1`, `BOARD_PIN_BTN_CONFIG == 18` — pins rev1 genuinely has, values frozen (FR-002 regression guard). Document in the TU comment that the expansion reservation is rev2-only (rev1 legitimately uses 18/27 — data-model invariant 3). +- [ ] T004 [US1] Edit `firmware/components/board/include/board/board.h`: add `BOARD_HAS_BTN_MANUAL 1` / `BOARD_HAS_BTN_CONFIG 1` to the rev1 section above the existing pins; in the rev2 section set both flags 0 and DELETE both pin defines (deliberate-undefine pattern, comment mirroring the RS485_DE/reservoir-pump wording). +- [ ] T005 [US1] Edit `firmware/components/board/include/board/board.h` sanity section: add flag↔pin consistency `#error` pairs for both button flags (pattern of lines 211–217); add the rev2-only expansion-reservation check (`#if`-chain erroring if any defined core pin ∈ {18,19,23,4,27}), placed inside a `CONFIG_BOARD_REV2` conditional per data-model invariant 3. +- [ ] T006 [US1] Edit `firmware/main/app_main.cpp`: wrap the config-button provisioning block (input `gpio_config` of `BOARD_PIN_BTN_CONFIG`, the poll loop and its STATUS_LED feedback, approx. lines 148–217) in `#if BOARD_HAS_BTN_CONFIG`; the credentials-absent provisioning path and all pump boot-safety code stay untouched. Update the block comment to state why (no button on rev2; IO18 is EXP_SCK). +- [ ] T007 [US1] Validate US1: host suite green, both board targets build green (quickstart §1–2). + +**Checkpoint**: US1 delivers the safety/correctness fix on its own. + +## Phase 4: User Story 2 — profile states every frozen signal (P2) + +**Goal**: add VBAT_SENSE (IO34), PWR_PG (IO35), SENS_PWR_EN (IO25) behind +capability flags. + +**Independent test**: rev2 TU asserts the three pins/flags; rev1 TU proves +unguarded references cannot compile on rev1. + +- [ ] T008 [US2] Extend `firmware/test_apps/host/main/test_board_contract_rev2.cpp`: assert `BOARD_HAS_VBAT_SENSE == 1` / `BOARD_PIN_VBAT_SENSE == 34`, `BOARD_HAS_PWR_PG == 1` / `BOARD_PIN_PWR_PG == 35`, `BOARD_HAS_SENS_PWR_EN == 1` / `BOARD_PIN_SENS_PWR_EN == 25`; fold the three pins into the T002 expansion-disjointness asserts. Red until T010. +- [ ] T009 [US2] Extend `firmware/test_apps/host/main/test_board_contract_rev1.cpp`: assert all three flags are `== 0` and pins undefined (`#ifdef` → `#error`) on rev1. +- [ ] T010 [US2] Edit `firmware/components/board/include/board/board.h`: rev1 section — three flags at 0; rev2 section — three flags at 1 with pins 34/35/25 and constraint comments (IO34 input-only ADC1, ADC2 dead with WiFi, FW-1 pointer; IO35 input-only, open-drain + external pull-up, no internal pulls IO34–39, FW-6; IO25 output, sensor rail OFF default, FW-3 settle owned by PR-14). Add the three flag↔pin sanity pairs; confirm the pin-distinctness checks still pass (25 = RS485_DE on rev1 vs SENS_PWR_EN on rev2 — never same target). +- [ ] T011 [US2] Validate US2: host suite green, both board targets build green. + +**Checkpoint**: PR-14 driver work now has every frozen fact available. + +## Phase 5: User Story 3 — annotations tell the truth (P3) + +**Goal**: zero stale markers; citations in; solar comment corrected. + +**Independent test**: quickstart §3 grep returns nothing. + +- [ ] T012 [US3] Edit `firmware/components/board/include/board/board.h`: delete all 11 `TODO(SYNC1)` markers and the "GPIO numbers provisionally mirror rev 1…" header sentence; each rev2 pin group cites `hardware/rev2/design-notes/02-mcu.md §2.2 SYNC 1 map (frozen 2026-08-12)` once, group-level not per-line; update the I²C address-map comment: `0x41 solar INA226 — populated on this node (decision 2026-08-12; driver support PR-14)`, BME280 = 0x77. +- [ ] T013 [US3] Validate US3: `grep -rn "TODO(SYNC1)\|provisionally" firmware/components/board/` returns nothing (quickstart §3); host suite + both targets still green. + +## Phase 6: Polish & cross-cutting + +- [ ] T014 [P] Check `firmware/CLAUDE.md` board-profile documentation (plan D4): if it enumerates capability flags or the pin-table convention, add the five new flags and the frozen-map citation convention; if it doesn't document the profile, change nothing. +- [ ] T015 Negative proof (quickstart §4): temporarily add an unguarded `BOARD_PIN_BTN_CONFIG` use to the rev2 TU, confirm the build FAILS, revert. Record in the implementation notes. +- [ ] T016 Full quickstart pass (§1–3) as the feature's final gate; assemble the implementer's test checklist for CP3 (host-test PR — CI suite is the verifier; no HIL needed, note the deferred bench verification lands in PR-14). + +## Dependencies & execution order + +- T001 → everything (green baseline gate). +- US1 (T002–T007) → US2 (T008–T011): same files, and T008 folds new pins + into T002's disjointness list. US2 → US3 (T012–T013): comment edits land + last so citations describe the final content. Polish last; T014 is [P] + (different file, no dependency). +- MVP scope: **US1 alone** — the safety/correctness fix stands on its own if + implementation must stop early. + +## Implementation strategy + +Test-first per story: write the contract assertions (red = build failure), +make them green with the header/app edit, validate, move on. Every task's +"green" is machine-checkable (build result or grep), so the implementer can +self-verify without judgment calls. Commit granularity: one commit per story +phase (see CLAUDE.md one-branch-at-a-time rule; commits stay on +fix/rev2-pin-map in the worktree). From 349bd01a9be6916df49716da1ea754ff32a0cdac Mon Sep 17 00:00:00 2001 From: Cryptotomte Date: Wed, 12 Aug 2026 15:09:47 +0200 Subject: [PATCH 2/7] fix(board): remove phantom rev2 buttons and reserve the expansion pins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rev2 profile defined BTN_MANUAL=5 and BTN_CONFIG=18, and the boot path configured IO18 as an input and read it to decide whether to force WiFi provisioning. The frozen rev2 board has neither button (only BOOT/RESET and the status LED), and IO18 is EXP_SCK on the expansion header J7 — so on real hardware expansion-bus traffic could be misread as a held config button, and core firmware claimed a pin the design reserves. - board.h: BOARD_HAS_BTN_MANUAL / BOARD_HAS_BTN_CONFIG on both boards (rev1 = 1 with pins 5/18 unchanged, rev2 = 0 with the pin macros deliberately undefined — the RS485-DE / reservoir-pump enforcement pattern, so unguarded references fail the build). - board.h sanity: flag<->pin consistency pairs for both button flags, the button distinctness check guarded by the flags, and a rev2-ONLY expansion-reservation check erroring if any defined pin lands on IO18/19/23/4/27 (rev1 legitimately uses 18 and 27 — the reservation is a property of the rev2 board, not of the firmware). - app_main: the config-button read, its poll loop and LED feedback compile only under BOARD_HAS_BTN_CONFIG; buttonless boards fall back to the credentials-absent provisioning path (feature 007), unchanged. The pump boot fail-safe is untouched. - Contract TUs pin both sides: rev1 buttons present with frozen values (FR-002 regression guard), rev2 buttons absent plus expansion disjointness for every core pin. Spec: specs/012-rev2-pin-map (US1, FR-001..004, FR-007) Co-Authored-By: Claude Opus 5 --- .../components/board/include/board/board.h | 95 ++++++++++++++++++- firmware/main/app_main.cpp | 21 ++++ .../host/main/test_board_contract_rev1.cpp | 26 +++++ .../host/main/test_board_contract_rev2.cpp | 45 +++++++++ specs/012-rev2-pin-map/tasks.md | 14 +-- 5 files changed, 190 insertions(+), 11 deletions(-) diff --git a/firmware/components/board/include/board/board.h b/firmware/components/board/include/board/board.h index cdd1f60..911cab9 100644 --- a/firmware/components/board/include/board/board.h +++ b/firmware/components/board/include/board/board.h @@ -72,10 +72,22 @@ /* Status LED */ #define BOARD_PIN_STATUS_LED 2 -/* Buttons (manual watering trigger, WiFi config/AP mode) */ +/* Buttons (manual watering trigger, WiFi config/AP mode). + * Rev 1 has both physical buttons; the config button drives the boot + * provisioning-force path (feature 007, docs/parity-checklist.md §7). */ +#define BOARD_HAS_BTN_MANUAL 1 #define BOARD_PIN_BTN_MANUAL 5 +#define BOARD_HAS_BTN_CONFIG 1 #define BOARD_PIN_BTN_CONFIG 18 +/* Power/rail monitoring: none on rev1. The devkit rig runs off USB with a + * permanently powered sensor rail, so battery sense, buck power-good and + * sensor-rail enable do not exist here. The pin macros are deliberately NOT + * defined (RS485-DE pattern): unguarded references fail the build. */ +#define BOARD_HAS_VBAT_SENSE 0 +#define BOARD_HAS_PWR_PG 0 +#define BOARD_HAS_SENS_PWR_EN 0 + #elif CONFIG_BOARD_REV2 /* ------------------------------------------------------------------------ @@ -139,9 +151,22 @@ /* Status LED */ #define BOARD_PIN_STATUS_LED 2 // TODO(SYNC1): final rev2 pin map frozen at hardware sync 1 -/* Buttons (manual watering trigger, WiFi config/AP mode) */ -#define BOARD_PIN_BTN_MANUAL 5 // TODO(SYNC1): final rev2 pin map frozen at hardware sync 1 -#define BOARD_PIN_BTN_CONFIG 18 // TODO(SYNC1): final rev2 pin map frozen at hardware sync 1 +/* Buttons: rev 2 has NONE. The frozen board carries only the BOOT (IO0) and + * RESET (EN) switches plus the status LED — no manual-watering and no + * WiFi-config button. BOARD_PIN_BTN_MANUAL / BOARD_PIN_BTN_CONFIG are + * therefore deliberately NOT defined when their flags are 0: any reference + * that is not guarded by #if BOARD_HAS_BTN_* becomes a compile error instead + * of reading a phantom GPIO (same enforcement pattern as + * BOARD_PIN_RS485_DE / BOARD_PIN_RESERVOIR_PUMP above). + * + * This is a correctness fix, not cosmetics: the pin the profile previously + * used for BTN_CONFIG is IO18 = EXP_SCK on the frozen board — an expansion + * header signal. Reading it at boot could mistake expansion-bus traffic for + * "operator holds the config button". Provisioning on rev2 is entered via + * the credentials-absent path (feature 007); a BOOT-button trigger is a + * possible future feature. */ +#define BOARD_HAS_BTN_MANUAL 0 +#define BOARD_HAS_BTN_CONFIG 0 #else #error "No board selected: enable CONFIG_BOARD_REV1_DEVKIT or CONFIG_BOARD_REV2" @@ -164,9 +189,15 @@ #if BOARD_PIN_LEVEL_LOW == BOARD_PIN_LEVEL_HIGH #error "Board sanity: BOARD_PIN_LEVEL_LOW and BOARD_PIN_LEVEL_HIGH must differ" #endif +/* Buttons are optional hardware (rev2 has none), so the distinctness check + * is guarded exactly like the reservoir pump's: with both flags at 0 the + * undefined macros would otherwise compare 0 == 0 in #if and fire a + * spurious error. */ +#if BOARD_HAS_BTN_MANUAL && BOARD_HAS_BTN_CONFIG #if BOARD_PIN_BTN_MANUAL == BOARD_PIN_BTN_CONFIG #error "Board sanity: BOARD_PIN_BTN_MANUAL and BOARD_PIN_BTN_CONFIG must differ" #endif +#endif /* Pumps must not share a pin with the level sensors */ #if (BOARD_PIN_MAIN_PUMP == BOARD_PIN_LEVEL_LOW) || \ @@ -217,6 +248,21 @@ #error "Board sanity: BOARD_PIN_RESERVOIR_PUMP is defined but BOARD_HAS_RESERVOIR_PUMP is 0" #endif +/* Feature flag consistency: BOARD_HAS_BTN_* == 1 iff the button pin exists + * (rev2 has no buttons — same pattern as RS485 DE) */ +#if BOARD_HAS_BTN_MANUAL && !defined(BOARD_PIN_BTN_MANUAL) +#error "Board sanity: BOARD_HAS_BTN_MANUAL is 1 but BOARD_PIN_BTN_MANUAL is not defined" +#endif +#if !BOARD_HAS_BTN_MANUAL && defined(BOARD_PIN_BTN_MANUAL) +#error "Board sanity: BOARD_PIN_BTN_MANUAL is defined but BOARD_HAS_BTN_MANUAL is 0" +#endif +#if BOARD_HAS_BTN_CONFIG && !defined(BOARD_PIN_BTN_CONFIG) +#error "Board sanity: BOARD_HAS_BTN_CONFIG is 1 but BOARD_PIN_BTN_CONFIG is not defined" +#endif +#if !BOARD_HAS_BTN_CONFIG && defined(BOARD_PIN_BTN_CONFIG) +#error "Board sanity: BOARD_PIN_BTN_CONFIG is defined but BOARD_HAS_BTN_CONFIG is 0" +#endif + /* Feature flag consistency: BOARD_HAS_INA226 == 1 iff the address exists */ #if BOARD_HAS_INA226 && !defined(BOARD_INA226_ADDR) #error "Board sanity: BOARD_HAS_INA226 is 1 but BOARD_INA226_ADDR is not defined" @@ -225,4 +271,45 @@ #error "Board sanity: BOARD_INA226_ADDR is defined but BOARD_HAS_INA226 is 0" #endif +/* Expansion-header reservation — REV 2 ONLY. + * J7 carries VSPI SCK/MOSI/MISO plus CS and IRQ on IO18/19/23/4/27; core + * firmware must never claim one of them, or an attached expansion device + * fights the core (and its bus traffic can be misread as core input). + * This is deliberately NOT a cross-board check: rev1 legitimately uses IO18 + * (config button) and IO27 (reservoir pump) — the reservation is a property + * of the rev2 board, not of the firmware. */ +#if CONFIG_BOARD_REV2 +#define BOARD_PIN_IS_EXPANSION(pin) \ + ((pin) == 18 || (pin) == 19 || (pin) == 23 || (pin) == 4 || (pin) == 27) + +#if BOARD_PIN_IS_EXPANSION(BOARD_PIN_I2C_SDA) || \ + BOARD_PIN_IS_EXPANSION(BOARD_PIN_I2C_SCL) || \ + BOARD_PIN_IS_EXPANSION(BOARD_PIN_RS485_TX) || \ + BOARD_PIN_IS_EXPANSION(BOARD_PIN_RS485_RX) || \ + BOARD_PIN_IS_EXPANSION(BOARD_PIN_MAIN_PUMP) || \ + BOARD_PIN_IS_EXPANSION(BOARD_PIN_LEVEL_LOW) || \ + BOARD_PIN_IS_EXPANSION(BOARD_PIN_LEVEL_HIGH) || \ + BOARD_PIN_IS_EXPANSION(BOARD_PIN_STATUS_LED) +#error "Board sanity: a rev2 core pin lands on the reserved expansion set (IO18/19/23/4/27)" +#endif + +/* Optional pins too — checked under their capability flags so an undefined + * macro is never evaluated. None of these exist on rev2 today; the checks + * exist so a future re-addition cannot silently take an expansion pin. */ +#if BOARD_HAS_RS485_DE && BOARD_PIN_IS_EXPANSION(BOARD_PIN_RS485_DE) +#error "Board sanity: BOARD_PIN_RS485_DE lands on the reserved expansion set" +#endif +#if BOARD_HAS_RESERVOIR_PUMP && BOARD_PIN_IS_EXPANSION(BOARD_PIN_RESERVOIR_PUMP) +#error "Board sanity: BOARD_PIN_RESERVOIR_PUMP lands on the reserved expansion set" +#endif +#if BOARD_HAS_BTN_MANUAL && BOARD_PIN_IS_EXPANSION(BOARD_PIN_BTN_MANUAL) +#error "Board sanity: BOARD_PIN_BTN_MANUAL lands on the reserved expansion set" +#endif +#if BOARD_HAS_BTN_CONFIG && BOARD_PIN_IS_EXPANSION(BOARD_PIN_BTN_CONFIG) +#error "Board sanity: BOARD_PIN_BTN_CONFIG lands on the reserved expansion set" +#endif + +#undef BOARD_PIN_IS_EXPANSION +#endif /* CONFIG_BOARD_REV2 */ + #endif /* WATERINGSYSTEM_BOARD_BOARD_H */ diff --git a/firmware/main/app_main.cpp b/firmware/main/app_main.cpp index 24361af..a66285c 100644 --- a/firmware/main/app_main.cpp +++ b/firmware/main/app_main.cpp @@ -80,6 +80,7 @@ static const char *TAG = "app_main"; +#if BOARD_HAS_BTN_CONFIG // Config-button emergency-provisioning hold (feature 007, US3). Parity // (docs/parity-checklist.md §7): the config button held >= 5 s during startup // forces WiFi provisioning; the status LED blinks every 100 ms while the hold @@ -88,6 +89,7 @@ static const char *TAG = "app_main"; // connect-attempt toggle (that one runs later, from wifi_task.cpp). static constexpr uint32_t kConfigButtonHoldMs = 5000; // hold to force prov. static constexpr uint32_t kConfigButtonBlinkMs = 100; // LED toggle interval +#endif /** * @brief Drive every pump GPIO that exists on this board to a safe OFF @@ -141,10 +143,19 @@ static void pumps_force_off(void) } } +#if BOARD_HAS_BTN_CONFIG /** * @brief Read the config button at boot and confirm a >= 5 s hold (feature * 007, US3/T024/T026). * + * Compiled ONLY on boards that actually have a config button + * (BOARD_HAS_BTN_CONFIG, feature 012 FR-003). The frozen rev2 board has no + * such button, and the pin this path used to read (IO18) is EXP_SCK on that + * board — an expansion-header signal the boot path must never touch, since + * expansion-bus traffic could be misread as a held button. On buttonless + * boards the credentials-absent path is the sole provisioning trigger + * (feature 007), unchanged. + * * The config button (BOARD_PIN_BTN_CONFIG, GPIO18) is wired to GND and read * with an internal pull-up, so it is active LOW: held == logic 0 (parity: the * legacy INPUT_PULLUP idiom, same as the level-sensor inputs). Semantics: @@ -222,6 +233,7 @@ static bool config_button_held_at_boot(void) static_cast(kConfigButtonHoldMs)); return true; } +#endif /* BOARD_HAS_BTN_CONFIG */ extern "C" void app_main(void) { @@ -394,8 +406,17 @@ extern "C" void app_main(void) // hold window. Credential VALUES are never logged: we only test whether an // SSID is present. WiFi never touches the watering path (FR-014); // everything below stays after the pump fail-safe. + // + // Boards without a config button (BOARD_HAS_BTN_CONFIG == 0, e.g. the + // frozen rev2) never take the button branch: the read is compiled out and + // the credentials-absent path is the sole provisioning trigger (feature + // 012 FR-003). No GPIO is claimed for a button that does not exist. const bool wifi_credentials_present = !config.getWifiSsid().empty(); +#if BOARD_HAS_BTN_CONFIG const bool config_button_held = config_button_held_at_boot(); +#else + const bool config_button_held = false; +#endif const WifiBootMode wifi_boot_mode = decideBootMode(wifi_credentials_present, config_button_held); diff --git a/firmware/test_apps/host/main/test_board_contract_rev1.cpp b/firmware/test_apps/host/main/test_board_contract_rev1.cpp index c41bbef..b6138c7 100644 --- a/firmware/test_apps/host/main/test_board_contract_rev1.cpp +++ b/firmware/test_apps/host/main/test_board_contract_rev1.cpp @@ -53,3 +53,29 @@ static_assert(BOARD_LEVEL_SETTLE_MS == 0, "rev1 board contract: no settle gating (rail always on)"); static_assert(BOARD_LEVEL_DEBOUNCE_MS == 300, "rev1 board contract: 300 ms debounce window"); + +// Buttons: the rev1 devkit rig HAS both buttons, and feature 012 must not +// change a single rev1 value (FR-002 regression guard). The manual-watering +// button sits on IO5, the WiFi-config button on IO18 — the pins the boot +// path reads today (feature 007, parity checklist §7). +// +// NOTE for future readers: feature 012 reserves IO18/19/23/4/27 for the +// rev2 expansion header J7, and rev1 legitimately uses two of them (IO18 +// config button, IO27 reservoir pump). That reservation is a rev2-ONLY +// invariant (data-model.md invariant 3) — never "fix" the rev1 values to +// satisfy it. +static_assert(BOARD_HAS_BTN_MANUAL == 1, + "rev1 board contract: manual button present on the devkit rig"); +#ifndef BOARD_PIN_BTN_MANUAL +#error "rev1 board contract: BOARD_PIN_BTN_MANUAL must be defined" +#endif +static_assert(BOARD_PIN_BTN_MANUAL == 5, + "rev1 board contract: manual button pin unchanged (FR-002)"); +static_assert(BOARD_HAS_BTN_CONFIG == 1, + "rev1 board contract: config button present on the devkit rig"); +#ifndef BOARD_PIN_BTN_CONFIG +#error "rev1 board contract: BOARD_PIN_BTN_CONFIG must be defined" +#endif +static_assert(BOARD_PIN_BTN_CONFIG == 18, + "rev1 board contract: config button pin unchanged (FR-002, " + "feature 007 boot provisioning path)"); diff --git a/firmware/test_apps/host/main/test_board_contract_rev2.cpp b/firmware/test_apps/host/main/test_board_contract_rev2.cpp index 9ea1284..aae8be3 100644 --- a/firmware/test_apps/host/main/test_board_contract_rev2.cpp +++ b/firmware/test_apps/host/main/test_board_contract_rev2.cpp @@ -52,3 +52,48 @@ static_assert(BOARD_HAS_RS485_DE == 0, #ifdef BOARD_PIN_RS485_DE #error "rev2 board contract: BOARD_PIN_RS485_DE must NOT be defined" #endif + +// Buttons: the frozen rev2 board has NO manual or config button — only the +// BOOT/RESET switches (IO0/EN) and the status LED (feature 012, FR-001). +// Both pin macros are therefore removed, so the boot path's button block +// cannot compile on this board (the same enforcement pattern as the +// reservoir pump above). This matters beyond tidiness: the pin the old +// profile used for BTN_CONFIG is IO18 = EXP_SCK, an expansion-header signal +// the boot path must never read. +static_assert(BOARD_HAS_BTN_MANUAL == 0, + "rev2 board contract: no manual button on the frozen rev2 board"); +#ifdef BOARD_PIN_BTN_MANUAL +#error "rev2 board contract: BOARD_PIN_BTN_MANUAL must NOT be defined \ +(unguarded references must fail the build)" +#endif +static_assert(BOARD_HAS_BTN_CONFIG == 0, + "rev2 board contract: no config button on the frozen rev2 board"); +#ifdef BOARD_PIN_BTN_CONFIG +#error "rev2 board contract: BOARD_PIN_BTN_CONFIG must NOT be defined \ +(unguarded references must fail the build)" +#endif + +// Expansion reservation (feature 012, FR-004). J7 carries VSPI +// SCK/MOSI/MISO plus CS and IRQ on IO18/19/23/4/27; core firmware must not +// claim any of them on rev2. board.h enforces this too — this TU is the +// belt to that header's braces: the contract survives even if the header +// check is ever removed. NOTE: this is a rev2-ONLY invariant; rev1 +// legitimately uses IO18 (config button) and IO27 (reservoir pump). +#define WS_REV2_NOT_EXPANSION(pin) \ + ((pin) != 18 && (pin) != 19 && (pin) != 23 && (pin) != 4 && (pin) != 27) + +static_assert(WS_REV2_NOT_EXPANSION(BOARD_PIN_I2C_SDA) && + WS_REV2_NOT_EXPANSION(BOARD_PIN_I2C_SCL), + "rev2 board contract: I2C pins must stay off the expansion set"); +static_assert(WS_REV2_NOT_EXPANSION(BOARD_PIN_RS485_TX) && + WS_REV2_NOT_EXPANSION(BOARD_PIN_RS485_RX), + "rev2 board contract: RS485 pins must stay off the expansion set"); +static_assert(WS_REV2_NOT_EXPANSION(BOARD_PIN_MAIN_PUMP), + "rev2 board contract: pump pin must stay off the expansion set"); +static_assert(WS_REV2_NOT_EXPANSION(BOARD_PIN_LEVEL_LOW) && + WS_REV2_NOT_EXPANSION(BOARD_PIN_LEVEL_HIGH), + "rev2 board contract: level pins must stay off the expansion set"); +static_assert(WS_REV2_NOT_EXPANSION(BOARD_PIN_STATUS_LED), + "rev2 board contract: status LED must stay off the expansion set"); + +#undef WS_REV2_NOT_EXPANSION diff --git a/specs/012-rev2-pin-map/tasks.md b/specs/012-rev2-pin-map/tasks.md index 885b1b5..f8f5121 100644 --- a/specs/012-rev2-pin-map/tasks.md +++ b/specs/012-rev2-pin-map/tasks.md @@ -14,7 +14,7 @@ build failure, which is the intended TDD signal for compile-time contracts. ## Phase 1: Setup -- [ ] T001 Record the green baseline: run the host test suite and both board-target builds per `specs/012-rev2-pin-map/quickstart.md` §1–2 on the unmodified branch; note results in the task log. A pre-existing red here is a STOP — this feature must start from green. +- [x] T001 Record the green baseline: run the host test suite and both board-target builds per `specs/012-rev2-pin-map/quickstart.md` §1–2 on the unmodified branch; note results in the task log. A pre-existing red here is a STOP — this feature must start from green. ## Phase 2: Foundational @@ -29,12 +29,12 @@ boot), guard the boot provisioning button path, reserve the expansion set. core pin collides with the expansion set; app_main compiles for rev2 without the button block and for rev1 with it, unchanged. -- [ ] T002 [US1] Extend `firmware/test_apps/host/main/test_board_contract_rev2.cpp`: assert `BOARD_HAS_BTN_MANUAL == 0` and `BOARD_HAS_BTN_CONFIG == 0`; `#ifdef BOARD_PIN_BTN_MANUAL/`BOARD_PIN_BTN_CONFIG` → `#error`; add static_asserts that every defined core `BOARD_PIN_*` (I2C, RS485, pump, levels, LED — and the US2 signals once they exist) differs from each of 18, 19, 23, 4, 27. Expect the build to go red (pins still defined). -- [ ] T003 [US1] Extend `firmware/test_apps/host/main/test_board_contract_rev1.cpp`: assert `BOARD_HAS_BTN_MANUAL == 1`, `BOARD_PIN_BTN_MANUAL == 5`, `BOARD_HAS_BTN_CONFIG == 1`, `BOARD_PIN_BTN_CONFIG == 18` — pins rev1 genuinely has, values frozen (FR-002 regression guard). Document in the TU comment that the expansion reservation is rev2-only (rev1 legitimately uses 18/27 — data-model invariant 3). -- [ ] T004 [US1] Edit `firmware/components/board/include/board/board.h`: add `BOARD_HAS_BTN_MANUAL 1` / `BOARD_HAS_BTN_CONFIG 1` to the rev1 section above the existing pins; in the rev2 section set both flags 0 and DELETE both pin defines (deliberate-undefine pattern, comment mirroring the RS485_DE/reservoir-pump wording). -- [ ] T005 [US1] Edit `firmware/components/board/include/board/board.h` sanity section: add flag↔pin consistency `#error` pairs for both button flags (pattern of lines 211–217); add the rev2-only expansion-reservation check (`#if`-chain erroring if any defined core pin ∈ {18,19,23,4,27}), placed inside a `CONFIG_BOARD_REV2` conditional per data-model invariant 3. -- [ ] T006 [US1] Edit `firmware/main/app_main.cpp`: wrap the config-button provisioning block (input `gpio_config` of `BOARD_PIN_BTN_CONFIG`, the poll loop and its STATUS_LED feedback, approx. lines 148–217) in `#if BOARD_HAS_BTN_CONFIG`; the credentials-absent provisioning path and all pump boot-safety code stay untouched. Update the block comment to state why (no button on rev2; IO18 is EXP_SCK). -- [ ] T007 [US1] Validate US1: host suite green, both board targets build green (quickstart §1–2). +- [x] T002 [US1] Extend `firmware/test_apps/host/main/test_board_contract_rev2.cpp`: assert `BOARD_HAS_BTN_MANUAL == 0` and `BOARD_HAS_BTN_CONFIG == 0`; `#ifdef BOARD_PIN_BTN_MANUAL/`BOARD_PIN_BTN_CONFIG` → `#error`; add static_asserts that every defined core `BOARD_PIN_*` (I2C, RS485, pump, levels, LED — and the US2 signals once they exist) differs from each of 18, 19, 23, 4, 27. Expect the build to go red (pins still defined). +- [x] T003 [US1] Extend `firmware/test_apps/host/main/test_board_contract_rev1.cpp`: assert `BOARD_HAS_BTN_MANUAL == 1`, `BOARD_PIN_BTN_MANUAL == 5`, `BOARD_HAS_BTN_CONFIG == 1`, `BOARD_PIN_BTN_CONFIG == 18` — pins rev1 genuinely has, values frozen (FR-002 regression guard). Document in the TU comment that the expansion reservation is rev2-only (rev1 legitimately uses 18/27 — data-model invariant 3). +- [x] T004 [US1] Edit `firmware/components/board/include/board/board.h`: add `BOARD_HAS_BTN_MANUAL 1` / `BOARD_HAS_BTN_CONFIG 1` to the rev1 section above the existing pins; in the rev2 section set both flags 0 and DELETE both pin defines (deliberate-undefine pattern, comment mirroring the RS485_DE/reservoir-pump wording). +- [x] T005 [US1] Edit `firmware/components/board/include/board/board.h` sanity section: add flag↔pin consistency `#error` pairs for both button flags (pattern of lines 211–217); add the rev2-only expansion-reservation check (`#if`-chain erroring if any defined core pin ∈ {18,19,23,4,27}), placed inside a `CONFIG_BOARD_REV2` conditional per data-model invariant 3. +- [x] T006 [US1] Edit `firmware/main/app_main.cpp`: wrap the config-button provisioning block (input `gpio_config` of `BOARD_PIN_BTN_CONFIG`, the poll loop and its STATUS_LED feedback, approx. lines 148–217) in `#if BOARD_HAS_BTN_CONFIG`; the credentials-absent provisioning path and all pump boot-safety code stay untouched. Update the block comment to state why (no button on rev2; IO18 is EXP_SCK). +- [x] T007 [US1] Validate US1: host suite green, both board targets build green (quickstart §1–2). **Checkpoint**: US1 delivers the safety/correctness fix on its own. From 9f7be167bd33aba3167cad12a1a0e6a48f23a3ed Mon Sep 17 00:00:00 2001 From: Cryptotomte Date: Wed, 12 Aug 2026 15:13:32 +0200 Subject: [PATCH 3/7] feat(board): state the rev2 power and rail signals in the profile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The frozen rev2 design provides three signals the board profile did not mention, forcing the next developer back to the schematic (project rule: pin tables live only in components/board/). - BOARD_HAS_VBAT_SENSE / BOARD_PIN_VBAT_SENSE 34 — battery divider on an input-only ADC1 pin (ADC2 is unusable with WiFi up); readings compress above ~2.45 V at the pin, FW-1. - BOARD_HAS_PWR_PG / BOARD_PIN_PWR_PG 35 — buck power-good, input-only, open-drain with an external pull-up; IO34-39 have no internal pulls, FW-6. - BOARD_HAS_SENS_PWR_EN / BOARD_PIN_SENS_PWR_EN 25 — switched 12 V sensor domain. The rail is OFF by HARDWARE default (R61 holds the gate off while the GPIO is hi-Z), so this feature deliberately does not drive the pin; rail sequencing and the FW-3 settle are PR-14 scope. All three carry the flag<->pin consistency pattern: rev1 defines the flags at 0 with the pin macros absent, so an unguarded PR-14 reference fails the rev1 build. Contract TUs pin both sides, and the new pins join the rev2 expansion-disjointness checks. Spec: specs/012-rev2-pin-map (US2, FR-005, FR-007) Co-Authored-By: Claude Opus 5 --- .../components/board/include/board/board.h | 55 ++++++++++++++++++- .../host/main/test_board_contract_rev1.cpp | 22 ++++++++ .../host/main/test_board_contract_rev2.cpp | 33 +++++++++++ specs/012-rev2-pin-map/tasks.md | 8 +-- 4 files changed, 113 insertions(+), 5 deletions(-) diff --git a/firmware/components/board/include/board/board.h b/firmware/components/board/include/board/board.h index 911cab9..b755d57 100644 --- a/firmware/components/board/include/board/board.h +++ b/firmware/components/board/include/board/board.h @@ -151,6 +151,34 @@ /* Status LED */ #define BOARD_PIN_STATUS_LED 2 // TODO(SYNC1): final rev2 pin map frozen at hardware sync 1 +/* Power and rail monitoring — rev 2 only. + * + * VBAT_SENSE (IO34): battery voltage through the 470 k/100 k divider. + * Input-only pin on ADC1 — mandatory, since ADC2 is unusable while WiFi is + * active. Readings compress above ~2.45 V at the pin (~14 V battery): the + * ESP32 11 dB linear range ends there, so top-of-charge accuracy is reduced + * (FW-1). Telemetry and soft-UVLO only — no safety decision depends on it. + * + * PWR_PG (IO35): 3V3 buck power-good, HIGH = in regulation. Input-only and + * open-drain with an external pull-up (R30) — configure as a plain input; + * IO34-39 have no internal pulls at all (FW-6). + * + * SENS_PWR_EN (IO25): enables the switched 12 V sensor domain (Q60 gate, + * also the THVD1426 SHDN-bar). The rail is OFF by HARDWARE default — the + * R61 gate pull-up holds the high-side switch off while the GPIO is hi-Z, + * so no firmware action is needed to keep it off at boot, and this feature + * deliberately does NOT drive the pin. Rail sequencing (assert, the 500 ms + * XKC-Y26 settle before level reads are trustworthy per FW-3, and the IO17 + * pull-up rule of FW-2 while the domain is off) is PR-14 scope. + * + * Consumers of all three land in PR-14; the profile only states the facts. */ +#define BOARD_HAS_VBAT_SENSE 1 +#define BOARD_PIN_VBAT_SENSE 34 +#define BOARD_HAS_PWR_PG 1 +#define BOARD_PIN_PWR_PG 35 +#define BOARD_HAS_SENS_PWR_EN 1 +#define BOARD_PIN_SENS_PWR_EN 25 + /* Buttons: rev 2 has NONE. The frozen board carries only the BOOT (IO0) and * RESET (EN) switches plus the status LED — no manual-watering and no * WiFi-config button. BOARD_PIN_BTN_MANUAL / BOARD_PIN_BTN_CONFIG are @@ -263,6 +291,28 @@ #error "Board sanity: BOARD_PIN_BTN_CONFIG is defined but BOARD_HAS_BTN_CONFIG is 0" #endif +/* Feature flag consistency: the rev2-only power/rail signals. Same pattern — + * on boards without them the pin macros stay undefined so a PR-14 driver + * cannot reference them unguarded. */ +#if BOARD_HAS_VBAT_SENSE && !defined(BOARD_PIN_VBAT_SENSE) +#error "Board sanity: BOARD_HAS_VBAT_SENSE is 1 but BOARD_PIN_VBAT_SENSE is not defined" +#endif +#if !BOARD_HAS_VBAT_SENSE && defined(BOARD_PIN_VBAT_SENSE) +#error "Board sanity: BOARD_PIN_VBAT_SENSE is defined but BOARD_HAS_VBAT_SENSE is 0" +#endif +#if BOARD_HAS_PWR_PG && !defined(BOARD_PIN_PWR_PG) +#error "Board sanity: BOARD_HAS_PWR_PG is 1 but BOARD_PIN_PWR_PG is not defined" +#endif +#if !BOARD_HAS_PWR_PG && defined(BOARD_PIN_PWR_PG) +#error "Board sanity: BOARD_PIN_PWR_PG is defined but BOARD_HAS_PWR_PG is 0" +#endif +#if BOARD_HAS_SENS_PWR_EN && !defined(BOARD_PIN_SENS_PWR_EN) +#error "Board sanity: BOARD_HAS_SENS_PWR_EN is 1 but BOARD_PIN_SENS_PWR_EN is not defined" +#endif +#if !BOARD_HAS_SENS_PWR_EN && defined(BOARD_PIN_SENS_PWR_EN) +#error "Board sanity: BOARD_PIN_SENS_PWR_EN is defined but BOARD_HAS_SENS_PWR_EN is 0" +#endif + /* Feature flag consistency: BOARD_HAS_INA226 == 1 iff the address exists */ #if BOARD_HAS_INA226 && !defined(BOARD_INA226_ADDR) #error "Board sanity: BOARD_HAS_INA226 is 1 but BOARD_INA226_ADDR is not defined" @@ -289,7 +339,10 @@ BOARD_PIN_IS_EXPANSION(BOARD_PIN_MAIN_PUMP) || \ BOARD_PIN_IS_EXPANSION(BOARD_PIN_LEVEL_LOW) || \ BOARD_PIN_IS_EXPANSION(BOARD_PIN_LEVEL_HIGH) || \ - BOARD_PIN_IS_EXPANSION(BOARD_PIN_STATUS_LED) + BOARD_PIN_IS_EXPANSION(BOARD_PIN_STATUS_LED) || \ + BOARD_PIN_IS_EXPANSION(BOARD_PIN_VBAT_SENSE) || \ + BOARD_PIN_IS_EXPANSION(BOARD_PIN_PWR_PG) || \ + BOARD_PIN_IS_EXPANSION(BOARD_PIN_SENS_PWR_EN) #error "Board sanity: a rev2 core pin lands on the reserved expansion set (IO18/19/23/4/27)" #endif diff --git a/firmware/test_apps/host/main/test_board_contract_rev1.cpp b/firmware/test_apps/host/main/test_board_contract_rev1.cpp index b6138c7..78ad089 100644 --- a/firmware/test_apps/host/main/test_board_contract_rev1.cpp +++ b/firmware/test_apps/host/main/test_board_contract_rev1.cpp @@ -79,3 +79,25 @@ static_assert(BOARD_HAS_BTN_CONFIG == 1, static_assert(BOARD_PIN_BTN_CONFIG == 18, "rev1 board contract: config button pin unchanged (FR-002, " "feature 007 boot provisioning path)"); + +// Power / rail signals: rev2-only hardware (feature 012, FR-005). The devkit +// rig runs off USB with a permanently powered sensor rail, so battery sense, +// buck power-good and sensor-rail enable do not exist here — flags 0 and pin +// macros undefined, so a PR-14 driver that references them without an +// #if BOARD_HAS_* guard fails the rev1 build instead of driving a phantom +// GPIO (the RS485-DE enforcement pattern). +static_assert(BOARD_HAS_VBAT_SENSE == 0, + "rev1 board contract: no battery voltage sense on the devkit rig"); +#ifdef BOARD_PIN_VBAT_SENSE +#error "rev1 board contract: BOARD_PIN_VBAT_SENSE must NOT be defined" +#endif +static_assert(BOARD_HAS_PWR_PG == 0, + "rev1 board contract: no buck power-good on the devkit rig"); +#ifdef BOARD_PIN_PWR_PG +#error "rev1 board contract: BOARD_PIN_PWR_PG must NOT be defined" +#endif +static_assert(BOARD_HAS_SENS_PWR_EN == 0, + "rev1 board contract: sensor rail is permanently on (no switch)"); +#ifdef BOARD_PIN_SENS_PWR_EN +#error "rev1 board contract: BOARD_PIN_SENS_PWR_EN must NOT be defined" +#endif diff --git a/firmware/test_apps/host/main/test_board_contract_rev2.cpp b/firmware/test_apps/host/main/test_board_contract_rev2.cpp index aae8be3..11fa159 100644 --- a/firmware/test_apps/host/main/test_board_contract_rev2.cpp +++ b/firmware/test_apps/host/main/test_board_contract_rev2.cpp @@ -73,6 +73,34 @@ static_assert(BOARD_HAS_BTN_CONFIG == 0, (unguarded references must fail the build)" #endif +// Power / rail signals the frozen rev2 board provides (feature 012, +// FR-005). Consumers (ADC calibration, PG monitoring, rail sequencing) are +// PR-14 scope; the profile only has to state the facts. +static_assert(BOARD_HAS_VBAT_SENSE == 1, + "rev2 board contract: battery voltage sense present"); +#ifndef BOARD_PIN_VBAT_SENSE +#error "rev2 board contract: BOARD_PIN_VBAT_SENSE must be defined" +#endif +static_assert(BOARD_PIN_VBAT_SENSE == 34, + "rev2 board contract: VBAT_SENSE on IO34 (ADC1_CH6, input-only)"); + +static_assert(BOARD_HAS_PWR_PG == 1, + "rev2 board contract: buck power-good present"); +#ifndef BOARD_PIN_PWR_PG +#error "rev2 board contract: BOARD_PIN_PWR_PG must be defined" +#endif +static_assert(BOARD_PIN_PWR_PG == 35, + "rev2 board contract: PWR_PG on IO35 (input-only, ext. pull-up)"); + +static_assert(BOARD_HAS_SENS_PWR_EN == 1, + "rev2 board contract: switched sensor rail present"); +#ifndef BOARD_PIN_SENS_PWR_EN +#error "rev2 board contract: BOARD_PIN_SENS_PWR_EN must be defined" +#endif +static_assert(BOARD_PIN_SENS_PWR_EN == 25, + "rev2 board contract: SENS_PWR_EN on IO25 (output, rail OFF " + "by hardware default)"); + // Expansion reservation (feature 012, FR-004). J7 carries VSPI // SCK/MOSI/MISO plus CS and IRQ on IO18/19/23/4/27; core firmware must not // claim any of them on rev2. board.h enforces this too — this TU is the @@ -95,5 +123,10 @@ static_assert(WS_REV2_NOT_EXPANSION(BOARD_PIN_LEVEL_LOW) && "rev2 board contract: level pins must stay off the expansion set"); static_assert(WS_REV2_NOT_EXPANSION(BOARD_PIN_STATUS_LED), "rev2 board contract: status LED must stay off the expansion set"); +static_assert(WS_REV2_NOT_EXPANSION(BOARD_PIN_VBAT_SENSE) && + WS_REV2_NOT_EXPANSION(BOARD_PIN_PWR_PG) && + WS_REV2_NOT_EXPANSION(BOARD_PIN_SENS_PWR_EN), + "rev2 board contract: power/rail pins must stay off the " + "expansion set"); #undef WS_REV2_NOT_EXPANSION diff --git a/specs/012-rev2-pin-map/tasks.md b/specs/012-rev2-pin-map/tasks.md index f8f5121..b66133e 100644 --- a/specs/012-rev2-pin-map/tasks.md +++ b/specs/012-rev2-pin-map/tasks.md @@ -46,10 +46,10 @@ capability flags. **Independent test**: rev2 TU asserts the three pins/flags; rev1 TU proves unguarded references cannot compile on rev1. -- [ ] T008 [US2] Extend `firmware/test_apps/host/main/test_board_contract_rev2.cpp`: assert `BOARD_HAS_VBAT_SENSE == 1` / `BOARD_PIN_VBAT_SENSE == 34`, `BOARD_HAS_PWR_PG == 1` / `BOARD_PIN_PWR_PG == 35`, `BOARD_HAS_SENS_PWR_EN == 1` / `BOARD_PIN_SENS_PWR_EN == 25`; fold the three pins into the T002 expansion-disjointness asserts. Red until T010. -- [ ] T009 [US2] Extend `firmware/test_apps/host/main/test_board_contract_rev1.cpp`: assert all three flags are `== 0` and pins undefined (`#ifdef` → `#error`) on rev1. -- [ ] T010 [US2] Edit `firmware/components/board/include/board/board.h`: rev1 section — three flags at 0; rev2 section — three flags at 1 with pins 34/35/25 and constraint comments (IO34 input-only ADC1, ADC2 dead with WiFi, FW-1 pointer; IO35 input-only, open-drain + external pull-up, no internal pulls IO34–39, FW-6; IO25 output, sensor rail OFF default, FW-3 settle owned by PR-14). Add the three flag↔pin sanity pairs; confirm the pin-distinctness checks still pass (25 = RS485_DE on rev1 vs SENS_PWR_EN on rev2 — never same target). -- [ ] T011 [US2] Validate US2: host suite green, both board targets build green. +- [x] T008 [US2] Extend `firmware/test_apps/host/main/test_board_contract_rev2.cpp`: assert `BOARD_HAS_VBAT_SENSE == 1` / `BOARD_PIN_VBAT_SENSE == 34`, `BOARD_HAS_PWR_PG == 1` / `BOARD_PIN_PWR_PG == 35`, `BOARD_HAS_SENS_PWR_EN == 1` / `BOARD_PIN_SENS_PWR_EN == 25`; fold the three pins into the T002 expansion-disjointness asserts. Red until T010. +- [x] T009 [US2] Extend `firmware/test_apps/host/main/test_board_contract_rev1.cpp`: assert all three flags are `== 0` and pins undefined (`#ifdef` → `#error`) on rev1. +- [x] T010 [US2] Edit `firmware/components/board/include/board/board.h`: rev1 section — three flags at 0; rev2 section — three flags at 1 with pins 34/35/25 and constraint comments (IO34 input-only ADC1, ADC2 dead with WiFi, FW-1 pointer; IO35 input-only, open-drain + external pull-up, no internal pulls IO34–39, FW-6; IO25 output, sensor rail OFF default, FW-3 settle owned by PR-14). Add the three flag↔pin sanity pairs; confirm the pin-distinctness checks still pass (25 = RS485_DE on rev1 vs SENS_PWR_EN on rev2 — never same target). +- [x] T011 [US2] Validate US2: host suite green, both board targets build green. **Checkpoint**: PR-14 driver work now has every frozen fact available. From 7c52acc7c3cf04725aa72683845fd2d6bd78c3a2 Mon Sep 17 00:00:00 2001 From: Cryptotomte Date: Wed, 12 Aug 2026 15:16:53 +0200 Subject: [PATCH 4/7] docs(board): replace the stale SYNC1 markers with frozen-map citations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SYNC 1 has happened (2026-08-12: all sheets drawn, ERC clean, components ordered) and every rev2 value in the profile matches the frozen contract, so the 11 TODO(SYNC1) markers and the "GPIO numbers provisionally mirror rev 1" header were actively misleading — they invite edits to values that are now fixed by hardware. - Each rev2 pin group cites hardware/rev2/design-notes/02-mcu.md §2.2 SYNC 1 map (frozen 2026-08-12) once, at group level. - I2C address map corrected: 0x41 is the solar INA226, POPULATED on this node per the 2026-08-12 decision (it was documented as a DNP footprint); driver support is PR-14. BME280 is 0x77 (SDO -> VDDIO, rev1 parity), and the note now says BOARD_INA226_ADDR names the pump monitor. - The rev2 contract TU's copy of the old DNP claim is corrected too. No macro values change in this commit. Spec: specs/012-rev2-pin-map (US3, FR-006, SC-004) Co-Authored-By: Claude Opus 5 --- .../components/board/include/board/board.h | 54 +++++++++++-------- .../host/main/test_board_contract_rev2.cpp | 7 +-- specs/012-rev2-pin-map/tasks.md | 4 +- 3 files changed, 39 insertions(+), 26 deletions(-) diff --git a/firmware/components/board/include/board/board.h b/firmware/components/board/include/board/board.h index b755d57..0c68cc5 100644 --- a/firmware/components/board/include/board/board.h +++ b/firmware/components/board/include/board/board.h @@ -92,27 +92,31 @@ /* ------------------------------------------------------------------------ * Rev 2 — custom PCB (THVD1426 auto-direction RS485, INA226, CP2102N). - * GPIO numbers provisionally mirror rev 1 until the rev 2 pin map is frozen. + * Every GPIO number below is FROZEN: it comes from the SYNC 1 map in + * hardware/rev2/design-notes/02-mcu.md §2.2 (frozen 2026-08-12, all sheets + * drawn, ERC clean, components ordered). Changing one here changes nothing + * on the board — pin changes go schematic-first and re-open this profile + * deliberately. * ------------------------------------------------------------------------ */ #define BOARD_NAME "rev2" -/* I2C bus (BME280, INA226) */ -#define BOARD_PIN_I2C_SDA 21 // TODO(SYNC1): final rev2 pin map frozen at hardware sync 1 -#define BOARD_PIN_I2C_SCL 22 // TODO(SYNC1): final rev2 pin map frozen at hardware sync 1 +/* I2C bus (BME280, INA226 x2) — 02-mcu.md §2.2 SYNC 1 map + * (frozen 2026-08-12). */ +#define BOARD_PIN_I2C_SDA 21 +#define BOARD_PIN_I2C_SCL 22 -/* RS485 (THVD1426 with automatic direction control — no DE pin). - * UART pins provisionally mirror rev 1 (TX=16/RX=17 per src/main.cpp). */ -#define BOARD_PIN_RS485_TX 16 // TODO(SYNC1): final rev2 pin map frozen at hardware sync 1 -#define BOARD_PIN_RS485_RX 17 // TODO(SYNC1): final rev2 pin map frozen at hardware sync 1 +/* RS485 (THVD1426 with automatic direction control — no DE pin) — + * 02-mcu.md §2.2 SYNC 1 map (frozen 2026-08-12). */ +#define BOARD_PIN_RS485_TX 16 +#define BOARD_PIN_RS485_RX 17 #define BOARD_HAS_RS485_DE 0 /* BOARD_PIN_RS485_DE is deliberately NOT defined when BOARD_HAS_RS485_DE * is 0: any reference that is not guarded by #if BOARD_HAS_RS485_DE becomes * a compile error instead of undefined behavior (e.g. 1ULL << -1) or a * silently dropped ESP_ERR_INVALID_ARG at runtime. */ /* Modbus RTU runs on UART2 at 9600 baud 8N1 (parity: legacy Serial2, - * docs/parity-checklist.md §5). The UART number is a parity fact, not part - * of the provisional rev2 pin map — no TODO(SYNC1). */ + * docs/parity-checklist.md §5) — a parity fact, not a pin-map fact. */ #define BOARD_RS485_UART_PORT 2 /* Pumps (MOSFET gates, active high). @@ -121,8 +125,9 @@ * defined when BOARD_HAS_RESERVOIR_PUMP is 0: any reference that is not * guarded by #if BOARD_HAS_RESERVOIR_PUMP becomes a compile error instead * of driving a phantom GPIO (same enforcement pattern as - * BOARD_PIN_RS485_DE above). */ -#define BOARD_PIN_MAIN_PUMP 26 // TODO(SYNC1): final rev2 pin map frozen at hardware sync 1 + * BOARD_PIN_RS485_DE above). + * Pin: 02-mcu.md §2.2 SYNC 1 map (frozen 2026-08-12). */ +#define BOARD_PIN_MAIN_PUMP 26 #define BOARD_HAS_RESERVOIR_PUMP 0 /* Reservoir level sensors (XKC-Y26), low/high mark with internal pull-ups @@ -132,26 +137,33 @@ * active LOW (water present = LOW) — FW-5. See PRD FR5. * Settle: the XKC-Y26 needs ≥500 ms after its rail powers on before the * output is trustworthy (FW-3); rail control itself arrives in PR-14 — - * this feature arms the gate once at boot. */ -#define BOARD_PIN_LEVEL_LOW 32 // TODO(SYNC1): final rev2 pin map frozen at hardware sync 1 -#define BOARD_PIN_LEVEL_HIGH 33 // TODO(SYNC1): final rev2 pin map frozen at hardware sync 1 + * this feature arms the gate once at boot. + * Pins: 02-mcu.md §2.2 SYNC 1 map (frozen 2026-08-12). */ +#define BOARD_PIN_LEVEL_LOW 32 +#define BOARD_PIN_LEVEL_HIGH 33 #define BOARD_LEVEL_ACTIVE_LOW 1 #define BOARD_LEVEL_DEBOUNCE_MS 300 #define BOARD_LEVEL_SETTLE_MS 500 /* Pump current monitoring (INA226 on the shared I2C bus). - * Rev 2 I2C address map (design notes §5.2.2): + * Rev 2 I2C address map (07-i2c-env.md §7.4, frozen 2026-08-12): * 0x40 INA226 pump monitor (A0 = A1 = GND) - * 0x41 reserved — solar-input INA226 footprint, DNP - * 0x76 / 0x77 BME280 - * The ALERT pin is not connected — no Mask/Enable/Alert register use. */ + * 0x41 INA226 solar/panel telemetry (A0 → VS) — populated on this node + * (decision 2026-08-12; driver support PR-14) + * 0x77 BME280 (SDO → VDDIO, rev1 parity; the driver still probes + * 0x76 first and settles on whichever answers) + * The ALERT pin is not connected — no Mask/Enable/Alert register use. + * BOARD_INA226_ADDR names the PUMP monitor; the solar device gets its own + * constant when PR-14 adds the second driver instance. */ #define BOARD_HAS_INA226 1 #define BOARD_INA226_ADDR 0x40 -/* Status LED */ -#define BOARD_PIN_STATUS_LED 2 // TODO(SYNC1): final rev2 pin map frozen at hardware sync 1 +/* Status LED — 02-mcu.md §2.2 SYNC 1 map (frozen 2026-08-12). */ +#define BOARD_PIN_STATUS_LED 2 /* Power and rail monitoring — rev 2 only. + * Pins: 02-mcu.md §2.2 SYNC 1 map (frozen 2026-08-12); electrical + * constraints: docs/rev2-firmware-notes.md FW-1/FW-3/FW-6. * * VBAT_SENSE (IO34): battery voltage through the 470 k/100 k divider. * Input-only pin on ADC1 — mandatory, since ADC2 is unusable while WiFi is diff --git a/firmware/test_apps/host/main/test_board_contract_rev2.cpp b/firmware/test_apps/host/main/test_board_contract_rev2.cpp index 11fa159..0e94d2e 100644 --- a/firmware/test_apps/host/main/test_board_contract_rev2.cpp +++ b/firmware/test_apps/host/main/test_board_contract_rev2.cpp @@ -25,9 +25,10 @@ static_assert(BOARD_HAS_RESERVOIR_PUMP == 0, (unguarded references must fail the build)" #endif -// rev2 carries the pump INA226 at 0x40 (A0 = A1 = GND; 0x41 reserved for -// the DNP solar footprint, 0x76/0x77 BME280 — the board-profile address -// map). +// rev2 carries the pump INA226 at 0x40 (A0 = A1 = GND). The board-profile +// address map also lists 0x41 (solar INA226 — populated on this node since +// the 2026-08-12 decision; its driver lands in PR-14) and 0x77 (BME280). +// BOARD_INA226_ADDR names the PUMP monitor specifically. static_assert(BOARD_HAS_INA226 == 1, "rev2 board contract: INA226 pump monitor present"); #ifndef BOARD_INA226_ADDR diff --git a/specs/012-rev2-pin-map/tasks.md b/specs/012-rev2-pin-map/tasks.md index b66133e..39c09df 100644 --- a/specs/012-rev2-pin-map/tasks.md +++ b/specs/012-rev2-pin-map/tasks.md @@ -59,8 +59,8 @@ unguarded references cannot compile on rev1. **Independent test**: quickstart §3 grep returns nothing. -- [ ] T012 [US3] Edit `firmware/components/board/include/board/board.h`: delete all 11 `TODO(SYNC1)` markers and the "GPIO numbers provisionally mirror rev 1…" header sentence; each rev2 pin group cites `hardware/rev2/design-notes/02-mcu.md §2.2 SYNC 1 map (frozen 2026-08-12)` once, group-level not per-line; update the I²C address-map comment: `0x41 solar INA226 — populated on this node (decision 2026-08-12; driver support PR-14)`, BME280 = 0x77. -- [ ] T013 [US3] Validate US3: `grep -rn "TODO(SYNC1)\|provisionally" firmware/components/board/` returns nothing (quickstart §3); host suite + both targets still green. +- [x] T012 [US3] Edit `firmware/components/board/include/board/board.h`: delete all 11 `TODO(SYNC1)` markers and the "GPIO numbers provisionally mirror rev 1…" header sentence; each rev2 pin group cites `hardware/rev2/design-notes/02-mcu.md §2.2 SYNC 1 map (frozen 2026-08-12)` once, group-level not per-line; update the I²C address-map comment: `0x41 solar INA226 — populated on this node (decision 2026-08-12; driver support PR-14)`, BME280 = 0x77. +- [x] T013 [US3] Validate US3: `grep -rn "TODO(SYNC1)\|provisionally" firmware/components/board/` returns nothing (quickstart §3); host suite + both targets still green. ## Phase 6: Polish & cross-cutting From 1ddcd6be4f97e1a6822c0a31c17929650e0f6bcd Mon Sep 17 00:00:00 2001 From: Cryptotomte Date: Wed, 12 Aug 2026 15:20:15 +0200 Subject: [PATCH 5/7] docs(firmware): document the frozen rev2 profile and the new capability flags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit firmware/CLAUDE.md still told agents that rev2 pins were provisional behind TODO(SYNC1) markers, and its capability-flag list predated feature 012. - Board-revision section: rev2 pins are FROZEN and cite the SYNC 1 map; rev2 has no buttons and reserves IO18/19/23/4/27 for J7 (a rev2-only invariant — rev1 legitimately uses IO18/IO27). - Capability-flag list extended with the five new flags, and the enforcement-pattern sentence now names which pin macros are undefined on which board. The rev2-only power/rail signals are noted as declared but consumer-less until PR-14. - WiFi section: the boot config-button read is compiled out where BOARD_HAS_BTN_CONFIG == 0. Verification notes for this feature (all in the pinned v6.0.1 container): host suite 316 tests / 0 failures; both board targets build with the right CONFIG_BOARD_* in sdkconfig and all five artifacts present; the rev1 image is byte-size identical to the pre-feature baseline (0x103df0), which is the practical evidence for the rev1 no-op requirement (FR-002). Negative proof (quickstart §4): an unguarded BOARD_PIN_BTN_CONFIG reference added to the rev2 contract TU fails the build ("was not declared in this scope") and was reverted — the enforcement pattern bites. Spec: specs/012-rev2-pin-map (polish, plan D4) Co-Authored-By: Claude Opus 5 --- firmware/CLAUDE.md | 27 ++++++++++++++++++++++----- specs/012-rev2-pin-map/tasks.md | 6 +++--- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/firmware/CLAUDE.md b/firmware/CLAUDE.md index 075984e..cc0e1ab 100644 --- a/firmware/CLAUDE.md +++ b/firmware/CLAUDE.md @@ -239,16 +239,30 @@ Two board revisions exist, selected via Kconfig (`main/Kconfig.projbuild`): manual RS485 DE pin, level sensors active HIGH). - `CONFIG_BOARD_REV2` — custom PCB (THVD1426 auto-direction RS485 — no DE pin, INA226 current monitors, level sensors active LOW via 2N7002 inverter). - Rev2 pins are provisional until hardware sync 1 (`TODO(SYNC1)` markers). + **Rev2 pins are FROZEN** (feature 012): they come from the SYNC 1 map in + `hardware/rev2/design-notes/02-mcu.md` §2.2 (frozen 2026-08-12), which each + rev2 pin group in `board.h` cites — a pin change goes schematic-first and + re-opens the profile deliberately. rev2 has **no buttons** (BOOT/RESET + only) and reserves IO18/19/23/4/27 for the expansion header J7; `board.h` + fails the build if a rev2 pin lands on that set (a rev2-only invariant — + rev1 legitimately uses IO18 and IO27). All pins and polarity/feature flags come from `board/board.h` (`BOARD_PIN_*`, `BOARD_HAS_RS485_DE`, `BOARD_HAS_RESERVOIR_PUMP`, -`BOARD_LEVEL_ACTIVE_LOW`, `BOARD_HAS_INA226`, `BOARD_NAME`). Never +`BOARD_LEVEL_ACTIVE_LOW`, `BOARD_HAS_INA226`, `BOARD_HAS_BTN_MANUAL`, +`BOARD_HAS_BTN_CONFIG`, `BOARD_HAS_VBAT_SENSE`, `BOARD_HAS_PWR_PG`, +`BOARD_HAS_SENS_PWR_EN`, `BOARD_NAME`). Never hard-code GPIO numbers elsewhere. Board-conditional code uses `#if CONFIG_BOARD_REV2` / `#if BOARD_HAS_INA226`. Enforcement pattern: a capability flag at 0 leaves its pin/address macro UNDEFINED -(`BOARD_PIN_RS485_DE`, `BOARD_PIN_RESERVOIR_PUMP`, `BOARD_INA226_ADDR`), -so an unguarded reference is a compile error, never a phantom GPIO. +(`BOARD_PIN_RS485_DE`, `BOARD_PIN_RESERVOIR_PUMP`, `BOARD_INA226_ADDR`, +`BOARD_PIN_BTN_MANUAL`/`_CONFIG` on rev2, `BOARD_PIN_VBAT_SENSE`/ +`_PWR_PG`/`_SENS_PWR_EN` on rev1), so an unguarded reference is a compile +error, never a phantom GPIO. The rev2-only power/rail signals +(`VBAT_SENSE` IO34 ADC1 input-only, `PWR_PG` IO35 input-only + external +pull-up, `SENS_PWR_EN` IO25 output with the rail OFF by hardware default) +are declared but have no consumers yet — drivers and rail sequencing are +PR-14 scope. ## BME280 environmental sensor (I2C) @@ -375,7 +389,10 @@ watering. Credentials come from PR-06's `IConfigStore` (never logged, FR-004). config button (`BOARD_PIN_BTN_CONFIG`, GPIO18, active LOW, >= 5 s hold, 100 ms LED blink) → `decideBootMode` → provisioning (button-forced on a configured device clears credentials first, per the data-model boot rule) or station -(`begin(Station)` + `wifi_task_start`). Kconfig: `WS_PROV_AP_SSID`, +(`begin(Station)` + `wifi_task_start`). The whole button read is compiled out +where `BOARD_HAS_BTN_CONFIG == 0` — rev2 has no button, so the +credentials-absent path is its sole provisioning trigger (feature 012 FR-003). +Kconfig: `WS_PROV_AP_SSID`, `WS_PROV_AP_PASSWORD`, `WS_WIFI_*` reconnect constants. LED scope (parity §7/§9): 500 ms connect-attempt toggle (wifi task) + 100 ms config-button-hold blink (app_main); HIL checklist in `specs/007-wifi-provisioning/checklists/hil.md`. diff --git a/specs/012-rev2-pin-map/tasks.md b/specs/012-rev2-pin-map/tasks.md index 39c09df..dd5d785 100644 --- a/specs/012-rev2-pin-map/tasks.md +++ b/specs/012-rev2-pin-map/tasks.md @@ -64,9 +64,9 @@ unguarded references cannot compile on rev1. ## Phase 6: Polish & cross-cutting -- [ ] T014 [P] Check `firmware/CLAUDE.md` board-profile documentation (plan D4): if it enumerates capability flags or the pin-table convention, add the five new flags and the frozen-map citation convention; if it doesn't document the profile, change nothing. -- [ ] T015 Negative proof (quickstart §4): temporarily add an unguarded `BOARD_PIN_BTN_CONFIG` use to the rev2 TU, confirm the build FAILS, revert. Record in the implementation notes. -- [ ] T016 Full quickstart pass (§1–3) as the feature's final gate; assemble the implementer's test checklist for CP3 (host-test PR — CI suite is the verifier; no HIL needed, note the deferred bench verification lands in PR-14). +- [x] T014 [P] Check `firmware/CLAUDE.md` board-profile documentation (plan D4): if it enumerates capability flags or the pin-table convention, add the five new flags and the frozen-map citation convention; if it doesn't document the profile, change nothing. +- [x] T015 Negative proof (quickstart §4): temporarily add an unguarded `BOARD_PIN_BTN_CONFIG` use to the rev2 TU, confirm the build FAILS, revert. Record in the implementation notes. +- [x] T016 Full quickstart pass (§1–3) as the feature's final gate; assemble the implementer's test checklist for CP3 (host-test PR — CI suite is the verifier; no HIL needed, note the deferred bench verification lands in PR-14). ## Dependencies & execution order From 176be1e02e5f89f17886c6c6940b2d19879c1b68 Mon Sep 17 00:00:00 2001 From: Cryptotomte Date: Wed, 12 Aug 2026 16:37:36 +0200 Subject: [PATCH 6/7] =?UTF-8?q?fix(board):=20review=20hardening=20?= =?UTF-8?q?=E2=80=94=20pin=20frozen=20values,=20close=20collision-net=20ga?= =?UTF-8?q?ps,=20correct=20citations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hardening pass over the feature 012 review findings. The rev2 profile itself was confirmed correct; these changes make its guarantees enforceable and its provenance accurate. No pin VALUE changes: board.h gains only #if/#error guards and comments. - Pin the frozen macro VALUES in both contract TUs (FR-002's durable guard): rev1 I2C 21/22, RS485 16/17, DE 25, UART2, pump 26, LED 2; the mirror set for rev2. Previously only the values feature 012 touched were asserted, so a stray edit to a pre-existing pin passed the host suite. - Extend the pin-distinctness net to the three new rev2 power/rail pins (VBAT_SENSE, PWR_PG, SENS_PWR_EN) against every core function pin and each other, under their capability flags. Mutation-proven: SENS_PWR_EN == 26 (the pump gate) compiled silently before this. - Require every capability flag to be DEFINED, not merely 0/1 — an undefined macro evaluates to 0 in #if with no diagnostic, silently deleting the behavior it gates and disabling the flag-guarded checks above. - Add a maintenance sentinel to the rev2 section: the preprocessor cannot enumerate macros, so a new BOARD_PIN_* must be added by hand to the expansion-reservation check and to the rev2 contract TU. - Correct the I2C address-map citation to 07-i2c-env.md §7.3 (§7.4 is open items), and the solar INA226 provenance to the 01-power.md §1.5 decision of 2026-06-20 with the populate gate cleared 2026-08-10 (2026-08-12 is the schematic freeze, a different event). - De-pair the expansion comment: SCK=IO18, MISO=IO19, MOSI=IO23, CS=IO4, IRQ=IO27 per 08-expansion.md §8.3 — "SCK/MOSI/MISO on IO18/19/23" read pairwise implied MOSI=IO19, which is wrong. Verification: host suite 316 tests / 0 failures; both board targets build green with CONFIG_BOARD_REV1_DEVKIT=y and CONFIG_BOARD_REV2=y respectively. Co-Authored-By: Claude Opus 5 --- .../components/board/include/board/board.h | 87 ++++++++++++++++++- .../host/main/test_board_contract_rev1.cpp | 27 ++++++ .../host/main/test_board_contract_rev2.cpp | 26 +++++- specs/012-rev2-pin-map/research.md | 2 +- specs/012-rev2-pin-map/spec.md | 7 +- 5 files changed, 139 insertions(+), 10 deletions(-) diff --git a/firmware/components/board/include/board/board.h b/firmware/components/board/include/board/board.h index 0c68cc5..c155950 100644 --- a/firmware/components/board/include/board/board.h +++ b/firmware/components/board/include/board/board.h @@ -97,6 +97,15 @@ * drawn, ERC clean, components ordered). Changing one here changes nothing * on the board — pin changes go schematic-first and re-open this profile * deliberately. + * + * MAINTENANCE SENTINEL: adding ANY new BOARD_PIN_* to this section requires + * two hand edits elsewhere, because the preprocessor cannot enumerate macros + * — nothing detects an omission: + * (a) add it to the expansion-reservation check at the bottom of this + * header (BOARD_PIN_IS_EXPANSION list), and + * (b) add it to the disjointness/frozen-value asserts in + * firmware/test_apps/host/main/test_board_contract_rev2.cpp. + * A pin left out of those lists is silently unguarded. * ------------------------------------------------------------------------ */ #define BOARD_NAME "rev2" @@ -146,10 +155,11 @@ #define BOARD_LEVEL_SETTLE_MS 500 /* Pump current monitoring (INA226 on the shared I2C bus). - * Rev 2 I2C address map (07-i2c-env.md §7.4, frozen 2026-08-12): + * Rev 2 I2C address map (07-i2c-env.md §7.3, frozen 2026-08-12): * 0x40 INA226 pump monitor (A0 = A1 = GND) * 0x41 INA226 solar/panel telemetry (A0 → VS) — populated on this node - * (decision 2026-08-12; driver support PR-14) + * (01-power.md §1.5: decision 2026-06-20, populate gate cleared + * 2026-08-10; driver support PR-14) * 0x77 BME280 (SDO → VDDIO, rev1 parity; the driver still probes * 0x76 first and settles on whichever answers) * The ALERT pin is not connected — no Mask/Enable/Alert register use. @@ -271,6 +281,75 @@ #endif #endif +/* The rev2-only power/rail pins must not collide with a core function pin or + * with each other. Each check is guarded by its capability flag, exactly like + * the reservoir-pump checks above: where the signal does not exist the pin + * macro is undefined and must stay a compile error when referenced — never be + * papered over here. Without these, a typo that puts a rail signal on the + * pump gate (SENS_PWR_EN == MAIN_PUMP) would compile silently. */ +#if BOARD_HAS_VBAT_SENSE +#if (BOARD_PIN_VBAT_SENSE == BOARD_PIN_MAIN_PUMP) || \ + (BOARD_PIN_VBAT_SENSE == BOARD_PIN_LEVEL_LOW) || \ + (BOARD_PIN_VBAT_SENSE == BOARD_PIN_LEVEL_HIGH) || \ + (BOARD_PIN_VBAT_SENSE == BOARD_PIN_I2C_SDA) || \ + (BOARD_PIN_VBAT_SENSE == BOARD_PIN_I2C_SCL) || \ + (BOARD_PIN_VBAT_SENSE == BOARD_PIN_RS485_TX) || \ + (BOARD_PIN_VBAT_SENSE == BOARD_PIN_RS485_RX) || \ + (BOARD_PIN_VBAT_SENSE == BOARD_PIN_STATUS_LED) +#error "Board sanity: BOARD_PIN_VBAT_SENSE collides with a core function pin" +#endif +#endif +#if BOARD_HAS_PWR_PG +#if (BOARD_PIN_PWR_PG == BOARD_PIN_MAIN_PUMP) || \ + (BOARD_PIN_PWR_PG == BOARD_PIN_LEVEL_LOW) || \ + (BOARD_PIN_PWR_PG == BOARD_PIN_LEVEL_HIGH) || \ + (BOARD_PIN_PWR_PG == BOARD_PIN_I2C_SDA) || \ + (BOARD_PIN_PWR_PG == BOARD_PIN_I2C_SCL) || \ + (BOARD_PIN_PWR_PG == BOARD_PIN_RS485_TX) || \ + (BOARD_PIN_PWR_PG == BOARD_PIN_RS485_RX) || \ + (BOARD_PIN_PWR_PG == BOARD_PIN_STATUS_LED) +#error "Board sanity: BOARD_PIN_PWR_PG collides with a core function pin" +#endif +#endif +#if BOARD_HAS_SENS_PWR_EN +#if (BOARD_PIN_SENS_PWR_EN == BOARD_PIN_MAIN_PUMP) || \ + (BOARD_PIN_SENS_PWR_EN == BOARD_PIN_LEVEL_LOW) || \ + (BOARD_PIN_SENS_PWR_EN == BOARD_PIN_LEVEL_HIGH) || \ + (BOARD_PIN_SENS_PWR_EN == BOARD_PIN_I2C_SDA) || \ + (BOARD_PIN_SENS_PWR_EN == BOARD_PIN_I2C_SCL) || \ + (BOARD_PIN_SENS_PWR_EN == BOARD_PIN_RS485_TX) || \ + (BOARD_PIN_SENS_PWR_EN == BOARD_PIN_RS485_RX) || \ + (BOARD_PIN_SENS_PWR_EN == BOARD_PIN_STATUS_LED) +#error "Board sanity: BOARD_PIN_SENS_PWR_EN collides with a core function pin" +#endif +#endif +#if BOARD_HAS_VBAT_SENSE && BOARD_HAS_PWR_PG +#if BOARD_PIN_VBAT_SENSE == BOARD_PIN_PWR_PG +#error "Board sanity: BOARD_PIN_VBAT_SENSE and BOARD_PIN_PWR_PG must differ" +#endif +#endif +#if BOARD_HAS_VBAT_SENSE && BOARD_HAS_SENS_PWR_EN +#if BOARD_PIN_VBAT_SENSE == BOARD_PIN_SENS_PWR_EN +#error "Board sanity: BOARD_PIN_VBAT_SENSE and BOARD_PIN_SENS_PWR_EN must differ" +#endif +#endif +#if BOARD_HAS_PWR_PG && BOARD_HAS_SENS_PWR_EN +#if BOARD_PIN_PWR_PG == BOARD_PIN_SENS_PWR_EN +#error "Board sanity: BOARD_PIN_PWR_PG and BOARD_PIN_SENS_PWR_EN must differ" +#endif +#endif + +/* Every capability flag must be DEFINED, not merely 0 or 1. An undefined + * macro evaluates to 0 in #if without a diagnostic, so a flag lost in an edit + * would silently delete the behavior it gates (and, worse, silently disable + * the flag-guarded sanity checks above) instead of failing the build. */ +#if !defined(BOARD_HAS_BTN_MANUAL) || !defined(BOARD_HAS_BTN_CONFIG) || \ + !defined(BOARD_HAS_VBAT_SENSE) || !defined(BOARD_HAS_PWR_PG) || \ + !defined(BOARD_HAS_SENS_PWR_EN) || !defined(BOARD_HAS_RS485_DE) || \ + !defined(BOARD_HAS_RESERVOIR_PUMP) || !defined(BOARD_HAS_INA226) +#error "Board sanity: every capability flag must be defined (0 or 1)" +#endif + /* Feature flag consistency: BOARD_HAS_RS485_DE == 1 iff the DE pin exists */ #if BOARD_HAS_RS485_DE && !defined(BOARD_PIN_RS485_DE) #error "Board sanity: BOARD_HAS_RS485_DE is 1 but BOARD_PIN_RS485_DE is not defined" @@ -334,7 +413,9 @@ #endif /* Expansion-header reservation — REV 2 ONLY. - * J7 carries VSPI SCK/MOSI/MISO plus CS and IRQ on IO18/19/23/4/27; core + * J7 carries VSPI SCK/MISO/MOSI plus CS and IRQ on IO18/19/23/4/27 — read + * pairwise: SCK=IO18, MISO=IO19, MOSI=IO23, CS=IO4, IRQ=IO27 + * (08-expansion.md §8.3). Core * firmware must never claim one of them, or an attached expansion device * fights the core (and its bus traffic can be misread as core input). * This is deliberately NOT a cross-board check: rev1 legitimately uses IO18 diff --git a/firmware/test_apps/host/main/test_board_contract_rev1.cpp b/firmware/test_apps/host/main/test_board_contract_rev1.cpp index 78ad089..432d678 100644 --- a/firmware/test_apps/host/main/test_board_contract_rev1.cpp +++ b/firmware/test_apps/host/main/test_board_contract_rev1.cpp @@ -24,6 +24,33 @@ #define CONFIG_BOARD_REV1_DEVKIT 1 #include "board/board.h" +// FROZEN rev1 values (feature 012, FR-002). Feature 012 reshaped the rev2 +// profile and must not have moved a single rev1 pin; these asserts are the +// durable guard, so a later rev2 edit that strays into the rev1 branch fails +// the host build instead of silently re-pinning the running bench rig. +// Source of truth: docs/parity-checklist.md, extracted from src/main.cpp. +static_assert(BOARD_PIN_I2C_SDA == 21 && BOARD_PIN_I2C_SCL == 22, + "rev1 board contract: I2C pins unchanged (FR-002)"); +// RS485 TX=16/RX=17 per src/main.cpp — docs/hardware.md has them swapped +// (docs/parity-checklist.md QUIRK 6); the checklist wins. +static_assert(BOARD_PIN_RS485_TX == 16 && BOARD_PIN_RS485_RX == 17, + "rev1 board contract: RS485 UART pins unchanged (FR-002, " + "parity checklist QUIRK 6)"); +static_assert(BOARD_HAS_RS485_DE == 1, + "rev1 board contract: manual direction control (SP3485 path)"); +#ifndef BOARD_PIN_RS485_DE +#error "rev1 board contract: BOARD_PIN_RS485_DE must be defined" +#endif +static_assert(BOARD_PIN_RS485_DE == 25, + "rev1 board contract: RS485 DE pin unchanged (FR-002)"); +static_assert(BOARD_RS485_UART_PORT == 2, + "rev1 board contract: Modbus RTU on UART2 (parity: legacy " + "Serial2, docs/parity-checklist.md §5)"); +static_assert(BOARD_PIN_MAIN_PUMP == 26, + "rev1 board contract: plant pump pin unchanged (FR-002)"); +static_assert(BOARD_PIN_STATUS_LED == 2, + "rev1 board contract: status LED pin unchanged (FR-002)"); + // rev1 is the two-pump bench node: the capability flag is set AND the pin // exists (flag ⇒ pin, the board.h consistency assert's positive branch). static_assert(BOARD_HAS_RESERVOIR_PUMP == 1, diff --git a/firmware/test_apps/host/main/test_board_contract_rev2.cpp b/firmware/test_apps/host/main/test_board_contract_rev2.cpp index 0e94d2e..2beb5c0 100644 --- a/firmware/test_apps/host/main/test_board_contract_rev2.cpp +++ b/firmware/test_apps/host/main/test_board_contract_rev2.cpp @@ -15,6 +15,25 @@ #define CONFIG_BOARD_REV2 1 #include "board/board.h" +// FROZEN rev2 pin values (feature 012, FR-002). Every number below comes from +// the SYNC 1 map in hardware/rev2/design-notes/02-mcu.md §2.2 (frozen +// 2026-08-12): the board exists, so changing one here changes nothing in +// copper. Pinning them makes an accidental edit a build failure rather than a +// profile that silently disagrees with the PCB. +static_assert(BOARD_PIN_I2C_SDA == 21 && BOARD_PIN_I2C_SCL == 22, + "rev2 board contract: I2C pins per the frozen SYNC 1 map"); +static_assert(BOARD_PIN_RS485_TX == 16 && BOARD_PIN_RS485_RX == 17, + "rev2 board contract: RS485 UART pins per the frozen SYNC 1 map"); +static_assert(BOARD_RS485_UART_PORT == 2, + "rev2 board contract: Modbus RTU on UART2 (parity: legacy " + "Serial2, docs/parity-checklist.md §5)"); +static_assert(BOARD_PIN_MAIN_PUMP == 26, + "rev2 board contract: plant pump pin per the frozen SYNC 1 map"); +static_assert(BOARD_PIN_LEVEL_LOW == 32 && BOARD_PIN_LEVEL_HIGH == 33, + "rev2 board contract: level pins per the frozen SYNC 1 map"); +static_assert(BOARD_PIN_STATUS_LED == 2, + "rev2 board contract: status LED pin per the frozen SYNC 1 map"); + // rev2 is the single-pump node: capability flag 0 AND the pin REMOVED // (flag=0 ⇒ pin undefined — the compile-error enforcement this feature's // US2 rests on; same pattern as BOARD_PIN_RS485_DE). @@ -26,8 +45,8 @@ static_assert(BOARD_HAS_RESERVOIR_PUMP == 0, #endif // rev2 carries the pump INA226 at 0x40 (A0 = A1 = GND). The board-profile -// address map also lists 0x41 (solar INA226 — populated on this node since -// the 2026-08-12 decision; its driver lands in PR-14) and 0x77 (BME280). +// address map also lists 0x41 (solar INA226 — populated on this node, see +// 01-power.md §1.5; its driver lands in PR-14) and 0x77 (BME280). // BOARD_INA226_ADDR names the PUMP monitor specifically. static_assert(BOARD_HAS_INA226 == 1, "rev2 board contract: INA226 pump monitor present"); @@ -103,7 +122,8 @@ static_assert(BOARD_PIN_SENS_PWR_EN == 25, "by hardware default)"); // Expansion reservation (feature 012, FR-004). J7 carries VSPI -// SCK/MOSI/MISO plus CS and IRQ on IO18/19/23/4/27; core firmware must not +// SCK=IO18, MISO=IO19, MOSI=IO23 plus CS=IO4 and IRQ=IO27 +// (08-expansion.md §8.3); core firmware must not // claim any of them on rev2. board.h enforces this too — this TU is the // belt to that header's braces: the contract survives even if the header // check is ever removed. NOTE: this is a rev2-ONLY invariant; rev1 diff --git a/specs/012-rev2-pin-map/research.md b/specs/012-rev2-pin-map/research.md index eb363d2..a163ed2 100644 --- a/specs/012-rev2-pin-map/research.md +++ b/specs/012-rev2-pin-map/research.md @@ -15,7 +15,7 @@ app_main, and contract tests. Verified divergences that remain: | 2 | Boot path reads the phantom button: `app_main.cpp` configures IO18 as input and polls it (with STATUS_LED feedback) to enter provisioning | `app_main.cpp:148-217` | | 3 | Missing signals: `VBAT_SENSE` (IO34), `PWR_PG` (IO35), `SENS_PWR_EN` (IO25) undefined | grep over `board.h`; `01-power.md` §1.0a; `rev2-firmware-notes.md` FW-1/FW-6 | | 4 | 11 stale `TODO(SYNC1)` markers + "provisionally mirror rev 1" header | grep count | -| 5 | Stale comment: I²C map calls 0x41 "solar footprint, DNP"; solar group is populated per 2026-08-12 decision | `board.h` rev2 INA226 block; `01-power.md` §1.5 | +| 5 | Stale comment: I²C map calls 0x41 "solar footprint, DNP"; solar group is populated per the 2026-06-20 decision (gate cleared 2026-08-10) | `board.h` rev2 INA226 block; `01-power.md` §1.5 | All present rev2 pin VALUES verified correct against the frozen contract (I2C 21/22, RS485 16/17, pump 26, level 32/33, LED 2). The fix is diff --git a/specs/012-rev2-pin-map/spec.md b/specs/012-rev2-pin-map/spec.md index 0ecc472..9789b9f 100644 --- a/specs/012-rev2-pin-map/spec.md +++ b/specs/012-rev2-pin-map/spec.md @@ -43,8 +43,9 @@ against the frozen design leaves three real divergence classes: saying the pin map "provisionally mirrors rev 1" — but SYNC 1 (the pin-map freeze) has happened and every present value matches the frozen contract. Also one stale hardware comment: the I²C address-map note calls the solar - INA226 (0x41) "footprint, DNP", but the populate decision of 2026-08-12 - made it a populated device on this node. + INA226 (0x41) "footprint, DNP", but the populate decision of 2026-06-20 + (gate cleared 2026-08-10, `01-power.md` §1.5) made it a populated device + on this node. Already correct on `origin/main` — to be regression-guarded, not re-done: single-pump profile with compile-error enforcement, level-sensor polarity and @@ -69,7 +70,7 @@ map) and `00-architecture.md` §0.5 pin contract (both on the | Battery voltage sense | IO34 | input-only, ADC1; ADC2 unusable with WiFi — **missing today** | | Buck power-good | IO35 | input-only, open-drain externally pulled up; IO34–39 have no internal pulls — **missing today** | | Manual / config buttons | — | **do not exist on rev2** (BOOT/RESET only) — profile must say so | -| Expansion (reserved, untouchable) | IO18/19/23/4/27 | J7: VSPI SCK/MOSI/MISO + CS/IRQ; core firmware must not claim any of them | +| Expansion (reserved, untouchable) | IO18/19/23/4/27 | J7: VSPI SCK=IO18, MISO=IO19, MOSI=IO23, CS=IO4, IRQ=IO27 (`08-expansion.md` §8.3); core firmware must not claim any of them | ## User Scenarios & Testing *(mandatory)* From 72614984386a0e88beb186f82a0473cd8c1a6493 Mon Sep 17 00:00:00 2001 From: Cryptotomte Date: Wed, 12 Aug 2026 16:47:45 +0200 Subject: [PATCH 7/7] fix(board): order the flag-definedness gate first and complete the sentinel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to 176be1e, from the verification re-review. board.h only; still zero #define additions or removals. - Move the capability-flag definedness check to the TOP of the sanity section, ahead of every flag-guarded check rather than after them. Those checks are gated on the very flags it validates, so an undefined flag could previously switch OFF the collision checks it was meant to enable and still reach the definedness error only afterwards. Ordering makes that structurally impossible. Placed above the reservoir-pump and RS485-DE guards too, not just the power/rail block — they have the same dependency. - Add the latent SENS_PWR_EN vs RS485_DE cross-check, matching the existing double-guard pattern. Unreachable on both boards today (no board has both signals), but a future board combining them would put both on IO25 and collide silently. Verified to fire by synthetically granting rev1 a sensor rail on 25. - Complete the maintenance sentinel: it listed only the two hand-maintained lists that predated it and omitted the third one the previous commit introduced — the eight-flag definedness check. Now covers new BOARD_HAS_* flags as well as new BOARD_PIN_*. - Sentinel wording: "disjointness/frozen-value asserts" overstated the rev2 TU, which asserts frozen values and expansion-set membership, not general pairwise disjointness. Corrected. Verification: host suite 316 tests / 0 failures; rev2 target builds green with CONFIG_BOARD_REV2=y. Co-Authored-By: Claude Opus 5 --- .../components/board/include/board/board.h | 50 ++++++++++++------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/firmware/components/board/include/board/board.h b/firmware/components/board/include/board/board.h index c155950..d53701b 100644 --- a/firmware/components/board/include/board/board.h +++ b/firmware/components/board/include/board/board.h @@ -98,14 +98,16 @@ * on the board — pin changes go schematic-first and re-open this profile * deliberately. * - * MAINTENANCE SENTINEL: adding ANY new BOARD_PIN_* to this section requires - * two hand edits elsewhere, because the preprocessor cannot enumerate macros - * — nothing detects an omission: - * (a) add it to the expansion-reservation check at the bottom of this - * header (BOARD_PIN_IS_EXPANSION list), and - * (b) add it to the disjointness/frozen-value asserts in - * firmware/test_apps/host/main/test_board_contract_rev2.cpp. - * A pin left out of those lists is silently unguarded. + * MAINTENANCE SENTINEL: adding ANY new BOARD_PIN_* or BOARD_HAS_* to this + * section requires hand edits elsewhere, because the preprocessor cannot + * enumerate macros — nothing detects an omission: + * (a) add a new pin to the expansion-reservation check at the bottom of + * this header (BOARD_PIN_IS_EXPANSION list), + * (b) add a new pin to the frozen-value and expansion-set asserts in + * firmware/test_apps/host/main/test_board_contract_rev2.cpp, and + * (c) add a new BOARD_HAS_* flag to the capability-flag definedness check + * at the top of the sanity section below. + * A macro left out of those lists is silently unguarded. * ------------------------------------------------------------------------ */ #define BOARD_NAME "rev2" @@ -227,6 +229,20 @@ * A wrong or inconsistent pin table must fail the build, not the rig. * ------------------------------------------------------------------------ */ +/* Every capability flag must be DEFINED, not merely 0 or 1. An undefined + * macro evaluates to 0 in #if without a diagnostic, so a flag lost in an edit + * would silently delete the behavior it gates instead of failing the build. + * This check comes FIRST, ahead of every flag-guarded check below: those + * checks are themselves gated on these flags, so an undefined flag would + * otherwise silently switch OFF the very collision checks it is supposed to + * enable. Ordering makes that structurally impossible. */ +#if !defined(BOARD_HAS_BTN_MANUAL) || !defined(BOARD_HAS_BTN_CONFIG) || \ + !defined(BOARD_HAS_VBAT_SENSE) || !defined(BOARD_HAS_PWR_PG) || \ + !defined(BOARD_HAS_SENS_PWR_EN) || !defined(BOARD_HAS_RS485_DE) || \ + !defined(BOARD_HAS_RESERVOIR_PUMP) || !defined(BOARD_HAS_INA226) +#error "Board sanity: every capability flag must be defined (0 or 1)" +#endif + /* Pin distinctness within each function group. Checks that reference the * reservoir pump pin are guarded: on single-pump boards the pin does not * exist (BOARD_HAS_RESERVOIR_PUMP == 0), and an unguarded reference must @@ -338,16 +354,14 @@ #error "Board sanity: BOARD_PIN_PWR_PG and BOARD_PIN_SENS_PWR_EN must differ" #endif #endif - -/* Every capability flag must be DEFINED, not merely 0 or 1. An undefined - * macro evaluates to 0 in #if without a diagnostic, so a flag lost in an edit - * would silently delete the behavior it gates (and, worse, silently disable - * the flag-guarded sanity checks above) instead of failing the build. */ -#if !defined(BOARD_HAS_BTN_MANUAL) || !defined(BOARD_HAS_BTN_CONFIG) || \ - !defined(BOARD_HAS_VBAT_SENSE) || !defined(BOARD_HAS_PWR_PG) || \ - !defined(BOARD_HAS_SENS_PWR_EN) || !defined(BOARD_HAS_RS485_DE) || \ - !defined(BOARD_HAS_RESERVOIR_PUMP) || !defined(BOARD_HAS_INA226) -#error "Board sanity: every capability flag must be defined (0 or 1)" +/* Latent cross-check: unreachable on both boards today, since no board has + * both signals (rev1 DE=25 without a sensor rail, rev2 SENS_PWR_EN=25 without + * a DE pin). A future board combining them would land both on IO25 and + * collide silently — the double guard keeps the check honest until then. */ +#if BOARD_HAS_SENS_PWR_EN && BOARD_HAS_RS485_DE +#if BOARD_PIN_SENS_PWR_EN == BOARD_PIN_RS485_DE +#error "Board sanity: BOARD_PIN_SENS_PWR_EN collides with the RS485 DE pin" +#endif #endif /* Feature flag consistency: BOARD_HAS_RS485_DE == 1 iff the DE pin exists */