From b6308c1edaaeaa3ce31ed639c87833cdc24ac723 Mon Sep 17 00:00:00 2001 From: pgdalmeida Date: Wed, 23 Sep 2026 16:07:07 +0200 Subject: [PATCH] Check the tower is running its own settings, not the firmware's fallback --- docs/test-command.md | 23 ++++++++++++++++------- include/fgc/DumpParser.h | 5 +++++ include/fgc/HardwareChecks.h | 17 +++++++++++++++++ src/core/DumpParser.cpp | 5 +++++ src/core/HardwareChecks.cpp | 22 ++++++++++++++++++++++ src/core/TestRunner.cpp | 7 +++++++ tests/test_hardwarechecks.cpp | 29 +++++++++++++++++++++++++++++ 7 files changed, 101 insertions(+), 7 deletions(-) diff --git a/docs/test-command.md b/docs/test-command.md index ce91296..4810372 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 **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` | `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`. Also checks the axis is not on the firmware's **limp-home** fallback, which `USE DEFAULTS` leaves looking fully configured | | `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,12 +99,21 @@ 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 reports three metrics per axis. `_not_limp` asks where the +values came from; `_current` covers `GLOBALSCALER`/`IRUN`/`homing IRUN`; and +`_declared` covers everything else the host owns. All three 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. + +`_not_limp` runs first because it explains the other two. `USE DEFAULTS` is +the deliberate bench escape from `NEEDCFG`, and it fills the firmware's `cfg_rx` +mask completely — so an axis running limp-home values leaves `NEEDCFG`, reports +`READY`, and passes every state check while running numbers that describe no real +drivetrain. Without this metric the only symptom is `_declared` listing six +mismatches, which reads as "host and firmware disagree" and sends someone to +re-send values that were never sent at all. The fix is a host restart, which +re-sends the whole declared block. 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 diff --git a/include/fgc/DumpParser.h b/include/fgc/DumpParser.h index 30a3a7a..1aa2874 100644 --- a/include/fgc/DumpParser.h +++ b/include/fgc/DumpParser.h @@ -42,6 +42,11 @@ struct DumpAxis { // Bitmask over the firmware's CfgKey: which required keys have arrived. // -1 when absent. long cfg_received = -1; + // Is the axis running the firmware's limp-home fallback rather than anything + // this host declared? USE DEFAULTS fills cfg_received with the whole required + // mask, so that field alone cannot tell the two apart - a bench board reads as + // fully configured. -1 when absent, i.e. a firmware predating the flag. + long limp_home = -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 a56ef21..a779de6 100644 --- a/include/fgc/HardwareChecks.h +++ b/include/fgc/HardwareChecks.h @@ -88,6 +88,23 @@ struct DrivetrainEcho { CheckResult checkDrivetrainAdopted(const std::string& axis, const DrivetrainExpectation& want, const DrivetrainEcho& got); +// Is the axis running the host's configuration, or the firmware's limp-home +// fallback? +// +// `USE DEFAULTS` is the deliberate bench escape from NEEDCFG, and it fills the +// firmware's cfg_rx mask completely - so an axis on limp-home values reports as +// fully configured and passes every state check. What gives it away is the DUMP +// `limp` flag, which says where the values came from rather than whether they +// arrived. +// +// checkDeclaredValues() would also catch this, but only as a list of mismatched +// numbers, which reads as "the host and firmware disagree" and sends someone to +// re-send them. The truthful diagnosis is that nobody ever sent any. +// +// `limp` < 0 means the firmware predates the flag: reported as not-run, because +// an old firmware is a version skew rather than a misconfigured tower. +CheckResult checkNotLimpHome(const std::string& axis, long limp); + // Does the homed travel fit entirely in positive counts? // // `center_count` defines where the middle of the found travel sits, so the range diff --git a/src/core/DumpParser.cpp b/src/core/DumpParser.cpp index c43810b..5854bcf 100644 --- a/src/core/DumpParser.cpp +++ b/src/core/DumpParser.cpp @@ -176,6 +176,7 @@ DumpData parseDump(const std::string& block) { 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.limp_home = asInt(get(m, "limp", "-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")); @@ -222,6 +223,10 @@ std::vector formatDump(const DumpData& d) { << " limits=[" << ax.lim_neg << ".." << ax.lim_pos << "]" << " hold_target=" << ax.hold_target << " has_encoder=" << (ax.has_encoder ? 1 : 0); + // Shown only when set. It is a fault on a tower - the axis is on the + // firmware's fallback values, not hardware.ini's - and printing + // "limp=0" on every healthy dump would bury it. + if (ax.limp_home > 0) os << " LIMP-HOME (not hardware.ini's values)"; out.push_back(os.str()); } for (const char* k : {"GCONF", "CHOPCONF", "XACTUAL", "X_ENC", "XTARGET", "VACTUAL"}) { diff --git a/src/core/HardwareChecks.cpp b/src/core/HardwareChecks.cpp index fadf9cf..a99cc6a 100644 --- a/src/core/HardwareChecks.cpp +++ b/src/core/HardwareChecks.cpp @@ -79,6 +79,28 @@ CheckResult checkI2cFunctionality(unsigned long funcs) { return r; } +CheckResult checkNotLimpHome(const std::string& axis, long limp) { + CheckResult r; + + if (limp < 0) { + r.ran = false; + r.detail = axis + + ": firmware does not report the limp flag, so whether it is running " + "hardware.ini's values or its own fallback cannot be told apart here"; + return r; + } + + r.pass = (limp == 0); + r.detail = r.pass + ? axis + ": running the host's declared values" + : axis + + ": running LIMP-HOME values, which describe no real drivetrain -- slow " + "speeds, a reduced search current and a placeholder travel. Something " + "sent USE DEFAULTS, so hardware.ini is NOT in force however healthy the " + "axis looks. Restart the host to re-send it"; + return r; +} + CheckResult checkDrivetrainAdopted(const std::string& axis, const DrivetrainExpectation& want, const DrivetrainEcho& got) { CheckResult r; diff --git a/src/core/TestRunner.cpp b/src/core/TestRunner.cpp index 0088b9a..2e930ec 100644 --- a/src/core/TestRunner.cpp +++ b/src/core/TestRunner.cpp @@ -828,6 +828,7 @@ long echoSwMode(const DumpData& d, char c) { return axisField(d, c, &DumpA 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); } +long echoLimp(const DumpData& d, char c) { return axisField(d, c, &DumpAxis::limp_home); } // 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, @@ -925,6 +926,12 @@ void TestRunner::testHwMotor(TestReport& r) { } auto check_axis = [&](const char* name, char code, const AxisDrivetrainConfig& cfg) { + // First, because it explains the rest. An axis on limp-home values will + // fail every comparison below, and that list of mismatched numbers reads + // as "host and firmware disagree" - sending someone to re-send values + // that were never sent at all. + verdict(s, std::string(name) + "_not_limp", checkNotLimpHome(name, echoLimp(d, code))); + DrivetrainExpectation want; if (cfg.run_current_ma > 0.0 && cfg.r_sense_ohm > 0.0) { const auto plan = planTmcCurrent(cfg.r_sense_ohm, cfg.run_current_ma, diff --git a/tests/test_hardwarechecks.cpp b/tests/test_hardwarechecks.cpp index ee47533..c8a58f1 100644 --- a/tests/test_hardwarechecks.cpp +++ b/tests/test_hardwarechecks.cpp @@ -41,6 +41,35 @@ TEST_CASE("an SMBus-only adapter is caught before it looks like a dead sensor") CHECK(smbus.detail.find("i2cdetect -F") != std::string::npos); } +TEST_CASE("a tower on limp-home values is a fault, however healthy it looks") { + // The case this exists for: USE DEFAULTS fills the firmware's cfg_rx mask + // completely, so the axis leaves NEEDCFG, reports READY and passes every + // state check - while running values that describe no real drivetrain. + auto r = checkNotLimpHome("yaw", 1); + CHECK(r.ran); + CHECK_FALSE(r.pass); + CHECK(r.detail.find("LIMP-HOME") != std::string::npos); + // It must send the reader to the cause, not to the symptom. "Re-send the + // values" is the wrong instruction when nobody ever sent any. + CHECK(r.detail.find("USE DEFAULTS") != std::string::npos); + CHECK(r.detail.find("Restart the host") != std::string::npos); +} + +TEST_CASE("an axis on the host's own values passes") { + auto r = checkNotLimpHome("yaw", 0); + CHECK(r.ran); + CHECK(r.pass); +} + +TEST_CASE("a firmware with no limp flag is a version skew, not a passing tower") { + // Absent must not read as limp=0. A firmware predating the flag genuinely + // cannot answer the question, and claiming PASS here would assert something + // the wire never said - the same -1-is-not-zero rule the current echo uses. + auto r = checkNotLimpHome("yaw", -1); + CHECK_FALSE(r.ran); + CHECK(r.detail.find("cannot be told apart") != std::string::npos); +} + TEST_CASE("an undeclared axis has no drivetrain skew to find") { DrivetrainExpectation want; // declared == false DrivetrainEcho got; // all -1