diff --git a/docs/hardware-config-plan.md b/docs/hardware-config-plan.md index 281b23f..6bdba7b 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`). -286 tests pass, up from a 204 baseline. +295 tests pass, up from a 204 baseline. | § | Item | State | |----|------|-------| @@ -440,6 +440,7 @@ Branch `worktree-hardware-config` (worktree at `.claude/worktrees/hardware-confi | 5 | Delete the `Application.cpp` speed literals | **done**, 13 tests. Decision 2 answered: speed is declared as `[Motor.] scan_speed_deg_s` (default **8.0 deg/s**) and converted per tower through `counts_per_deg` (`AxisSpeed.h`). The literals sent 36.4 deg/s against an intended 21.8 | | 6 | Config handshake — **host half** | **done**, 13 tests. `FirmwareConfig.h` parses `NEED` and plans the `SET` lines; unanswered keys and unknown keys are reported separately. `[Features] mock_needcfg` makes the simulated controller run the handshake, so the host half is verified with no firmware. **The firmware half is not written** | | 6 | Config handshake — **firmware half** | **written, NOT compile-verified.** `NEEDCFG` state (`ST` code `C`), `NEED` emitted with the banner, `SET `, `USE DEFAULTS`, and the init block split so the residual-motion stop still runs at boot. Parser verified locally (30 native tests, g++); the rest type-checks against stubs on both board branches but has never been built by PlatformIO or run on hardware | +| 6 | Adoption check generalised | **done**, 9 tests. `DUMP` echoes every host-owned value plus a `cfg_rx` bitmask; `test hardware motor` checks all of them, and reports an absent field as unconfirmable rather than as a mismatch against -1. Round trip NEED -> SET -> DUMP -> PASS verified against `mock_needcfg` | Presence reconciliation (§10) changes nothing on a tower without a `hardware.ini`: absent is silent and `hardware.declared` stays false. Verified by running the binary both ways. diff --git a/docs/test-command.md b/docs/test-command.md index 0ae7238..ce91296 100644 --- a/docs/test-command.md +++ b/docs/test-command.md @@ -54,7 +54,7 @@ While a test runs, interfering commands (moves, homing, a second test, capture, | `host` | `memory` | available RAM / swap (`/proc/meminfo`) | | `host` | `load` | load average vs CPU count | | `hardware` | `axes` | the axes `[Hardware] axes` declares appear in telemetry, **and no others** | -| `hardware` | `motor` | `DUMP` echoes the drivetrain current `hardware.ini` asked for — catches a stale flash or a tower left on limp-home defaults | +| `hardware` | `motor` | `DUMP` echoes **everything** `hardware.ini` declares — current, `steps_per_rev`, `enc_deviation`, `sw_mode` and all three speeds — catching a stale flash, a lost `SET`, or an axis that never left `NEEDCFG` | | `hardware` | `imu` | a declared IMU is connected and a sample arrives | | `hardware` | `env` | a declared ambient sensor sits on a **real I2C** adapter (not SMBus-only) and returns a CRC-valid reading | | `hardware` | `camera` | every configured camera opened; `usbfs_memory_mb` and USB3 LPM hold | @@ -99,6 +99,13 @@ Three rules hold across every leaf: look identical from the sensor's side, and the distinction is what known-issues #2 cost to learn. +The `motor` leaf reports two metrics per axis. `_current` covers +`GLOBALSCALER`/`IRUN`/`homing IRUN`; `_declared` covers everything else the +host owns. Both treat an **absent** field as unconfirmable rather than as a +mismatch — comparing against a missing value would report "firmware has -1", which +reads as a wrong setting and sends someone to re-send a command that was never the +problem. + The `motor` leaf is the one worth running after every flash: `GLOBALSCALER` and `IRUN` are **write-only** on the TMC5160, so the `DUMP` echo is the only evidence a `CURRENT` command landed. A homing current that silently did not take is the diff --git a/include/fgc/DumpParser.h b/include/fgc/DumpParser.h index 7cf0f84..55bdc56 100644 --- a/include/fgc/DumpParser.h +++ b/include/fgc/DumpParser.h @@ -31,6 +31,17 @@ struct DumpAxis { long global_scaler = -1; long run_irun = -1; long homing_irun = -1; + // The rest of what the host declares and the firmware is told. Same -1 + // convention and the same reason: absent means a firmware that predates the + // config handshake, which is a version skew, not a value of zero. + long steps_per_rev = -1; + long enc_deviation = -1; + long homing_speed = -1; + long center_speed = -1; + long sw_mode = -1; + // Bitmask over the firmware's CfgKey: which required keys have arrived. + // -1 when absent. + long cfg_received = -1; // Encoder-tracking health accumulated by the firmware hold corrector. long hold_corrections = 0; // # of hold resyncs since firmware boot/RESET long hold_peak_dev = 0; // largest deviation seen while holding (counts) diff --git a/include/fgc/HardwareChecks.h b/include/fgc/HardwareChecks.h index 991ed5a..60ff736 100644 --- a/include/fgc/HardwareChecks.h +++ b/include/fgc/HardwareChecks.h @@ -51,6 +51,26 @@ struct DrivetrainExpectation { int homing_irun = 0; }; +// One value the host declared and the firmware should be holding. Used for the +// parameters the config handshake added, where "not declared" and "not echoed" +// both mean the check does not apply rather than that it failed. +struct DeclaredValue { + const char* name = ""; + bool want = false; // hardware.ini declares it + long value = 0; + long got = -1; // as DUMP echoed it; -1 = absent +}; + +// Does the firmware hold what hardware.ini says? This is the generalisation of +// the old "did CENTER land?" check: with the drivetrain owned by the host, every +// declared value is one the firmware can be silently wrong about, and a tower +// running on stale or defaulted values looks identical to a correct one from the +// outside. +// +// `values` may be empty, which is how an axis with nothing declared reports +// "not checked" rather than passing. +CheckResult checkDeclaredValues(const std::string& axis, const DeclaredValue* values, int count); + // What DUMP echoed back. -1 means the field was absent, which is a firmware // predating the CURRENT command - a version skew, not a current of zero, and // the two must stay distinguishable. diff --git a/include/fgc/TestRunner.h b/include/fgc/TestRunner.h index 926d055..c8eb15b 100644 --- a/include/fgc/TestRunner.h +++ b/include/fgc/TestRunner.h @@ -76,6 +76,10 @@ struct HardwareTestContext { std::vector configured_cameras; // [Camera.] blocks in open order std::string i2c_device; // [Env] i2c_device, for the adapter-class check + // The degree maps, so a declared deg/s can be turned back into the VMAX the + // firmware should be holding. + AxisMap geo_yaw; + AxisMap geo_pitch; double r_sense_ohm = 0.0; // to recompute what CURRENT should have sent // sysfs nodes, overridable so the checks are exercisable off a tower. diff --git a/include/fgc/mock/MockMotorController.h b/include/fgc/mock/MockMotorController.h index d93fd3c..49f851c 100644 --- a/include/fgc/mock/MockMotorController.h +++ b/include/fgc/mock/MockMotorController.h @@ -32,9 +32,10 @@ public: LOG_INFO << "[mock] motor controller started"; if (!needcfg_) return; std::lock_guard lock(mutex_); - static const char* kKeys[] = {"steps_per_rev", "global_scaler", "sw_mode", - "enc_deviation", "normal_speed", "homing_speed", - "center_speed", "center", "margin"}; + static const char* kKeys[] = {"steps_per_rev", "global_scaler", "run_irun", + "homing_irun", "sw_mode", "enc_deviation", + "normal_speed", "homing_speed", "center_speed", + "center", "margin"}; need_lines_.clear(); unanswered_.clear(); for (const char* axis : {"Y", "P"}) { @@ -112,10 +113,34 @@ public: std::ostringstream d; d << "DUMP BEGIN build=mock uptime=0 mcusr=0x01 free_ram=4096\n"; auto axis = [&](char L, const AxisTelemetry& a) { + // In NEEDCFG mode, echo back what the host actually SET, so the + // whole round trip - NEED, SET, DUMP, `test hardware motor` - + // can be exercised with no firmware. Outside it, stay on the old + // fixed line: that is a firmware predating the handshake, and + // the host must keep telling the two apart. + std::string owned = " speed=200000 center=500000 soft_margin=5000"; + if (needcfg_) { + const std::string ax(1, L); + auto val = [&](const char* k, const char* dflt) { + auto it = settings_.find(ax + "." + k); + return it == settings_.end() ? std::string(dflt) : it->second; + }; + owned = " speed=" + val("normal_speed", "200000") + + " center=" + val("center", "500000") + + " soft_margin=" + val("margin", "5000") + + " steps_per_rev=" + val("steps_per_rev", "0") + + " enc_dev=" + val("enc_deviation", "0") + + " homing_speed=" + val("homing_speed", "0") + + " center_speed=" + val("center_speed", "0") + + " sw_mode=" + val("sw_mode", "0") + + " gs=" + val("global_scaler", "0") + + " irun=" + val("run_irun", "0") + + " irun_home=" + val("homing_irun", "0"); + } d << "DUMP " << L << " state=" << st << " hsub=0 enabled=" << (homed_ ? 1 : 0) << " lim_neg=0 lim_pos=1000000 hold_target=" << a.xenc - << " speed=200000 center=500000 soft_margin=5000" + << owned << " hold_corr=0 hold_peak=0 hold_cumul=0 has_encoder=1\n"; d << "DUMP " << L << " TMC GCONF=0x00000004 GSTAT=0x00000000" << " IOIN=0x30000008 TSTEP=0x000fffff RAMPMODE=0x00000000" diff --git a/src/core/Application.cpp b/src/core/Application.cpp index 5c104f2..e32bed0 100644 --- a/src/core/Application.cpp +++ b/src/core/Application.cpp @@ -2111,6 +2111,8 @@ struct Application::Impl { hw.camera = camera.get(); for (const auto* c : cfg.camera.active()) hw.configured_cameras.push_back(c->name); hw.i2c_device = cfg.env.i2c_device; + hw.geo_yaw = cfg.geometry.yaw; + hw.geo_pitch = cfg.geometry.pitch; hw.usbfs_min_mb = cfg.test.usbfs_min_mb; hw.lpm_path = cfg.test.usb3_lpm_path; diff --git a/src/core/DumpParser.cpp b/src/core/DumpParser.cpp index 235495b..322b117 100644 --- a/src/core/DumpParser.cpp +++ b/src/core/DumpParser.cpp @@ -165,6 +165,12 @@ DumpData parseDump(const std::string& block) { ax.global_scaler = asInt(get(m, "gs", "-1")); ax.run_irun = asInt(get(m, "irun", "-1")); ax.homing_irun = asInt(get(m, "irun_home", "-1")); + ax.steps_per_rev = asInt(get(m, "steps_per_rev", "-1")); + ax.enc_deviation = asInt(get(m, "enc_dev", "-1")); + ax.homing_speed = asInt(get(m, "homing_speed", "-1")); + ax.center_speed = asInt(get(m, "center_speed", "-1")); + ax.sw_mode = asInt(get(m, "sw_mode", "-1")); + ax.cfg_received = asInt(get(m, "cfg_rx", "-1"), -1); ax.hold_corrections = asInt(get(m, "hold_corr", "0")); ax.hold_peak_dev = asInt(get(m, "hold_peak", "0")); ax.hold_cumulative_dev = asInt(get(m, "hold_cumul", "0")); diff --git a/src/core/HardwareChecks.cpp b/src/core/HardwareChecks.cpp index 9499329..cf54c8e 100644 --- a/src/core/HardwareChecks.cpp +++ b/src/core/HardwareChecks.cpp @@ -100,18 +100,32 @@ CheckResult checkDrivetrainAdopted(const std::string& axis, const DrivetrainExpe return r; } - auto mismatch = [&](const char* field, int wanted, long saw) { + // A field is compared only when it was actually echoed. Comparing against -1 + // would report "firmware has -1", which reads as a wrong value rather than + // as a field this firmware does not report - and sends someone to re-send a + // CURRENT that was never the problem. + int missing = 0; + auto compare = [&](const char* field, int wanted, long saw) { + if (saw < 0) { ++missing; return; } + if (saw == wanted) return; r.pass = false; if (!r.detail.empty()) r.detail += "; "; r.detail += std::string(field) + " wanted " + std::to_string(wanted) + ", firmware has " + std::to_string(saw); }; - if (got.global_scaler != want.global_scaler) - mismatch("GLOBALSCALER", want.global_scaler, got.global_scaler); - if (got.run_irun != want.run_irun) mismatch("IRUN", want.run_irun, got.run_irun); - if (got.homing_irun != want.homing_irun) - mismatch("homing IRUN", want.homing_irun, got.homing_irun); + compare("GLOBALSCALER", want.global_scaler, got.global_scaler); + compare("IRUN", want.run_irun, got.run_irun); + compare("homing IRUN", want.homing_irun, got.homing_irun); + + if (missing > 0 && r.pass) { + r.pass = false; + r.detail = axis + ": firmware echoes only part of the current (" + + std::to_string(missing) + + " of 3 fields absent), so it cannot be confirmed -- it predates the full " + "CURRENT echo. Re-flash it"; + return r; + } if (r.pass) { r.detail = axis + ": firmware has the declared current (GLOBALSCALER " + @@ -125,4 +139,49 @@ CheckResult checkDrivetrainAdopted(const std::string& axis, const DrivetrainExpe return r; } +CheckResult checkDeclaredValues(const std::string& axis, const DeclaredValue* values, int count) { + CheckResult r; + + int checked = 0, absent = 0; + for (int i = 0; i < count; ++i) { + const DeclaredValue& v = values[i]; + if (!v.want) continue; // nothing declared: nothing to be wrong about + if (v.got < 0) { + // The firmware does not echo this field at all, which is a version + // skew rather than a mismatch - and it means the value cannot be + // confirmed either way, so it must not read as a pass. + ++absent; + continue; + } + ++checked; + if (v.got != v.value) { + r.pass = false; + if (!r.detail.empty()) r.detail += "; "; + r.detail += std::string(v.name) + " wanted " + std::to_string(v.value) + + ", firmware has " + std::to_string(v.got); + } + } + + if (checked == 0 && absent == 0) { + r.ran = false; + r.detail = axis + ": nothing declared for this axis beyond the current"; + return r; + } + if (absent > 0 && r.pass) { + r.pass = false; + r.detail = axis + ": the firmware does not echo " + std::to_string(absent) + + " declared value(s), so what it is running on cannot be confirmed -- it " + "predates the config handshake. Re-flash it"; + return r; + } + if (!r.pass) { + r.detail = axis + ": " + r.detail + + " -- the firmware is not running what hardware.ini declares. A SET was lost, " + "or the axis never left NEEDCFG"; + return r; + } + r.detail = axis + ": firmware holds all " + std::to_string(checked) + " declared values"; + return r; +} + } // namespace fgc diff --git a/src/core/TestRunner.cpp b/src/core/TestRunner.cpp index 39c26dc..6531bc3 100644 --- a/src/core/TestRunner.cpp +++ b/src/core/TestRunner.cpp @@ -3,6 +3,7 @@ #include "fgc/Calibration.h" // unwrapNear #include "fgc/DiagParser.h" +#include "fgc/AxisSpeed.h" #include "fgc/HardwareChecks.h" #include "fgc/HardwarePresence.h" #include "fgc/ICameraSource.h" @@ -815,6 +816,31 @@ void verdict(TestSection& s, const std::string& name, const CheckResult& c) { s.notes.push_back(name + ": " + c.detail); } +// Pull one axis's echo out of a DUMP, or -1 when the axis or field is absent. +long axisField(const DumpData& d, char code, long DumpAxis::*field) { + for (const auto& ax : d.axes) + if (ax.axis == code) return ax.*field; + return -1; +} +long echoSteps(const DumpData& d, char c) { return axisField(d, c, &DumpAxis::steps_per_rev); } +long echoDev(const DumpData& d, char c) { return axisField(d, c, &DumpAxis::enc_deviation); } +long echoSwMode(const DumpData& d, char c) { return axisField(d, c, &DumpAxis::sw_mode); } +long echoHomingSpeed(const DumpData& d, char c) { return axisField(d, c, &DumpAxis::homing_speed); } +long echoCenterSpeed(const DumpData& d, char c) { return axisField(d, c, &DumpAxis::center_speed); } +long echoSpeed(const DumpData& d, char c) { return axisField(d, c, &DumpAxis::speed); } + +// The VMAX a declared deg/s becomes on this axis, or 0 when it cannot be formed. +// Mirrors Application's fallback: the live calibration when it is trustworthy, +// otherwise the one the declaration implies. +long speedVmax(double deg_s, const AxisDrivetrainConfig& d, const AxisMap& map) { + if (deg_s <= 0.0) return 0; + const double live = std::fabs(map.counts_per_deg); + const double cpd = live >= kMinPlausibleCountsPerDeg ? map.counts_per_deg + : d.preHomeCountsPerDeg(); + const auto p = planAxisSpeed(deg_s, cpd); + return p.ok ? p.vmax : 0; +} + std::string readSysfs(const std::string& path) { if (path.empty()) return ""; std::ifstream f(path); @@ -915,6 +941,24 @@ void TestRunner::testHwMotor(TestReport& r) { got.homing_irun = ax.homing_irun; } verdict(s, std::string(name) + "_current", checkDrivetrainAdopted(name, want, got)); + + // Everything else the host declares and the firmware is told. Before the + // config handshake these lived only in the firmware's own table, where a + // stale or defaulted value was invisible from here. + const AxisMap& map = (code == 'Y') ? hw_.geo_yaw : hw_.geo_pitch; + const long want_normal = speedVmax(cfg.scan_speed_deg_s, cfg, map); + const DeclaredValue vals[] = { + {"steps_per_rev", cfg.steps_per_rev > 0, cfg.steps_per_rev, echoSteps(d, code)}, + {"enc_deviation", cfg.enc_deviation > 0, cfg.enc_deviation, echoDev(d, code)}, + {"sw_mode", cfg.wiring_declared, (long)cfg.sw_mode, echoSwMode(d, code)}, + {"homing_speed", cfg.homing_speed_deg_s > 0, + speedVmax(cfg.homing_speed_deg_s, cfg, map), echoHomingSpeed(d, code)}, + {"center_speed", cfg.center_speed_deg_s > 0, + speedVmax(cfg.center_speed_deg_s, cfg, map), echoCenterSpeed(d, code)}, + {"normal_speed", want_normal > 0, want_normal, echoSpeed(d, code)}, + }; + verdict(s, std::string(name) + "_declared", + checkDeclaredValues(name, vals, (int)(sizeof(vals) / sizeof(vals[0])))); }; check_axis("yaw", 'Y', hw_.drive->yaw); if (!hw_.declared || hw_.declared->hasAxis("pitch")) check_axis("pitch", 'P', hw_.drive->pitch); diff --git a/tests/test_dumpparser.cpp b/tests/test_dumpparser.cpp index 4aca8c5..a81d53b 100644 --- a/tests/test_dumpparser.cpp +++ b/tests/test_dumpparser.cpp @@ -165,3 +165,39 @@ TEST_CASE("a DUMP from firmware without the current fields reports -1, not zero" CHECK(d.axes[0].run_irun == -1); CHECK(d.axes[0].homing_irun == -1); } + +TEST_CASE("the host-owned drivetrain values are read back from DUMP") { + const std::string text = + "DUMP BEGIN build=abc uptime=1 mcusr=0x01 free_ram=4096\n" + "DUMP Y state=2 hsub=0 enabled=1 lim_neg=0 lim_pos=1000000 hold_target=0 speed=39738 " + "center=500000 soft_margin=5000 gs=80 irun=31 irun_home=7 steps_per_rev=1210778 " + "enc_dev=1500 homing_speed=9958 center_speed=29873 sw_mode=0 cfg_rx=0xFFE " + "hold_corr=0 hold_peak=0 hold_cumul=0 has_encoder=1\n" + "DUMP END\n"; + auto d = fgc::parseDump(text); + REQUIRE(d.valid); + REQUIRE(d.axes.size() == 1); + const auto& a = d.axes[0]; + CHECK(a.steps_per_rev == 1210778); + CHECK(a.enc_deviation == 1500); + CHECK(a.homing_speed == 9958); + CHECK(a.center_speed == 29873); + CHECK(a.sw_mode == 0); + CHECK(a.cfg_received == 0xFFE); +} + +TEST_CASE("a firmware predating the handshake leaves those fields absent, not zero") { + // -1 keeps "did not say" distinguishable from "said zero" - sw_mode 0 is a + // legitimate setting on the yaw axis, so zero-as-absent would be a real bug. + const std::string text = + "DUMP BEGIN build=old uptime=1 mcusr=0x01 free_ram=4096\n" + "DUMP Y state=2 hsub=0 enabled=1 lim_neg=0 lim_pos=1000000 hold_target=0 speed=200000 " + "center=500000 soft_margin=5000 hold_corr=0 hold_peak=0 hold_cumul=0 has_encoder=1\n" + "DUMP END\n"; + auto d = fgc::parseDump(text); + REQUIRE(d.valid); + REQUIRE(d.axes.size() == 1); + CHECK(d.axes[0].steps_per_rev == -1); + CHECK(d.axes[0].sw_mode == -1); + CHECK(d.axes[0].cfg_received == -1); +} diff --git a/tests/test_hardwarechecks.cpp b/tests/test_hardwarechecks.cpp index 25f3cbd..6f4e90b 100644 --- a/tests/test_hardwarechecks.cpp +++ b/tests/test_hardwarechecks.cpp @@ -87,3 +87,90 @@ TEST_CASE("every mismatched field is reported, not just the first") { CHECK(r.detail.find("IRUN wanted 31") != std::string::npos); CHECK(r.detail.find("homing IRUN") != std::string::npos); } + +// --- the generalised adoption check (config-plan A.6) ----------------------- + +TEST_CASE("an axis with nothing declared beyond the current is not checked") { + const DeclaredValue none[] = {{"steps_per_rev", false, 0, -1}}; + auto r = checkDeclaredValues("yaw", none, 1); + CHECK_FALSE(r.ran); + CHECK(r.pass); +} + +TEST_CASE("every declared value matching is a pass that says how many") { + const DeclaredValue v[] = { + {"steps_per_rev", true, 1210778, 1210778}, + {"enc_deviation", true, 1500, 1500}, + {"normal_speed", true, 39738, 39738}, + }; + auto r = checkDeclaredValues("yaw", v, 3); + CHECK(r.ran); + CHECK(r.pass); + CHECK(r.detail.find("all 3") != std::string::npos); +} + +TEST_CASE("a firmware that cannot echo a declared value fails, and is named a skew") { + // Not a mismatch: the value cannot be confirmed either way, and a check that + // did not happen must never read as a pass. + const DeclaredValue v[] = { + {"steps_per_rev", true, 1210778, -1}, + {"enc_deviation", true, 1500, -1}, + }; + auto r = checkDeclaredValues("yaw", v, 2); + CHECK(r.ran); + CHECK_FALSE(r.pass); + CHECK(r.detail.find("does not echo 2") != std::string::npos); + CHECK(r.detail.find("Re-flash") != std::string::npos); +} + +TEST_CASE("a lost SET is caught, and every wrong field is named") { + // The failure mode the handshake introduces: most SETs land, one does not, + // and the axis runs on a value nobody declared. + const DeclaredValue v[] = { + {"steps_per_rev", true, 1210778, 1210778}, + {"enc_deviation", true, 1500, 177000}, // stale Delta value + {"homing_speed", true, 9958, 12000}, // firmware default + }; + auto r = checkDeclaredValues("yaw", v, 3); + CHECK_FALSE(r.pass); + CHECK(r.detail.find("enc_deviation wanted 1500, firmware has 177000") != std::string::npos); + CHECK(r.detail.find("homing_speed wanted 9958, firmware has 12000") != std::string::npos); + CHECK(r.detail.find("NEEDCFG") != std::string::npos); +} + +TEST_CASE("an undeclared value is skipped even when the firmware echoes one") { + // hardware.ini not declaring it means the firmware default IS the intent. + const DeclaredValue v[] = { + {"steps_per_rev", true, 1210778, 1210778}, + {"sw_mode", false, 0, 3}, + }; + auto r = checkDeclaredValues("yaw", v, 2); + CHECK(r.pass); + CHECK(r.detail.find("all 1") != std::string::npos); +} + +TEST_CASE("a partly echoed current is a version skew, not a wrong value") { + // The bug this caught: comparing an absent field against -1 reported + // "firmware has -1", which reads as a wrong value and sends someone to + // re-send a CURRENT that was never the problem. + DrivetrainExpectation want{true, 80, 31, 7}; + DrivetrainEcho got; + got.global_scaler = 80; // echoed + got.run_irun = -1; // not echoed + got.homing_irun = -1; + auto r = checkDrivetrainAdopted("yaw", want, got); + CHECK_FALSE(r.pass); + CHECK(r.detail.find("2 of 3 fields absent") != std::string::npos); + CHECK(r.detail.find("firmware has -1") == std::string::npos); +} + +TEST_CASE("a real mismatch alongside an absent field still names the mismatch") { + DrivetrainExpectation want{true, 80, 31, 7}; + DrivetrainEcho got; + got.global_scaler = 255; // wrong + got.run_irun = 31; // right + got.homing_irun = -1; // absent + auto r = checkDrivetrainAdopted("yaw", want, got); + CHECK_FALSE(r.pass); + CHECK(r.detail.find("GLOBALSCALER wanted 80, firmware has 255") != std::string::npos); +}