address copilot review

2026-06-25 13:44:09 +01:00
parent 90f01c2b95
commit 0094ad0444
5 changed files with 169 additions and 26 deletions
@@ -1,5 +1,21 @@
 # Meshtastic Firmware - Copilot Instructions

+> **TL;DR**
+> | | |
+> |---|---|
+> | Local tests | `./bin/run-tests.sh` (exit 0 GREEN · 1 RED · 2 AMBER · 3 FILTERED) |
+> | Hardware tests | `./mcp-server/run-tests.sh` |
+> | Format | `trunk fmt` |
+> | Mirror docs | `AGENTS.md` (short pointer for agents that don't read this file) · `CLAUDE.md` (Claude Code) |
+>
+> **Need this? It's here.**
+> | | |
+> |---|---|
+> | General helpers (clamp, UTF-8, string fmt…) | `src/meshUtils.h` |
+> | Logging macros (LOG_DEBUG / INFO / WARN…) | `src/DebugConfiguration.h` |
+> | New module skeleton | inherit `ProtobufModule<T>` in `src/mesh/ProtobufModule.h` |
+> | Observer / event wiring | `src/Observer.h` |
+
 This document provides context and guidelines for AI assistants working with the Meshtastic firmware codebase.

 ## Project Overview
@@ -37,7 +53,7 @@ MQTT provides a bridge between Meshtastic mesh networks and the internet, enabli

 Messages are published/subscribed using a hierarchical topic format:

-```
+```text
 {root}/{channel_id}/{gateway_id}
 ```

@@ -247,7 +263,7 @@ Unit tests for the conversion layer live in `test/test_type_conversions/test_mai

 ## Project Structure

-```
+```text
 firmware/
 ├── src/                    # Main source code
 │   ├── main.cpp           # Application entry point
@@ -482,7 +498,7 @@ Key defines in variant.h:

 ## Build System

-## Agent Tooling Baseline
+### Agent Tooling Baseline

 Mirror counterpart: `AGENTS.md` under **Agent Tooling Baseline**.

@@ -635,7 +651,7 @@ Most workflows can be triggered manually via `workflow_dispatch` for testing.

 ### Native unit tests (C++)

-Unit tests in `test/` directory with 19 test suites:
+Unit tests in `test/` directory. The canonical suite count is in `test/native-suite-count` and is cross-checked on every full run. Current suites:

 - `test_admin_radio/` - LoRa region/config validation and AdminModule dispatch
 - `test_atak/` - ATAK integration
@@ -647,30 +663,57 @@ Unit tests in `test/` directory with 19 test suites:
 - `test_mesh_module/` - Module framework
 - `test_meshpacket_serializer/` - Packet serialization
 - `test_mqtt/` - MQTT integration
+- `test_nexthop_routing/` - Next-hop routing logic
+- `test_nodedb_blocked/` - NodeDB blocked-node handling
 - `test_packet_history/` - Packet history tracking
 - `test_packet_signing/` - Packet signing
+- `test_position_module/` - Position module behaviour
 - `test_position_precision/` - Position precision helpers
 - `test_radio/` - Radio interface
+- `test_rtc/` - RTC / time handling
 - `test_serial/` - Serial communication
 - `test_traffic_management/` - Traffic management (dedup, rate-limit, hop-trim, role exceptions)
 - `test_transmit_history/` - Retransmission tracking
 - `test_type_conversions/` - NodeDB v25 type conversion (bitfield round-trips, NodeInfoLite)
 - `test_utf8/` - UTF-8 utilities
+- `test_warm_store/` - Warm-tier node store

-**Preferred run command — `bin/run-tests.sh`** (uses the `coverage` env with ASan/LSan sanitizers, emits an unambiguous RED/AMBER/GREEN verdict, and catches missing suites as AMBER):
+**Preferred run command — `bin/run-tests.sh`** (uses the `coverage` env with ASan/LSan sanitizers; emits a machine-readable verdict on the final line; update `test/native-suite-count` when adding or removing suites):

 ```bash
-./bin/run-tests.sh                          # all suites
-./bin/run-tests.sh -f test_traffic_management  # single suite
-./bin/run-tests.sh -f test_traffic_management > /tmp/test_out.txt 2>&1; tail -5 /tmp/test_out.txt
+./bin/run-tests.sh                             # all suites
+./bin/run-tests.sh -f test_traffic_management  # single suite (yields FILTERED, not GREEN)
 ```

-Exit codes: 0 = GREEN, 1 = RED, 2 = AMBER. The final line is machine-readable:
+Exit codes and verdicts (exact counts will vary; examples below are illustrative):

-```
-RESULT: GREEN 19/19 suites passed
-RESULT: RED test_traffic_management: 1 failed
-RESULT: AMBER 18/19 suites ran (missing: test_radio) — all that ran passed
+| Exit | Verdict    | Meaning                                                                                                                                                                                                               |
+| ---- | ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| 0    | `GREEN`    | All canonical suites ran, all passed, no ignored test cases                                                                                                                                                           |
+| 1    | `RED`      | At least one failure, build error, or sanitizer fault                                                                                                                                                                 |
+| 2    | `AMBER`    | All that ran passed, but something was lost: a suite silently went missing on a full run, individual test cases were skipped (`TEST_IGNORE`), or `test/native-suite-count` disagrees with the `test/` directory count |
+| 3    | `FILTERED` | A `-f` run completed cleanly; suites outside the filter were intentionally not run                                                                                                                                    |
+
+Examples — exact counts will vary by suite count and env:
+
+```text
+# GREEN: all suites ran and passed
+RESULT: GREEN N/N suites passed [canonical: N/N]
+
+# RED: real test failure
+RESULT: RED 1 failed
+
+# RED: sanitizer exit-time abort (all tests passed but process aborted at exit)
+RESULT: RED exit-time abort (tests passed; likely sanitizer — see hint above)
+
+# AMBER: native-suite-count disagrees with test/ directory count (too low)
+RESULT: AMBER test/ has 24 suite directories but native-suite-count says 5 — update test/native-suite-count after registering new suites
+
+# AMBER: native-suite-count disagrees with test/ directory count (too high)
+RESULT: AMBER test/ has 24 suite directories but native-suite-count says 99 — update test/native-suite-count after removing suites
+
+# FILTERED: single suite run completed cleanly
+RESULT: FILTERED 1/24 suites ran (not run: test_admin_radio test_atak …) — filtered: test_serial [canonical: 1/24]
 ```

 > **Copilot interface note:** When running tests via the Copilot chat interface, edits made through the chat may not be reflected in the on-disk files that the test binary reads. If tests pass in chat but fail locally (or vice versa), verify the files on disk match what you expect before trusting the result. Always confirm with a local terminal run.
@@ -1,5 +1,21 @@
 # Agent instructions

+> **TL;DR**
+> | | |
+> |---|---|
+> | Local tests | `./bin/run-tests.sh` (exit 0 GREEN · 1 RED · 2 AMBER · 3 FILTERED) |
+> | Hardware tests | `./mcp-server/run-tests.sh` |
+> | Format | `trunk fmt` |
+> | Mirror docs | `.github/copilot-instructions.md` (canonical) · `CLAUDE.md` (Claude Code) |
+>
+> **Need this? It's here.**
+> | | |
+> |---|---|
+> | General helpers (clamp, UTF-8, string fmt…) | `src/meshUtils.h` |
+> | Logging macros (LOG_DEBUG / INFO / WARN…) | `src/DebugConfiguration.h` |
+> | New module skeleton | inherit `ProtobufModule<T>` in `src/mesh/ProtobufModule.h` |
+> | Observer / event wiring | `src/Observer.h` |
+
 This repository is the [Meshtastic](https://meshtastic.org) firmware — a C++17 embedded codebase targeting ESP32 / nRF52 / RP2040 / STM32WL / Linux-Portduino LoRa mesh radios — plus a Python MCP server in `mcp-server/` that AI agents use to flash, configure, and test connected devices.

 ## Primary instruction file
@@ -0,0 +1,21 @@
+# Claude Code instructions
+
+> **TL;DR**
+> | | |
+> |---|---|
+> | Local tests | `./bin/run-tests.sh` (exit 0 GREEN · 1 RED · 2 AMBER · 3 FILTERED) |
+> | Hardware tests | `./mcp-server/run-tests.sh` |
+> | Format | `trunk fmt` |
+> | Mirror docs | `.github/copilot-instructions.md` (canonical) · `AGENTS.md` |
+>
+> **Need this? It's here.**
+> | | |
+> |---|---|
+> | General helpers (clamp, UTF-8, string fmt…) | `src/meshUtils.h` |
+> | Logging macros (LOG_DEBUG / INFO / WARN…) | `src/DebugConfiguration.h` |
+> | New module skeleton | inherit `ProtobufModule<T>` in `src/mesh/ProtobufModule.h` |
+> | Observer / event wiring | `src/Observer.h` |
+
+**Read `.github/copilot-instructions.md` first.** That file is the canonical agent-facing document for this repo. It covers project layout, coding conventions, the build system, CI/CD, the native C++ test suite, and the MCP Server & Hardware Test Harness. Read it top-to-bottom before starting any non-trivial change.
+
+This file (`CLAUDE.md`) is a short pointer for Claude Code sessions. Slash commands live in `.claude/commands/`.
@@ -1,5 +1,5 @@
 #!/usr/bin/env bash
-# Run native PlatformIO unit tests and emit a single, unambiguous RED/AMBER/GREEN verdict.
+# Run native PlatformIO unit tests and emit a single, unambiguous verdict.
 #
 # Why this exists: PlatformIO reports failures three different ways ([FAILED], :FAIL:,
 # [ERRORED]) and an all-pass run prints "N succeeded" with NO "0 failed" clause — so naive
@@ -8,16 +8,26 @@
 # canonical set in test/ so a suite silently going missing shows up as AMBER, not green.
 #
 # Usage:
-#   ./bin/run-tests.sh                      # run all suites, full RAG + count cross-check
-#   ./bin/run-tests.sh -f test_utf8         # run one suite (no count cross-check)
+#   ./bin/run-tests.sh                      # run all suites, full verdict + count cross-check
+#   ./bin/run-tests.sh -f test_utf8         # run one suite (yields FILTERED, not GREEN)
 #   ./bin/run-tests.sh -e native            # override env (default: coverage)
 #   ./bin/run-tests.sh --quiet              # only print the final RESULT line
 #
-# Exit codes: 0 = GREEN, 1 = RED, 2 = AMBER.
+# Exit codes: 0 = GREEN, 1 = RED, 2 = AMBER, 3 = FILTERED.
+#
+# Verdicts:
+#   GREEN    — all canonical suites ran, all passed, no ignored test cases.
+#   AMBER    — all that ran passed, but something was lost: a suite silently went missing on a
+#              full run, or individual test cases were skipped (Unity TEST_IGNORE / :IGNORE:).
+#   FILTERED — a -f run completed cleanly; suites not in the filter were intentionally skipped.
+#              Use this when iterating on a single suite; it is not a quality signal.
+#   RED      — at least one failure, build error, or sanitizer fault.
 #
 # The final line is machine-readable, e.g.:
-#   RESULT: GREEN 19/19 suites passed
-#   RESULT: AMBER 17/19 suites ran (missing: test_radio test_serial) — all that ran passed
+#   RESULT: GREEN N/N suites passed
+#   RESULT: AMBER N/M suites ran (missing: test_radio test_serial) — all that ran passed
+#   RESULT: AMBER 3 test case(s) ignored
+#   RESULT: FILTERED 1/N suites ran (not run: …) — filtered: test_utf8
 #   RESULT: RED test_traffic_management: 1 failed  (or: build/crash error)
 #   RESULT: RED sanitizer fault — SUMMARY: AddressSanitizer: 1272 byte(s) leaked  (tests may have
 #           all passed; the coverage build aborts at exit on an ASan/LSan fault — often shown only
@@ -73,6 +83,15 @@ trap 'rm -f "$LOG" "${MARKER:-}"; [[ -n ${PROGRESS_PID:-} ]] && kill "$PROGRESS_
 mapfile -t ALL_SUITES < <(find test -maxdepth 1 -type d -name 'test_*' -printf '%f\n' | sort)
 EXPECTED_COUNT=${#ALL_SUITES[@]}

+# Canonical suite count — the registered total, maintained in test/native-suite-count.
+# Update that file whenever a test suite is added or removed.
+CANONICAL_COUNT_FILE="test/native-suite-count"
+if [[ -f $CANONICAL_COUNT_FILE ]]; then
+	CANONICAL_COUNT=$(tr -d '[:space:]' <"$CANONICAL_COUNT_FILE")
+else
+	CANONICAL_COUNT=""
+fi
+
 # Cached object-count for this env, written after each completed build (in the gitignored build
 # dir). Used as the progress denominator: accurate for a full rebuild (every object recompiles),
 # only a rough upper bound for an incremental run.
@@ -129,6 +148,9 @@ if ! $QUIET; then
 	echo "Running: $PIO test -e $ENV ${PASSTHRU[*]-} (expecting $EXPECTED_COUNT suites)"
 fi
 echo "progress: tail -f $PROGRESS_FILE" >&2
+if [[ ! -t 1 ]] && ! $QUIET; then
+	echo "hint: stdout is a pipe — build errors appear at the top of output and may be lost; use --quiet to get just the RESULT line" >&2
+fi

 # Run pio, tee to log. PIPESTATUS[0] is pio's real exit (NOT tee's).
 if $QUIET; then
@@ -227,8 +249,41 @@ if ! grep -qE "$PASS_RE" "$LOG"; then
 	exit 1
 fi

-# AMBER: everything that ran passed, but (full run only) a canonical suite neither ran NOR was
-# explicitly skipped — i.e. it silently went missing. SKIPPED suites are accounted for.
+# Canonical-count rating suffix — appended to every verdict line so the result is always
+# rated against the registered total, not just the directory count.
+# If the two counts diverge (suite added/removed without updating native-suite-count), that
+# is itself surfaced as AMBER before we reach any verdict.
+canonical_rating() {
+	if [[ -n $CANONICAL_COUNT ]]; then
+		echo "[canonical: ${RAN_COUNT}/${CANONICAL_COUNT}]"
+	fi
+}
+
+# AMBER: directory count disagrees with native-suite-count — file needs updating.
+if [[ -n $CANONICAL_COUNT && $EXPECTED_COUNT -ne $CANONICAL_COUNT ]]; then
+	echo ""
+	if [[ $EXPECTED_COUNT -gt $CANONICAL_COUNT ]]; then
+		echo "RESULT: AMBER test/ has $EXPECTED_COUNT suite directories but native-suite-count says $CANONICAL_COUNT — update test/native-suite-count after registering new suites"
+	else
+		echo "RESULT: AMBER test/ has $EXPECTED_COUNT suite directories but native-suite-count says $CANONICAL_COUNT — update test/native-suite-count after removing suites"
+	fi
+	exit 2
+fi
+
+# AMBER: individual test cases were skipped (Unity TEST_IGNORE → :IGNORE: in output).
+# Applies to both full and filtered runs — a skipped test case is a lost signal either way.
+mapfile -t IGNORED_TESTS < <(grep -oE '[^:]+:[0-9]+:[^:]+:IGNORE:.*' "$LOG" 2>/dev/null | sed 's/:IGNORE:.*//' | sort -u)
+IGNORED_COUNT=${#IGNORED_TESTS[@]}
+if [[ $IGNORED_COUNT -gt 0 ]]; then
+	IGNORE_DETAIL="$(printf '%s\n' "${IGNORED_TESTS[@]}" | head -5 | sed 's/^/    /')"
+	echo ""
+	echo "$IGNORE_DETAIL"
+	echo ""
+	echo "RESULT: AMBER ${IGNORED_COUNT} test case(s) ignored $(canonical_rating)"
+	exit 2
+fi
+
+# AMBER: full run only — a canonical suite neither ran NOR was explicitly skipped (silently missing).
 ACCOUNTED_COUNT=$((RAN_COUNT + ${#SKIPPED_SUITES[@]}))
 if [[ -z $FILTER && $ACCOUNTED_COUNT -lt $EXPECTED_COUNT ]]; then
 	missing=()
@@ -236,14 +291,21 @@ if [[ -z $FILTER && $ACCOUNTED_COUNT -lt $EXPECTED_COUNT ]]; then
 		printf '%s\n' "${RAN_SUITES[@]}" "${SKIPPED_SUITES[@]}" | grep -qx "$s" || missing+=("$s")
 	done
 	echo ""
-	echo "RESULT: AMBER ${RAN_COUNT}/${EXPECTED_COUNT} suites ran (missing: ${missing[*]}) — all that ran passed"
+	echo "RESULT: AMBER ${RAN_COUNT}/${EXPECTED_COUNT} suites ran (missing: ${missing[*]}) — all that ran passed $(canonical_rating)"
 	exit 2
 fi

-# GREEN.
+# FILTERED: a -f run completed cleanly. Suites outside the filter were intentionally not run;
+# this is not a quality signal and is distinct from suites that went missing unexpectedly.
 if [[ -n $FILTER ]]; then
-	echo "RESULT: GREEN ${RAN_COUNT} suite(s) passed (filtered: $FILTER)"
-else
-	echo "RESULT: GREEN ${RAN_COUNT}/${EXPECTED_COUNT} suites passed"
+	not_run=()
+	for s in "${ALL_SUITES[@]}"; do
+		printf '%s\n' "${RAN_SUITES[@]}" "${SKIPPED_SUITES[@]}" | grep -qx "$s" || not_run+=("$s")
+	done
+	echo "RESULT: FILTERED ${RAN_COUNT}/${EXPECTED_COUNT} suites ran (not run: ${not_run[*]}) — filtered: $FILTER $(canonical_rating)"
+	exit 3
 fi
+
+# GREEN: all canonical suites ran, all passed, no ignored test cases.
+echo "RESULT: GREEN ${RAN_COUNT}/${EXPECTED_COUNT} suites passed $(canonical_rating)"
 exit 0
@@ -0,0 +1 @@
+24