Let a declared sensor enable itself, and retire the duplicate flag

This commit is contained in:
pgdalmeida 2026-09-18 15:57:10 +02:00
parent b5d1fac4cb
commit 579c4ec166
Signed by: pedro.almeida
GPG Key ID: D4A6C394DF13F1D7
12 changed files with 164 additions and 19 deletions

View File

@ -78,8 +78,9 @@ baud = 115200
; (an onboard hardware UART), so this is a stable /dev/ttyS* node - NOT a USB ; (an onboard hardware UART), so this is a stable /dev/ttyS* node - NOT a USB
; device and NOT /dev/ttyUSB0-3 (those are the Quectel modem). Find it with ; device and NOT /dev/ttyUSB0-3 (those are the Quectel modem). Find it with
; ls /dev/ttyS* ; dmesg | grep -iE 'ttyS|LPSS|HSUART' (often /dev/ttyS4) ; ls /dev/ttyS* ; dmesg | grep -iE 'ttyS|LPSS|HSUART' (often /dev/ttyS4)
; Enable with [Features] enable_imu = true. The host reconfigures the MTi to a ; Enable by declaring it in hardware.ini as [Hardware] imu = <model>; the legacy
; Euler + calibrated 100 Hz stream at startup. ; [Features] enable_imu applies only on a tower with no hardware.ini. The host
; reconfigures the MTi to a Euler + calibrated 100 Hz stream at startup.
device = device =
baud = 115200 baud = 115200
@ -145,8 +146,9 @@ z = 0
; Ambient temperature/humidity sensor (Adafruit SHT41), wired to the ; Ambient temperature/humidity sensor (Adafruit SHT41), wired to the
; LattePanda's OWN native I2C bus - NOT the Arduino/motor serial link. Bus ; LattePanda's OWN native I2C bus - NOT the Arduino/motor serial link. Bus
; number varies by board; confirm with `i2cdetect -y N` (SHT41 shows up at ; number varies by board; confirm with `i2cdetect -y N` (SHT41 shows up at
; 0x44). Enable with [Features] enable_env = true (or run with --mock-env, ; 0x44). Enable by declaring it in hardware.ini as [Hardware] env = <model>; the
; which implies it). i2c_addr takes decimal or 0x-prefixed hex. ; legacy [Features] enable_env applies only on a tower with no hardware.ini (or
; run with --mock-env, which implies it). i2c_addr takes decimal or 0x-hex.
i2c_device = /dev/i2c-1 i2c_device = /dev/i2c-1
i2c_addr = 0x44 i2c_addr = 0x44
period_ms = 2000 period_ms = 2000
@ -227,11 +229,17 @@ enable_mqtt = true
; no profile XMLs required. For bringing a tower up one subsystem at a time. ; no profile XMLs required. For bringing a tower up one subsystem at a time.
enable_camera = true enable_camera = true
enable_serial = true enable_serial = true
; LEGACY. On a tower with a hardware.ini these are derived from [Hardware] imu
; and [Hardware] env - declaring a sensor there is what enables it, and a flag
; here that contradicts the declaration is warned about and ignored. They remain
; only for towers that have not been migrated.
enable_imu = false enable_imu = false
enable_env = false
; The mocks are NOT legacy: substituting a simulated device is a choice about
; this run, not a statement about what is bolted to the tower, so it stays here.
mock_camera = false mock_camera = false
mock_serial = false mock_serial = false
mock_imu = false mock_imu = false
enable_env = false
mock_env = false mock_env = false
[Test] [Test]

View File

@ -39,8 +39,13 @@ firmware_env = leonardo
; Axes this gimbal has. A yaw-only rig says: axes = yaw ; Axes this gimbal has. A yaw-only rig says: axes = yaw
axes = yaw, pitch axes = yaw, pitch
; Orientation sensor: a model name, or 'none'. Most of the fleet has none. ; Orientation sensor: a model name, or 'none'. Most of the fleet has none.
;
; Naming a model here is what ENABLES it - there is no second key to remember.
; [Features] enable_imu still exists for towers with no hardware.ini, but where
; this file declares a sensor the declaration wins, and a flag contradicting it
; is warned about and ignored.
imu = none imu = none
; Ambient temperature/humidity sensor: a model name, or 'none'. ; Ambient temperature/humidity sensor: a model name, or 'none'. Same rule.
env = sht41 env = sht41
; The [Camera.<name>] blocks expected to be present and found. ; The [Camera.<name>] blocks expected to be present and found.
cameras = rgb cameras = rgb

View File

@ -49,11 +49,11 @@ Parsed and validated by `ConfigLoader` ([src/core/Config.cpp](../src/core/Config
| `Features` | `enable_mqtt` | bool | `true` | Use MQTT (vs null channel) | | `Features` | `enable_mqtt` | bool | `true` | Use MQTT (vs null channel) |
| `Features` | `enable_camera` | bool | `true` | (reserved) | | `Features` | `enable_camera` | bool | `true` | (reserved) |
| `Features` | `enable_serial` | bool | `true` | (reserved) | | `Features` | `enable_serial` | bool | `true` | (reserved) |
| `Features` | `enable_imu` | bool | `false` | Use the Xsens MTi orientation/IMU | | `Features` | `enable_imu` | bool | `false` | **Legacy.** Derived from `[Hardware] imu` wherever that is declared; kept only for towers without a `hardware.ini`. A flag contradicting the declaration is warned about and ignored |
| `Features` | `mock_camera` | bool | `false` | Use the simulated camera | | `Features` | `mock_camera` | bool | `false` | Use the simulated camera |
| `Features` | `mock_serial` | bool | `false` | Use the simulated motor controller | | `Features` | `mock_serial` | bool | `false` | Use the simulated motor controller |
| `Features` | `mock_imu` | bool | `false` | Use the simulated IMU instead of the MTi | | `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` | `enable_env` | bool | `false` | **Legacy.** Derived from `[Hardware] env` wherever that is declared; same rule as `enable_imu` |
| `Features` | `mock_env` | bool | `false` | Use the simulated env sensor instead of the 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 | | `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` | `device` | string | — | MTi serial device (see `[IMU]` note); required when `enable_imu` |
@ -401,6 +401,26 @@ generic `IEnvSensor` interface
([include/fgc/IEnvSensor.h](../include/fgc/IEnvSensor.h)), so swapping in a different ambient sensor ([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. later means adding a new backend, not touching `Application`, the UI, or MQTT.
### One key, not two: declaring a sensor is what enables it
`[Hardware] imu` and `[Hardware] env` name what is fitted. `[Features] enable_imu`
and `enable_env` used to say the same thing a second time, and the two could
disagree — declare `imu = mti-630`, forget the flag, and the program would report
**missing hardware** for what was a missing config key.
The declaration now wins wherever it exists:
| `[Hardware] imu` | `[Features] enable_imu` | Result |
|---|---|---|
| *(absent)* | `true` / `false` | the flag governs, exactly as before — every un-migrated tower |
| `mti-630` | *(absent)* | **enabled.** No second key is needed |
| `mti-630` | `false` | **enabled**, and the clash is warned about so the stale key can be deleted |
| `none` | `true` | **disabled.** A rig that says it has no IMU cannot be argued into having one |
`mock_imu` / `mock_env` / `mock_camera` / `mock_serial` stay in `config.ini` and are
untouched by this: substituting a simulated device is an operational choice about
this *run*, not a statement about what is bolted to the tower.
### Declared vs found — what `hardware.ini` is checked against ### Declared vs found — what `hardware.ini` is checked against
Everything in this program degrades gracefully: `TestRunner` skips the pitch and IMU leaves, `CamEvent` Everything in this program degrades gracefully: `TestRunner` skips the pitch and IMU leaves, `CamEvent`

View File

@ -387,7 +387,7 @@ a yaw-only tower, captures one elevation band, and reports itself healthy.
| ~~Production speeds hardcoded in `Application.cpp` contradict `firmware/src/config.h` — and win~~ **fixed** | `[Motor.<axis>] scan_speed_deg_s` in `hardware.ini`, in degrees per second, converted to VMAX per tower | | ~~Production speeds hardcoded in `Application.cpp` contradict `firmware/src/config.h` — and win~~ **fixed** | `[Motor.<axis>] scan_speed_deg_s` in `hardware.ini`, in degrees per second, converted to VMAX per tower |
| Firmware `MOTOR_CONFIG` duplicates every drivetrain value the host also knows | firmware holds none of them until told | | Firmware `MOTOR_CONFIG` duplicates every drivetrain value the host also knows | firmware holds none of them until told |
| Firmware `NUM_AXES` hardcoded; host infers axes from telemetry | generated from `[Hardware] axes` | | Firmware `NUM_AXES` hardcoded; host infers axes from telemetry | generated from `[Hardware] axes` |
| `[Features] enable_imu` / `enable_env` | derived from `imu` / `env`; the flags retire | | ~~`[Features] enable_imu` / `enable_env`~~ **fixed** | derived from `[Hardware] imu` / `env`; the flags remain only for towers without a `hardware.ini`, and a flag that contradicts the declaration is warned about and ignored |
`mock_camera` / `mock_serial` / `mock_imu` / `mock_env` stay in `config.ini` — dev-box substitution is `mock_camera` / `mock_serial` / `mock_imu` / `mock_env` stay in `config.ini` — dev-box substitution is
operational, and it is what lets all of this be exercised with no hardware. operational, and it is what lets all of this be exercised with no hardware.
@ -426,7 +426,7 @@ Phase 1 is deployable on its own. Phase 2 requires it.
## Progress ## Progress
Branch `worktree-hardware-config` (worktree at `.claude/worktrees/hardware-config`). Branch `worktree-hardware-config` (worktree at `.claude/worktrees/hardware-config`).
269 tests pass, up from a 204 baseline. 273 tests pass, up from a 204 baseline.
| § | Item | State | | § | Item | State |
|----|------|-------| |----|------|-------|

View File

@ -190,6 +190,33 @@ struct FeaturesConfig {
bool strict_presence = false; bool strict_presence = false;
}; };
// Where a sensor's on/off state came from. [Hardware] imu / env DECLARE what is
// fitted; [Features] enable_imu / enable_env are the older flags that said the
// same thing a second time, and could disagree with it.
enum class SensorEnableSource {
Declaration, // [Hardware] imu / env said so - the new, single source
Flag, // no declaration, but the legacy flag was written explicitly
Default, // neither: the built-in default
};
struct SensorEnable {
bool enabled = false;
SensorEnableSource source = SensorEnableSource::Default;
// The legacy flag was present AND contradicted the declaration. The
// declaration wins; this exists so the stale key can be named and deleted
// rather than silently ignored.
bool conflict = false;
};
// Resolve one sensor's enablement.
//
// The declaration wins wherever it exists, which is what retires the flag: a rig
// that says `imu = mti-630` needs no second key agreeing with it, and a rig that
// says `imu = none` cannot be contradicted into enabling one. With nothing
// declared - every tower until it is migrated - the flag governs exactly as
// before.
SensorEnable resolveSensorEnable(const std::string& declared, bool flag_value, bool flag_present);
// [IMU]: Xsens MTi connected over the LattePanda's RS-232 UART (a hardware // [IMU]: Xsens MTi connected over the LattePanda's RS-232 UART (a hardware
// /dev/ttyS* node, stable across reboots — not a USB device). // /dev/ttyS* node, stable across reboots — not a USB device).
struct ImuConfig { struct ImuConfig {

View File

@ -607,7 +607,7 @@ struct Application::Impl {
return; return;
} }
if (!imu) { if (!imu) {
LOG_WARN << "no IMU (needs [Features] enable_imu, or mock_imu)"; LOG_WARN << "no IMU (declare [Hardware] imu, or use --mock-imu)";
return; return;
} }
@ -1836,7 +1836,7 @@ struct Application::Impl {
// gravity, and nothing else on the tower can measure it. Yaw does not - // gravity, and nothing else on the tower can measure it. Yaw does not -
// `gimbal heading setup` uses landmarks instead. // `gimbal heading setup` uses landmarks instead.
if (!imu) { if (!imu) {
LOG_WARN << "gimbal pitch setup requires the IMU (set [Features] enable_imu); " LOG_WARN << "gimbal pitch setup requires the IMU (declare [Hardware] imu); "
"yaw does not - use 'gimbal heading setup'"; "yaw does not - use 'gimbal heading setup'";
return; return;
} }
@ -1984,7 +1984,7 @@ struct Application::Impl {
} }
// IMU leaves need an IMU; drop them (with a warning) if absent. // IMU leaves need an IMU; drop them (with a warning) if absent.
if ((mask & T_NEEDS_IMU) && !imu) { if ((mask & T_NEEDS_IMU) && !imu) {
LOG_WARN << "test: IMU tests skipped (set [Features] enable_imu)"; LOG_WARN << "test: IMU tests skipped (declare [Hardware] imu)";
mask &= ~T_NEEDS_IMU; mask &= ~T_NEEDS_IMU;
} }
// Motor leaves need a homed gimbal. // Motor leaves need a homed gimbal.

View File

@ -353,6 +353,29 @@ AppConfig ConfigLoader::fromMap(const std::map<std::string, std::string>& kv) {
cfg.features.strict_presence = cfg.features.strict_presence =
getBool(kv, "Features.strict_presence", cfg.features.strict_presence); getBool(kv, "Features.strict_presence", cfg.features.strict_presence);
// [Hardware] imu / env are the declaration; enable_imu / enable_env are the
// older flags that said it a second time. Resolve them into one answer here,
// once, so nothing downstream has to know there were ever two keys - and so a
// rig cannot declare a sensor and then be told it is disabled.
{
auto settle = [&](const char* what, const std::string& declared, const char* flag_key,
bool& out) {
const SensorEnable e =
resolveSensorEnable(declared, out, kv.count(flag_key) > 0);
if (e.conflict)
LOG_WARN << "[Hardware] " << what << " = '" << declared << "' and [" << flag_key
<< "] disagree; the declaration wins (" << what << " "
<< (e.enabled ? "enabled" : "disabled")
<< ") -- delete the flag, it is what hardware.ini is for";
else if (e.source == SensorEnableSource::Declaration && kv.count(flag_key))
LOG_INFO << "[" << flag_key << "] is redundant: [Hardware] " << what
<< " already declares it";
out = e.enabled;
};
settle("imu", cfg.hardware.imu, "Features.enable_imu", cfg.features.enable_imu);
settle("env", cfg.hardware.env, "Features.enable_env", cfg.features.enable_env);
}
cfg.imu.device = get(kv, "IMU.device", cfg.imu.device); cfg.imu.device = get(kv, "IMU.device", cfg.imu.device);
cfg.imu.baud = static_cast<unsigned>(getInt(kv, "IMU.baud", cfg.imu.baud)); cfg.imu.baud = static_cast<unsigned>(getInt(kv, "IMU.baud", cfg.imu.baud));
@ -525,6 +548,21 @@ AppConfig ConfigLoader::fromMap(const std::map<std::string, std::string>& kv) {
return cfg; return cfg;
} }
SensorEnable resolveSensorEnable(const std::string& declared, bool flag_value, bool flag_present) {
SensorEnable r;
if (declared.empty()) {
// Nothing stated, so the legacy flag still governs. This is every tower
// that has not been migrated, and it must behave exactly as before.
r.enabled = flag_value;
r.source = flag_present ? SensorEnableSource::Flag : SensorEnableSource::Default;
return r;
}
r.enabled = (declared != "none");
r.source = SensorEnableSource::Declaration;
r.conflict = flag_present && (flag_value != r.enabled);
return r;
}
bool HardwareConfig::hasAxis(const std::string& name) const { bool HardwareConfig::hasAxis(const std::string& name) const {
// An undeclared rig answers true for everything - see the header. // An undeclared rig answers true for everything - see the header.
if (axes.empty()) return true; if (axes.empty()) return true;

View File

@ -79,7 +79,8 @@ PresenceReport reconcilePresence(const HardwareConfig& declared, const FoundHard
if (names(declared.imu) && !found.imu) if (names(declared.imu) && !found.imu)
missing(r, "imu", missing(r, "imu",
"declared as '" + declared.imu + "declared as '" + declared.imu +
"' but no IMU came up -- check [IMU] device, and that enable_imu is set"); "' but no IMU came up -- check [IMU] device; declaring it here is what "
"enables it");
if (stated(declared.imu) && !names(declared.imu) && found.imu) if (stated(declared.imu) && !names(declared.imu) && found.imu)
undeclared(r, "imu", "running, but [Hardware] imu says none -- the declaration is stale"); undeclared(r, "imu", "running, but [Hardware] imu says none -- the declaration is stale");

View File

@ -144,7 +144,7 @@ void PitchSetupRoutine::run() {
// 1. Preconditions. // 1. Preconditions.
if (!imu_.connected()) { if (!imu_.connected()) {
LOG_WARN << "pitch setup aborted: IMU not connected (set [Features] enable_imu)"; LOG_WARN << "pitch setup aborted: IMU not connected (declare [Hardware] imu)";
return; return;
} }

View File

@ -941,7 +941,8 @@ void TestRunner::testHwImu(TestReport& r) {
if (!imu_) { if (!imu_) {
c.pass = false; c.pass = false;
c.detail = "declared as '" + hw_.declared->imu + c.detail = "declared as '" + hw_.declared->imu +
"' but no IMU source was built -- check [Features] enable_imu and [IMU] device"; "' but no IMU source was built -- check [IMU] device; the declaration itself is "
"what enables it";
} else if (!imu_->connected()) { } else if (!imu_->connected()) {
c.pass = false; c.pass = false;
c.detail = "source exists but is not connected -- the port opened without a 0xFA-framed " c.detail = "source exists but is not connected -- the port opened without a 0xFA-framed "
@ -1002,7 +1003,8 @@ void TestRunner::testHwEnv(TestReport& r) {
if (!hw_.env) { if (!hw_.env) {
m.pass = false; m.pass = false;
m.detail = "declared as '" + hw_.declared->env + m.detail = "declared as '" + hw_.declared->env +
"' but no sensor was built -- check [Features] enable_env"; "' but no sensor was built -- check [Env] i2c_device; the declaration itself is "
"what enables it";
} else if (auto sample = hw_.env->sample()) { } else if (auto sample = hw_.env->sample()) {
m.detail = "CRC-valid reading from " + hw_.env->name(); m.detail = "CRC-valid reading from " + hw_.env->name();
} else { } else {

View File

@ -1058,7 +1058,7 @@ Element sensorsDetailPanel(const ImuView& v, const EnvView& e, ScrollState& st)
| color(Color::Magenta)); | color(Color::Magenta));
if (!v.present) { if (!v.present) {
body.push_back(text("(no IMU data)") | color(Color::Red) | bold); body.push_back(text("(no IMU data)") | color(Color::Red) | bold);
body.push_back(text("check [Features] enable_imu and [IMU] device, or --mock-imu") | dim); body.push_back(text("declare [Hardware] imu and set [IMU] device, or --mock-imu") | dim);
} else { } else {
body.push_back(hbox({ body.push_back(hbox({
text("ORIENTATION") | dim | size(WIDTH, EQUAL, 14), text("ORIENTATION") | dim | size(WIDTH, EQUAL, 14),
@ -1103,7 +1103,7 @@ Element sensorsDetailPanel(const ImuView& v, const EnvView& e, ScrollState& st)
if (!e.enabled) { if (!e.enabled) {
body.push_back(text("(env sensor disabled)") | dim); body.push_back(text("(env sensor disabled)") | dim);
body.push_back( body.push_back(
text("enable with [Features] enable_env and [Env] i2c_device, or --mock-env") text("declare [Hardware] env and set [Env] i2c_device, or --mock-env")
| dim); | dim);
} else if (!e.present) { } else if (!e.present) {
body.push_back(text("(no reading)") | color(Color::Red) | bold); body.push_back(text("(no reading)") | color(Color::Red) | bold);

View File

@ -691,3 +691,47 @@ TEST_CASE("non-numeric values still compare as text") {
{{"device", "/dev/ttyUSB0"}}, "a save"); {{"device", "/dev/ttyUSB0"}}, "a save");
CHECK(out.find(";prev device = /dev/ttyACM0") != std::string::npos); CHECK(out.find(";prev device = /dev/ttyACM0") != std::string::npos);
} }
// --- [Hardware] imu/env retiring [Features] enable_imu/enable_env (plan A.12) ---
TEST_CASE("an undeclared rig still obeys the legacy flag exactly as before") {
// Every tower until it is migrated. Behaviour must not change.
auto on = resolveSensorEnable("", true, true);
CHECK(on.enabled);
CHECK(on.source == SensorEnableSource::Flag);
CHECK_FALSE(on.conflict);
auto off = resolveSensorEnable("", false, true);
CHECK_FALSE(off.enabled);
auto absent = resolveSensorEnable("", false, false);
CHECK_FALSE(absent.enabled);
CHECK(absent.source == SensorEnableSource::Default);
}
TEST_CASE("a declared sensor needs no second key agreeing with it") {
// The trap this removes: declare the IMU, forget enable_imu, and presence
// reconciliation reports missing HARDWARE for what is a missing flag.
auto e = resolveSensorEnable("mti-630", /*flag_value=*/false, /*flag_present=*/false);
CHECK(e.enabled);
CHECK(e.source == SensorEnableSource::Declaration);
CHECK_FALSE(e.conflict);
}
TEST_CASE("'none' cannot be contradicted into enabling a sensor") {
auto e = resolveSensorEnable("none", /*flag_value=*/true, /*flag_present=*/true);
CHECK_FALSE(e.enabled);
CHECK(e.source == SensorEnableSource::Declaration);
CHECK(e.conflict); // named, so the stale key can be deleted
}
TEST_CASE("a declaration beats a flag that disagrees, and the clash is reported") {
auto e = resolveSensorEnable("sht41", /*flag_value=*/false, /*flag_present=*/true);
CHECK(e.enabled);
CHECK(e.conflict);
// Agreeing is not a conflict, merely redundant.
auto agree = resolveSensorEnable("sht41", true, true);
CHECK(agree.enabled);
CHECK_FALSE(agree.conflict);
}