From 4f57651c9b0595d702a4854670a12e279c8c50c9 Mon Sep 17 00:00:00 2001 From: pgdalmeida Date: Fri, 18 Sep 2026 14:50:50 +0200 Subject: [PATCH] Refuse to start when the rig is missing hardware it declares --- CMakeLists.txt | 1 + config/hardware.example.ini | 8 ++ docs/configuration.md | 38 ++++++ docs/hardware-config-plan.md | 6 +- docs/hardware-porting-plan.md | 32 ++--- docs/modules-reference.md | 1 + include/fgc/Application.h | 4 + include/fgc/Config.h | 7 ++ include/fgc/HardwarePresence.h | 92 ++++++++++++++ main.cpp | 4 + src/core/Application.cpp | 73 ++++++++++++ src/core/Config.cpp | 2 + src/core/HardwarePresence.cpp | 170 ++++++++++++++++++++++++++ tests/CMakeLists.txt | 1 + tests/test_hardwarepresence.cpp | 204 ++++++++++++++++++++++++++++++++ 15 files changed, 624 insertions(+), 19 deletions(-) create mode 100644 include/fgc/HardwarePresence.h create mode 100644 src/core/HardwarePresence.cpp create mode 100644 tests/test_hardwarepresence.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index f1fb2b7..455cada 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -64,6 +64,7 @@ add_library(fgc_core STATIC src/core/Logger.cpp src/core/Geometry.cpp src/core/TmcCurrent.cpp + src/core/HardwarePresence.cpp src/core/ScanGrid.cpp src/core/TelemetryParser.cpp src/core/CaptureScheduler.cpp diff --git a/config/hardware.example.ini b/config/hardware.example.ini index ec9fca7..b64d3ba 100644 --- a/config/hardware.example.ini +++ b/config/hardware.example.ini @@ -44,6 +44,14 @@ imu = none env = sht41 ; The [Camera.] blocks expected to be present and found. cameras = rgb +; +; What happens when this does not match the rig: +; declared but absent -> warned about by name, and with [Features] +; strict_presence = 1 in config.ini the program refuses +; to start (exit 3). --degraded starts it anyway. +; found but undeclared -> warned about as a stale file. Never fatal. +; A tower with no hardware.ini at all is never reconciled, whatever +; strict_presence says: gaining this file must not change how a tower behaves. ; --- Where to find it ----------------------------------------------------- ; Device bindings. These follow the hardware, not the tower, so they belong here diff --git a/docs/configuration.md b/docs/configuration.md index 2e98376..4bccbfa 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -55,6 +55,7 @@ Parsed and validated by `ConfigLoader` ([src/core/Config.cpp](../src/core/Config | `Features` | `mock_imu` | bool | `false` | Use the simulated IMU instead of the MTi | | `Features` | `enable_env` | bool | `false` | Use the ambient temp/humidity sensor (SHT41) | | `Features` | `mock_env` | bool | `false` | Use the simulated env sensor instead of the SHT41 | +| `Features` | `strict_presence` | bool | `false` | Refuse to start when `hardware.ini` declares a subsystem this rig does not have. Off during the fleet rollout; turn it on per tower once that file is known good. `--degraded` overrides it | | `IMU` | `device` | string | — | MTi serial device (see `[IMU]` note); required when `enable_imu` | | `IMU` | `baud` | int | `115200` | MTi serial baud rate | | `IMU` | `x` / `y` / `z` | float ±360 | `0` | Mounting rotation in degrees about forward/right/down, applied to every IMU reading (see below). All zero = the IMU is aligned with the gimbal | @@ -399,6 +400,42 @@ generic `IEnvSensor` interface ([include/fgc/IEnvSensor.h](../include/fgc/IEnvSensor.h)), so swapping in a different ambient sensor later means adding a new backend, not touching `Application`, the UI, or MQTT. +### Declared vs found — what `hardware.ini` is checked against + +Everything in this program degrades gracefully: `TestRunner` skips the pitch and IMU leaves, `CamEvent` +falls back to encoder pitch, `CaptureScheduler` sends the single-axis `MOVE Y` form. That is correct for a +rig that genuinely lacks them, and **indistinguishable from a failure on a rig that does not**. A two-axis +tower whose pitch stops enumerating becomes a yaw-only tower, captures one elevation band, and reports +itself healthy. + +Declaring the assembly is what separates those two. At startup — once the motor, sensors and camera have +all been brought up, so "found" means *came up* rather than *was configured* — the program compares +`[Hardware]` against what it actually has: + +| Declared | Found | Result | +|---|---|---| +| present | present | one `INFO` line confirming the assembly | +| absent | present | **warn**: `hardware.ini` is stale. Never fatal — more hardware than declared is a documentation fault | +| **present** | **absent** | **warn, or refuse to start under `strict_presence`**, naming each missing subsystem | + +Two properties are deliberate: + +- **A tower with no `hardware.ini` is never reconciled**, whatever `strict_presence` says. Gaining that + file must not be what changes how a tower behaves; nothing is compared until something is declared. +- **An empty `axes` states nothing**, matching `hasAxis()` answering true for everything when nothing is + said. Reconciling silence would turn a missing key into a fault. + +`strict_presence` defaults to **off** so the fleet rollout cannot change behaviour on upgrade. Turn it on +per tower once that tower's file is known good; the fleet-wide flip is then a config change rather than a +release. When it refuses, the program exits **3** (distinct from a config error's 1 and the +single-instance lock's 2) before the control loop is entered. + +`--degraded` overrides a refusal for field triage, and logs `RUNNING DEGRADED BY REQUEST` with the list of +what is missing — worded so it cannot be mistaken for a normal startup line. + +> Which camera is missing is reported as a count (`2 configured, only 1 opened`), because the camera +> source reports how many it opened, not which. `test hardware camera` is what names the device. + ### Secrets `mqtt_user` / `mqtt_pw` are read from the environment variables **`FGC_MQTT_USER` / `FGC_MQTT_PW`** first, @@ -420,6 +457,7 @@ Parsed by Boost.Program_options ([main.cpp](../main.cpp)). Flags override `[Feat | `--mock-camera` | Use the simulated camera | | `--mock-serial` | Use the simulated motor controller | | `--mock-env` | Use the simulated ambient temp/humidity sensor (implies `[Features] enable_env`) | +| `--degraded` | Start even though declared hardware is missing (field triage; overrides `strict_presence` and says so loudly in the log) | | `--tui` | Show the full-screen terminal dashboard (overrides `[UI] enable_tui`) | | `--no-tui` | Force the headless line console (overrides config; wins over `--tui`) | | `--log-level ` | `trace`/`debug`/`info`/`warn`/`error`/`off` | diff --git a/docs/hardware-config-plan.md b/docs/hardware-config-plan.md index caf8e18..7fa4f1c 100644 --- a/docs/hardware-config-plan.md +++ b/docs/hardware-config-plan.md @@ -426,7 +426,7 @@ Phase 1 is deployable on its own. Phase 2 requires it. ## Progress Branch `worktree-hardware-config` (worktree at `.claude/worktrees/hardware-config`). -216 tests pass, up from a 204 baseline. +247 tests pass, up from a 204 baseline. | § | Item | State | |----|------|-------| @@ -434,8 +434,8 @@ Branch `worktree-hardware-config` (worktree at `.claude/worktrees/hardware-confi | 9 | Calibration `;prev` comments, one generation, changed keys only, stamped with the command | **done**, 7 tests | | 3 | `hardware.ini`: `[Board]` / `[Hardware]` typed, layered over `config.ini`, shadowed keys warned | **done**, 5 tests | | 4 | Resolution chain `--hardware` / `$FGC_HARDWARE` / `/etc/...` / beside `config.ini` | **done**, 2 tests | -| 8 | `config/hardware.example.ini` documenting the shape | **done** (the `hardware//` setup kit is not started) | -| 10 | Presence reconciliation (declared vs found) | **not started** — `hasAxis()` is in place and returns true for an undeclared rig, so callers can adopt it before reconciliation exists | +| 8 | `config/hardware.example.ini` documenting the shape | **done**, and the `hardware/lattepanda-iota/` setup kit now exists too (`install.sh`, `packages.txt`, three precondition files) | +| 10 | Presence reconciliation (declared vs found) | **done**, 13 tests. `HardwarePresence.h` compares `[Hardware]` against what came up; warn-only by default, refusing under `[Features] strict_presence` with `--degraded` as the field-triage escape (decision 4) | | 11 | `test hardware` | **not started** | | 5 | Delete the `Application.cpp:1040` speed literals | **not started** — blocked on decision 2 | diff --git a/docs/hardware-porting-plan.md b/docs/hardware-porting-plan.md index 0844a98..d2eca3d 100644 --- a/docs/hardware-porting-plan.md +++ b/docs/hardware-porting-plan.md @@ -297,7 +297,23 @@ commit `aab1925`. |---|---|---| | 0 | Remote-flash question researched — two viable paths, design rule derived (§2) | **done, and now measured on the board: neither works** (§2). First flash needs the MicroPython REPL or BOOTSEL; remote *update* is unsolved pending a `BOOTSEL` command | | 2 | **Phase 2 gate: RP2040 + TMC5160-BOB at 3.3 V** | **PASSED** on tower 121, 2026-09-15: `IOIN Y=0x3000001E version=0x30`, encoder reading (`X_ENC`), one axis, `speed=3000` | +| 2 | `src/platform.h` — reset cause and free RAM behind one interface per MCU family | **done** | +| 2 | `Serial.begin()` moved ahead of all hardware init, so a wedged image stays flashable | **done** | +| 2 | `[env:iota_rp2040]` PlatformIO target (arduino-pico core, 8 MB flash) | **done and built** on tower 121, 2026-09-15: 49452 B flash (0.6 % of 8 MB), 9068 B RAM. `leonardo` still builds (19254 B) and the 22 native tests still pass | +| 2 | Per-board pin map + `NUM_AXES` in `platform_leonardo.h` / `platform_iota.h`, as-built wiring | **done** — compile-time, not yet generated from `hardware.ini` | +| 2 | Pins + `NUM_AXES` generated from `hardware.ini` | **not started** — needs the config plan's phase 2 first | | 3 | Yaw homing on the real drivetrain | **done** — the endstop was being sheared through at full torque; homing at 189 mA holds. See below | +| 1 | Everything else | blocked on [hardware-config-plan.md](hardware-config-plan.md) | + +> **Not compile-verified.** PlatformIO is not installed on the dev box and the firmware is built +> *remotely on the tower* by `firmware/deploy.sh`. What was verified locally: both branches of +> `platform.h` syntax-check against stub headers — the AVR branch under `gnu++11` (what the Arduino AVR +> core uses) and the RP2040 branch under `gnu++17` (what arduino-pico uses) — and an unrecognised +> architecture fails on the `#error` rather than compiling silently. +> +> To actually build it: `cd firmware && ./deploy.sh --compile-only` compiles on the device without +> flashing. That rsyncs over the tower's firmware source tree, so it is a deliberate step, not one to +> take by surprise. ### Phase 3, first attempt — 2026-09-16 @@ -357,22 +373,6 @@ turns, or did the motor spin while the platform stayed put? Check any cabling through that joint before moving the axis further. -| 2 | `src/platform.h` — reset cause and free RAM behind one interface per MCU family | **done** | -| 2 | `Serial.begin()` moved ahead of all hardware init, so a wedged image stays flashable | **done** | -| 2 | `[env:iota_rp2040]` PlatformIO target (arduino-pico core, 8 MB flash) | **done and built** on tower 121, 2026-09-15: 49452 B flash (0.6 % of 8 MB), 9068 B RAM. `leonardo` still builds (19254 B) and the 22 native tests still pass | -| 2 | Per-board pin map + `NUM_AXES` in `platform_leonardo.h` / `platform_iota.h`, as-built wiring | **done** — compile-time, not yet generated from `hardware.ini` | -| 2 | Pins + `NUM_AXES` generated from `hardware.ini` | **not started** — needs the config plan's phase 2 first | -| 1 | Everything else | blocked on [hardware-config-plan.md](hardware-config-plan.md) | - -> **Not compile-verified.** PlatformIO is not installed on the dev box and the firmware is built -> *remotely on the tower* by `firmware/deploy.sh`. What was verified locally: both branches of -> `platform.h` syntax-check against stub headers — the AVR branch under `gnu++11` (what the Arduino AVR -> core uses) and the RP2040 branch under `gnu++17` (what arduino-pico uses) — and an unrecognised -> architecture fails on the `#error` rather than compiling silently. -> -> To actually build it: `cd firmware && ./deploy.sh --compile-only` compiles on the device without -> flashing. That rsyncs over the tower's firmware source tree, so it is a deliberate step, not one to -> take by surprise. --- diff --git a/docs/modules-reference.md b/docs/modules-reference.md index 3b764ad..94918ac 100644 --- a/docs/modules-reference.md +++ b/docs/modules-reference.md @@ -15,6 +15,7 @@ Per-file reference for the refactored tree, plus the shared data structures. | [include/fgc/CommandParser.h](../include/fgc/CommandParser.h), [src/core/CommandParser.cpp](../src/core/CommandParser.cpp) | `parseCommand` whitespace tokenizer → `Command` | | [include/fgc/HelpText.h](../include/fgc/HelpText.h), [src/core/HelpText.cpp](../src/core/HelpText.cpp) | Operator command catalog (`helpCatalog`) + `renderHelp` (console & TUI help) | | [include/fgc/TmcCurrent.h](../include/fgc/TmcCurrent.h), [src/core/TmcCurrent.cpp](../src/core/TmcCurrent.cpp) | Milliamps → TMC5160 `GLOBALSCALER`/`IRUN` register fields, using the sense resistor from `[Motor.]`. Pure arithmetic, so it is unit-tested; the host owns the conversion because only it knows the resistor | +| [include/fgc/HardwarePresence.h](../include/fgc/HardwarePresence.h), [src/core/HardwarePresence.cpp](../src/core/HardwarePresence.cpp) | `reconcilePresence` (what `hardware.ini` declares vs what actually came up) + `presenceVerdict` (proceed / warn / refuse, per `strict_presence` and `--degraded`). Pure comparison, so the policy is unit-tested without a rig | | [include/fgc/DumpParser.h](../include/fgc/DumpParser.h), [src/core/DumpParser.cpp](../src/core/DumpParser.cpp) | `parseDump` firmware `DUMP` block → `DumpData` (per-axis state + decoded TMC registers/flags); `formatDump` | | [include/fgc/DiagParser.h](../include/fgc/DiagParser.h), [src/core/DiagParser.cpp](../src/core/DiagParser.cpp) | `parseDiag` firmware `DG` self-test stream → `DiagResult` (per-axis tests, PASS/FAIL); `formatDiag` | | [include/fgc/Calibration.h](../include/fgc/Calibration.h), [src/core/Calibration.cpp](../src/core/Calibration.cpp) | `linearFit` (least-squares, R²) + `circularMeanDeg` for the IMU-referenced calibration | diff --git a/include/fgc/Application.h b/include/fgc/Application.h index f34ae1e..514b0ba 100644 --- a/include/fgc/Application.h +++ b/include/fgc/Application.h @@ -13,6 +13,10 @@ struct RuntimeOptions { bool init = false; // run the endstop-finding init sequence bool start = false; // begin capture automatically bool demo = false; // demo mode (copy placeholder image instead of encoding) + // Start anyway when hardware.ini declares something this rig does not have. + // Field triage only: it overrides [Features] strict_presence, and the log + // says so in those words so it cannot become the normal way to run a tower. + bool degraded = false; // Tri-state feature overrides (unset => use the config value). std::optional use_mqtt; diff --git a/include/fgc/Config.h b/include/fgc/Config.h index 5aa2141..ed1a4f0 100644 --- a/include/fgc/Config.h +++ b/include/fgc/Config.h @@ -174,6 +174,13 @@ struct FeaturesConfig { bool mock_imu = false; // use a simulated IMU instead of the MTi bool enable_env = false; // ambient temp/humidity sensor (off by default) bool mock_env = false; // use a simulated env sensor instead of the SHT41 + + // Refuse to start when hardware.ini declares a subsystem this rig does not + // have (HardwarePresence.h). Off by default so that gaining a hardware.ini + // cannot change how a tower behaves: it is turned on per tower once that + // file is known good, and the fleet-wide flip is a config change rather than + // a release. --degraded overrides it for field triage. + bool strict_presence = false; }; // [IMU]: Xsens MTi connected over the LattePanda's RS-232 UART (a hardware diff --git a/include/fgc/HardwarePresence.h b/include/fgc/HardwarePresence.h new file mode 100644 index 0000000..128aa5d --- /dev/null +++ b/include/fgc/HardwarePresence.h @@ -0,0 +1,92 @@ +#pragma once + +#include "fgc/Config.h" + +#include +#include + +namespace fgc { + +// Reconciling what hardware.ini DECLARES against what the rig actually turned +// out to have. +// +// The program degrades gracefully everywhere - TestRunner skips the pitch and +// IMU leaves, CamEvent falls back to encoder pitch, CaptureScheduler sends the +// single-axis MOVE form. That is right for a rig that genuinely lacks them, and +// indistinguishable from a FAILURE on a rig that does not: a 2-axis tower whose +// pitch stops enumerating becomes a yaw-only tower, captures one elevation band, +// and reports itself healthy. Declaring the assembly is what lets startup tell +// those two apart. + +// What this run actually found, from the sources that can answer: telemetry for +// the axes, the constructed sensor objects for the IMU and env sensor, camera +// enumeration for the cameras. +struct FoundHardware { + std::vector axes; // yaw implied by any ST line, pitch by pitch_present + bool imu = false; + bool env = false; + std::vector cameras; // by [Camera.] block name, as configured + + // How many of those the source actually opened. A camera can be configured + // and declared and still not be on the bus, which is the whole failure this + // section exists to catch - but the source reports a count, not which ones + // are missing, so the shortfall is named as a number. -1 = not asked. + int cameras_opened = -1; + + // Telemetry never arrived, so the axis list is unknown rather than empty. + // Without this an unplugged MCU would report every declared axis as missing, + // which is true but useless - the one fault worth naming is the serial link. + bool axes_known = true; +}; + +enum class PresenceIssue { + MissingDeclared, // declared, not found - the failure this exists to catch + UndeclaredFound, // found, not declared - the file is stale +}; + +struct PresenceFinding { + PresenceIssue issue; + std::string subsystem; // "axis pitch", "imu", "env", "camera rgb" + std::string detail; // one operator-readable sentence, naming the fix +}; + +struct PresenceReport { + std::vector findings; + + // False when no hardware.ini was read, or it declared nothing comparable. + // Every tower is in this state until it is migrated, and it is what keeps + // this change invisible during the rollout. + bool reconciled = false; + + bool anyMissing() const; + bool anyUndeclared() const; + std::vector missing() const; // subsystem names, for the refusal line +}; + +// Pure comparison; says nothing about what to do about it. +PresenceReport reconcilePresence(const HardwareConfig& declared, const FoundHardware& found); + +enum class PresenceAction { + Proceed, // nothing to reconcile, or everything matches + WarnOnly, // mismatches, but policy (or --degraded) says carry on + Refuse, // declared hardware is missing and this tower is strict +}; + +struct PresenceVerdict { + PresenceAction action = PresenceAction::Proceed; + std::string reason; // the line to log; empty when there is nothing to say +}; + +// `strict` is [Features] strict_presence, `degraded` is --degraded. +// +// Warn-only is the default deliberately: through the fleet rollout no tower may +// change behaviour on upgrade just by gaining a hardware.ini. A tower flips to +// strict once its file is known good, so the fleet-wide flip is a config change +// rather than a release. +PresenceVerdict presenceVerdict(const PresenceReport& report, bool strict, bool degraded); + +// Every finding as one log-ready line, most serious first. Empty when there are +// none. +std::vector formatPresence(const PresenceReport& report); + +} // namespace fgc diff --git a/main.cpp b/main.cpp index 45dff65..6e1d043 100644 --- a/main.cpp +++ b/main.cpp @@ -29,6 +29,9 @@ int main(int argc, char* argv[]) { "use a simulated ambient temp/humidity sensor (implies enable_env)") ("tui", po::bool_switch(), "show the full-screen terminal dashboard") ("no-tui", po::bool_switch(), "force the headless line console (overrides config)") + ("degraded", po::bool_switch(), + "start even though hardware.ini declares a subsystem this rig does not have " + "(field triage; says so loudly in the log)") ("no-lock", po::bool_switch(), "skip the single-instance lock (dev only: for a second mock-only instance)") ("lock-file", po::value(), "path to the single-instance lockfile") @@ -103,6 +106,7 @@ int main(int argc, char* argv[]) { if (vm["mock-camera"].as()) opts.mock_camera = true; if (vm["mock-serial"].as()) opts.mock_serial = true; if (vm["mock-env"].as()) opts.mock_env = true; + opts.degraded = vm["degraded"].as(); if (vm["tui"].as()) opts.use_tui = true; if (vm["no-tui"].as()) opts.use_tui = false; // --no-tui wins if (vm.count("log-level")) opts.log_level = vm["log-level"].as(); diff --git a/src/core/Application.cpp b/src/core/Application.cpp index c695436..11dbf1f 100644 --- a/src/core/Application.cpp +++ b/src/core/Application.cpp @@ -9,6 +9,7 @@ #include "fgc/HelpText.h" #include "fgc/Paths.h" #include "fgc/CameraProfile.h" +#include "fgc/HardwarePresence.h" #include "fgc/TmcCurrent.h" #include "fgc/ICameraSource.h" #include "fgc/IControlChannel.h" @@ -1256,6 +1257,65 @@ struct Application::Impl { // Request a DUMP and wait briefly for a parseable one. Several callers need // the firmware's own view of itself - the homed limits, and whether it adopted // what we sent - so the round trip lives in one place. + // What this run actually has, for reconciliation against hardware.ini. + // Called once at startup, after every subsystem has been constructed and + // started, because "found" can only mean "came up", not "was configured". + FoundHardware observeHardware(std::chrono::milliseconds wait) { + using namespace std::chrono_literals; + FoundHardware f; + + // Axes come from telemetry, which takes a moment to arrive. Yaw reaching + // any real state is the signal the link is alive; until then a + // default-constructed snapshot would report every axis missing. + MotorTelemetry t = motor->telemetry(); + const auto deadline = std::chrono::steady_clock::now() + wait; + while (running && t.yaw.state == AxisState::Unknown && + std::chrono::steady_clock::now() < deadline) { + std::this_thread::sleep_for(100ms); + t = motor->telemetry(); + } + f.axes_known = t.yaw.state != AxisState::Unknown; + if (f.axes_known) { + f.axes.push_back("yaw"); // any well-formed ST line carries Y: + if (t.pitch_present) f.axes.push_back("pitch"); + } + + // A null source is the "not fitted / not enabled" case; makeImu() and + // makeEnv() already log why. + f.imu = imu != nullptr; + f.env = env != nullptr; + + for (const auto* c : cfg.camera.active()) f.cameras.push_back(c->name); + f.cameras_opened = camera ? camera->cameraCount() : -1; + return f; + } + + // Compare the rig against its declaration and act on the difference. + // Returns false when the run must not continue. + bool reconcileHardware() { + using namespace std::chrono_literals; + if (!cfg.hardware.declared) return true; // nothing declared, nothing to check + + const PresenceReport report = reconcilePresence(cfg.hardware, observeHardware(3s)); + for (const auto& line : formatPresence(report)) LOG_WARN << "hardware: " << line; + + const PresenceVerdict v = + presenceVerdict(report, cfg.features.strict_presence, opts.degraded); + switch (v.action) { + case PresenceAction::Proceed: + LOG_INFO << "hardware: declared assembly confirmed (" << cfg.hardware.axesLabel() + << ")"; + return true; + case PresenceAction::WarnOnly: + LOG_WARN << "hardware: " << v.reason; + return true; + case PresenceAction::Refuse: + LOG_ERROR << "hardware: " << v.reason; + return false; + } + return true; + } + std::optional freshDump(std::chrono::milliseconds wait) { using namespace std::chrono_literals; motor->sendCommand("DUMP"); @@ -2330,6 +2390,19 @@ struct Application::Impl { if (imu) imu->start(); if (env) env->start(); camera->open(); + + // Everything is up, so "found" now means what it says. A tower that + // declares hardware it does not have stops here rather than quietly + // capturing a narrower panorama and reporting itself healthy. + if (!reconcileHardware()) { + camera->close(); + if (imu) imu->stop(); + if (env) env->stop(); + motor->stop(); + channel->disconnect(); + return 3; // distinct from the config (1) and lock (2) exits + } + pipeline->start(); channel->publishStatus(0); diff --git a/src/core/Config.cpp b/src/core/Config.cpp index e27ef46..ee239d6 100644 --- a/src/core/Config.cpp +++ b/src/core/Config.cpp @@ -350,6 +350,8 @@ AppConfig ConfigLoader::fromMap(const std::map& kv) { cfg.features.mock_imu = getBool(kv, "Features.mock_imu", cfg.features.mock_imu); cfg.features.enable_env = getBool(kv, "Features.enable_env", cfg.features.enable_env); cfg.features.mock_env = getBool(kv, "Features.mock_env", cfg.features.mock_env); + cfg.features.strict_presence = + getBool(kv, "Features.strict_presence", cfg.features.strict_presence); cfg.imu.device = get(kv, "IMU.device", cfg.imu.device); cfg.imu.baud = static_cast(getInt(kv, "IMU.baud", cfg.imu.baud)); diff --git a/src/core/HardwarePresence.cpp b/src/core/HardwarePresence.cpp new file mode 100644 index 0000000..459ea36 --- /dev/null +++ b/src/core/HardwarePresence.cpp @@ -0,0 +1,170 @@ +#include "fgc/HardwarePresence.h" + +#include +#include + +namespace fgc { +namespace { + +bool listed(const std::vector& v, const std::string& name) { + return std::find(v.begin(), v.end(), name) != v.end(); +} + +// A sensor slot is declared when it names a model. Empty means the file said +// nothing; "none" means it said "deliberately not fitted" - and the difference +// matters, because only the second one makes a found sensor a stale-file warning. +bool names(const std::string& slot) { return !slot.empty() && slot != "none"; } +bool stated(const std::string& slot) { return !slot.empty(); } + +void missing(PresenceReport& r, std::string subsystem, std::string detail) { + r.findings.push_back({PresenceIssue::MissingDeclared, std::move(subsystem), std::move(detail)}); +} + +void undeclared(PresenceReport& r, std::string subsystem, std::string detail) { + r.findings.push_back({PresenceIssue::UndeclaredFound, std::move(subsystem), std::move(detail)}); +} + +} // namespace + +bool PresenceReport::anyMissing() const { + return std::any_of(findings.begin(), findings.end(), + [](const PresenceFinding& f) { return f.issue == PresenceIssue::MissingDeclared; }); +} + +bool PresenceReport::anyUndeclared() const { + return std::any_of(findings.begin(), findings.end(), + [](const PresenceFinding& f) { return f.issue == PresenceIssue::UndeclaredFound; }); +} + +std::vector PresenceReport::missing() const { + std::vector out; + for (const auto& f : findings) + if (f.issue == PresenceIssue::MissingDeclared) out.push_back(f.subsystem); + return out; +} + +PresenceReport reconcilePresence(const HardwareConfig& declared, const FoundHardware& found) { + PresenceReport r; + + // Nothing was read, so there is nothing to contradict. Every un-migrated + // tower lands here and must behave exactly as it did before. + if (!declared.declared) return r; + r.reconciled = true; + + // ---- axes ---- + // An empty axes list states nothing, matching hasAxis()'s "answers true for + // everything". Reconciling it would turn silence into a fault. + if (!declared.axes.empty()) { + if (!found.axes_known) { + missing(r, "motor telemetry", + "no telemetry arrived, so the declared axes (" + declared.axesLabel() + + ") could not be confirmed -- check [Serial] device and that the MCU is " + "flashed and enumerating"); + } else { + for (const auto& a : declared.axes) + if (!listed(found.axes, a)) + missing(r, "axis " + a, + "declared in " + declared.source_path + + " but absent from telemetry -- the driver is not answering, or " + "the firmware was built for fewer axes than this rig has"); + for (const auto& a : found.axes) + if (!listed(declared.axes, a)) + undeclared(r, "axis " + a, + "present in telemetry but not in [Hardware] axes -- add it, or the " + "declaration is describing a different rig"); + } + } + + // ---- IMU ---- + if (names(declared.imu) && !found.imu) + missing(r, "imu", + "declared as '" + declared.imu + + "' but no IMU came up -- check [IMU] device, and that enable_imu is set"); + if (stated(declared.imu) && !names(declared.imu) && found.imu) + undeclared(r, "imu", "running, but [Hardware] imu says none -- the declaration is stale"); + + // ---- env sensor ---- + if (names(declared.env) && !found.env) + missing(r, "env", + "declared as '" + declared.env + + "' but no ambient sensor came up -- check [Env] i2c_device, and that it is a " + "real I2C adapter rather than SMBus-only"); + if (stated(declared.env) && !names(declared.env) && found.env) + undeclared(r, "env", "running, but [Hardware] env says none -- the declaration is stale"); + + // ---- cameras ---- + if (!declared.cameras.empty()) { + for (const auto& c : declared.cameras) + if (!listed(found.cameras, c)) + missing(r, "camera " + c, + "declared in " + declared.source_path + + " but not found -- check the [Camera." + c + + "] block is enabled and the device is on the bus"); + for (const auto& c : found.cameras) + if (!listed(declared.cameras, c)) + undeclared(r, "camera " + c, + "active but not in [Hardware] cameras -- add it, or the declaration is " + "describing a different rig"); + + // Configured and declared is not the same as present on the bus. The + // source counts what it opened without saying which, so the shortfall is + // reported as a number and `test hardware camera` names the device. + const int want = static_cast(found.cameras.size()); + if (found.cameras_opened >= 0 && found.cameras_opened < want) + missing(r, "cameras", + std::to_string(want) + " configured, only " + + std::to_string(found.cameras_opened) + + " opened -- one is not on the bus; 'test hardware camera' names which"); + } + + return r; +} + +PresenceVerdict presenceVerdict(const PresenceReport& report, bool strict, bool degraded) { + PresenceVerdict v; + if (report.findings.empty()) return v; // Proceed, nothing to say + + if (!report.anyMissing()) { + // Only stale-file warnings. Never fatal: the rig has MORE than was + // declared, which is a documentation fault, not a broken tower. + v.action = PresenceAction::WarnOnly; + v.reason = "hardware.ini is out of date with this rig"; + return v; + } + + std::string gone; + for (const auto& m : report.missing()) gone += (gone.empty() ? "" : ", ") + m; + + if (!strict) { + v.action = PresenceAction::WarnOnly; + v.reason = "declared hardware is missing (" + gone + + "); continuing because [Features] strict_presence is off -- set it once this " + "tower's hardware.ini is known good"; + return v; + } + if (degraded) { + // Loudly, per the plan: this is field triage, and it must not become the + // way a tower is normally run. + v.action = PresenceAction::WarnOnly; + v.reason = "RUNNING DEGRADED BY REQUEST: declared hardware is missing (" + gone + + ") and --degraded overrode the refusal -- this tower is not doing the job it " + "is declared to do"; + return v; + } + v.action = PresenceAction::Refuse; + v.reason = "declared hardware is missing: " + gone + + " -- fix the rig, correct " + + "hardware.ini, or pass --degraded to start anyway"; + return v; +} + +std::vector formatPresence(const PresenceReport& report) { + std::vector out; + for (const auto& f : report.findings) + if (f.issue == PresenceIssue::MissingDeclared) out.push_back(f.subsystem + ": " + f.detail); + for (const auto& f : report.findings) + if (f.issue == PresenceIssue::UndeclaredFound) out.push_back(f.subsystem + ": " + f.detail); + return out; +} + +} // namespace fgc diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f4605a4..575bca1 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -23,6 +23,7 @@ add_executable(fgc_tests test_singleinstance.cpp test_config.cpp test_tmccurrent.cpp + test_hardwarepresence.cpp test_cameraprofile.cpp test_camerasettingsxml.cpp test_hash.cpp diff --git a/tests/test_hardwarepresence.cpp b/tests/test_hardwarepresence.cpp new file mode 100644 index 0000000..f6276dd --- /dev/null +++ b/tests/test_hardwarepresence.cpp @@ -0,0 +1,204 @@ +#include "doctest/doctest.h" +#include "fgc/HardwarePresence.h" + +#include + +using namespace fgc; + +namespace { + +// A 2-axis Delta tower with both sensors and one camera, as hardware.ini +// declares it. +HardwareConfig declaredTower() { + HardwareConfig h; + h.declared = true; + h.source_path = "/etc/fire_gimbal_control/hardware.ini"; + h.axes = {"yaw", "pitch"}; + h.imu = "mti-630"; + h.env = "sht41"; + h.cameras = {"rgb"}; + return h; +} + +// The same tower, working. +FoundHardware foundTower() { + FoundHardware f; + f.axes = {"yaw", "pitch"}; + f.imu = true; + f.env = true; + f.cameras = {"rgb"}; + f.cameras_opened = 1; + return f; +} + +bool mentions(const PresenceReport& r, const std::string& subsystem) { + return std::any_of(r.findings.begin(), r.findings.end(), + [&](const PresenceFinding& f) { return f.subsystem == subsystem; }); +} + +} // namespace + +TEST_CASE("an undeclared tower is never reconciled") { + // Every tower is in this state until it gains a hardware.ini, and gaining + // one must not be what changes its behaviour. + HardwareConfig none; // declared == false + FoundHardware bare; // nothing found at all + bare.axes = {"yaw"}; + + auto r = reconcilePresence(none, bare); + CHECK_FALSE(r.reconciled); + CHECK(r.findings.empty()); + CHECK(presenceVerdict(r, /*strict=*/true, /*degraded=*/false).action == PresenceAction::Proceed); +} + +TEST_CASE("a matching rig produces no findings") { + auto r = reconcilePresence(declaredTower(), foundTower()); + CHECK(r.reconciled); + CHECK(r.findings.empty()); + CHECK_FALSE(r.anyMissing()); + CHECK(presenceVerdict(r, true, false).action == PresenceAction::Proceed); +} + +TEST_CASE("the bug this exists to catch: a declared axis that stopped enumerating") { + // The 2-axis tower whose pitch vanished. Before reconciliation this looked + // exactly like a healthy yaw-only tower. + auto found = foundTower(); + found.axes = {"yaw"}; + + auto r = reconcilePresence(declaredTower(), found); + REQUIRE(r.anyMissing()); + CHECK(mentions(r, "axis pitch")); + CHECK(r.missing() == std::vector{"axis pitch"}); +} + +TEST_CASE("an axis found but not declared is a stale file, never fatal") { + auto declared = declaredTower(); + declared.axes = {"yaw"}; // the file still describes the old single-axis rig + + auto r = reconcilePresence(declared, foundTower()); + CHECK(r.anyUndeclared()); + CHECK_FALSE(r.anyMissing()); + CHECK(mentions(r, "axis pitch")); + + // Strict, no --degraded: still only a warning. More hardware than declared is + // a documentation fault, not a broken tower. + auto v = presenceVerdict(r, /*strict=*/true, /*degraded=*/false); + CHECK(v.action == PresenceAction::WarnOnly); +} + +TEST_CASE("an empty axes list states nothing and is not reconciled") { + // Matches hasAxis(), which answers true for everything when nothing is said. + auto declared = declaredTower(); + declared.axes = {}; + auto found = foundTower(); + found.axes = {"yaw"}; + + auto r = reconcilePresence(declared, found); + CHECK_FALSE(mentions(r, "axis yaw")); + CHECK_FALSE(mentions(r, "axis pitch")); +} + +TEST_CASE("no telemetry at all names the link, not every axis") { + auto found = foundTower(); + found.axes = {}; + found.axes_known = false; + + auto r = reconcilePresence(declaredTower(), found); + REQUIRE(r.anyMissing()); + CHECK(mentions(r, "motor telemetry")); + // The useful fault is the serial link; listing both axes would bury it. + CHECK_FALSE(mentions(r, "axis yaw")); + CHECK_FALSE(mentions(r, "axis pitch")); +} + +TEST_CASE("sensors distinguish 'not fitted' from 'not stated'") { + SUBCASE("declared and missing is a fault") { + auto found = foundTower(); + found.imu = false; + auto r = reconcilePresence(declaredTower(), found); + CHECK(mentions(r, "imu")); + CHECK(r.anyMissing()); + } + SUBCASE("declared 'none' but running is a stale file") { + auto declared = declaredTower(); + declared.imu = "none"; + auto r = reconcilePresence(declared, foundTower()); + CHECK(mentions(r, "imu")); + CHECK_FALSE(r.anyMissing()); + CHECK(r.anyUndeclared()); + } + SUBCASE("not stated at all is silent in both directions") { + auto declared = declaredTower(); + declared.imu = ""; + declared.env = ""; + auto found = foundTower(); + found.env = false; + auto r = reconcilePresence(declared, found); + CHECK_FALSE(mentions(r, "imu")); + CHECK_FALSE(mentions(r, "env")); + } +} + +TEST_CASE("a camera that is configured but never opened is missing") { + auto found = foundTower(); + found.cameras_opened = 0; // configured, declared, not on the bus + + auto r = reconcilePresence(declaredTower(), found); + REQUIRE(r.anyMissing()); + CHECK(mentions(r, "cameras")); +} + +TEST_CASE("an unasked camera count is not a shortfall") { + auto found = foundTower(); + found.cameras_opened = -1; // no camera source in this run + + auto r = reconcilePresence(declaredTower(), found); + CHECK_FALSE(mentions(r, "cameras")); +} + +TEST_CASE("the rollout default is warn-only, and says how to tighten it") { + auto found = foundTower(); + found.axes = {"yaw"}; + auto r = reconcilePresence(declaredTower(), found); + + auto v = presenceVerdict(r, /*strict=*/false, /*degraded=*/false); + CHECK(v.action == PresenceAction::WarnOnly); + CHECK(v.reason.find("strict_presence") != std::string::npos); +} + +TEST_CASE("strict refuses, and names both the fault and the escape") { + auto found = foundTower(); + found.axes = {"yaw"}; + auto r = reconcilePresence(declaredTower(), found); + + auto v = presenceVerdict(r, /*strict=*/true, /*degraded=*/false); + CHECK(v.action == PresenceAction::Refuse); + CHECK(v.reason.find("axis pitch") != std::string::npos); + CHECK(v.reason.find("--degraded") != std::string::npos); +} + +TEST_CASE("--degraded overrides the refusal, and is loud about it") { + auto found = foundTower(); + found.axes = {"yaw"}; + auto r = reconcilePresence(declaredTower(), found); + + auto v = presenceVerdict(r, /*strict=*/true, /*degraded=*/true); + CHECK(v.action == PresenceAction::WarnOnly); + // It must not read like a normal startup line. + CHECK(v.reason.find("DEGRADED") != std::string::npos); +} + +TEST_CASE("formatting puts the faults before the stale-file warnings") { + auto declared = declaredTower(); + declared.axes = {"yaw", "pitch"}; + auto found = foundTower(); + found.axes = {"yaw"}; // pitch missing -> a fault + found.imu = true; + declared.imu = "none"; // imu undeclared -> stale file + + auto r = reconcilePresence(declared, found); + auto lines = formatPresence(r); + REQUIRE(lines.size() == 2); + CHECK(lines[0].rfind("axis pitch:", 0) == 0); + CHECK(lines[1].rfind("imu:", 0) == 0); +}