Understand the NEEDCFG state instead of reading it as a dead link

This commit is contained in:
pgdalmeida 2026-09-18 17:15:16 +02:00
parent 6f88b00856
commit 041ad2a18f
Signed by: pedro.almeida
GPG Key ID: D4A6C394DF13F1D7
11 changed files with 76 additions and 5 deletions

View File

@ -723,6 +723,10 @@ Three rules, each protecting against a different silent failure:
- **A partial answer does not unlock the axis.** Most keys landing is the common
case when a declaration is nearly complete, and it must not look like success.
> An axis waiting on its config reports `NEEDCFG` in the dashboard and `C` in the
> `ST` line — deliberately not red, and deliberately not "no telemetry": the link
> is fine and the axis is talking, it just has not been told what it is driving.
>
> **Status: the host half only.** The host answers `NEED` and the responder is
> verified end to end, but the firmware that *asks* is not written yet. Set
> `[Features] mock_needcfg = 1` to make the simulated controller run the

View File

@ -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`).
295 tests pass, up from a 204 baseline.
298 tests pass, up from a 204 baseline.
| § | Item | State |
|----|------|-------|
@ -442,6 +442,7 @@ Branch `worktree-hardware-config` (worktree at `.claude/worktrees/hardware-confi
| 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** | **done and built** on both targets 2026-09-18 (iota_rp2040 and leonardo, `./deploy.sh --compile-only`); behaviour still unproven on a running rig. `NEEDCFG` state (`ST` code `C`), `NEED` emitted with the banner, `SET <axis> <key> <value>`, `USE DEFAULTS`, and the init block split so the residual-motion stop still runs at boot. 33 native tests pass under `pio test -e native`. **Behaviour on a running rig is the one check still outstanding** |
| 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` |
| 6 | `NEEDCFG` understood host-side | **done**, 5 tests. `AxisState::NeedConfig` parsed from `ST` `C`, named in `DUMP` and the dashboard, and counted as telemetry arriving rather than as a dead link. The firmware enum APPENDS the state (5) rather than inserting it — inserting renumbered every later state and made a READY axis read back as ERROR |
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.

View File

@ -86,7 +86,7 @@ Per-file reference for the refactored tree, plus the shared data structures.
### `MotorTelemetry` / `AxisTelemetry` ([IMotorController.h](../include/fgc/IMotorController.h))
Per-axis `yaw` and `pitch` segments (`pitch_present` flag), each an `AxisTelemetry`: `state`
(`AxisState` B/R/H/A/E), `xactual`, `xenc` (encoder counts), `drv_status`, `sg`/`cs`/`pwm`, and flags
(`AxisState` B/R/H/A/E/C — the last being NEEDCFG, a firmware waiting to be told its drivetrain), `xactual`, `xenc` (encoder counts), `drv_status`, `sg`/`cs`/`pwm`, and flags
`standstill`/`stall`/`deviation`/`overtemp`/`endstop_l`/`endstop_r`, with `moving()`/`ready()` helpers.
Parsed from the firmware `ST Y:...[ P:...]` line; degrees are derived via `Geometry`.

View File

@ -6,7 +6,13 @@
namespace fgc {
// Per-axis lifecycle state, from the firmware ST line's state char (B/R/H/A/E).
enum class AxisState { Boot, Reset, Homing, Ready, Error, Unknown };
// Per-axis lifecycle, from the firmware ST line's state char.
// B Boot R Reset H Homing A reAdy E Error C NeedConfig
//
// NeedConfig means the firmware has been told nothing about the drivetrain it is
// driving and is refusing to move until the host answers its NEED lines. It is
// NOT a fault and NOT "no telemetry": the link is fine and the axis is talking.
enum class AxisState { Boot, Reset, Homing, Ready, Error, NeedConfig, Unknown };
// One axis segment of a firmware `ST` status line:
// ST Y:<state>,<xactual>,<xenc>,<drv_hex>,<sg>,<cs>,<pwm>,<flags> [P:...]

View File

@ -109,7 +109,9 @@ public:
// and main.cpp) so the host-side DumpParser is exercised without
// hardware. Register values are illustrative, not simulated; the
// encoder positions track the mock's live state.
const int st = homed_ ? 3 : 1; // READY : RESET
// 5 = NEEDCFG, appended to the firmware enum so the older codes keep
// their meanings. See motor.h.
const int st = (needcfg_ && !configured_) ? 5 : (homed_ ? 3 : 1);
std::ostringstream d;
d << "DUMP BEGIN build=mock uptime=0 mcusr=0x01 free_ram=4096\n";
auto axis = [&](char L, const AxisTelemetry& a) {
@ -240,7 +242,10 @@ private:
a.standstill = false;
}
a.xactual = a.xenc;
a.state = homed_ ? AxisState::Ready : AxisState::Reset;
// An axis waiting on its config says so, rather than looking like a
// healthy un-homed one - that distinction is the whole point of the state.
a.state = (needcfg_ && !configured_) ? AxisState::NeedConfig
: (homed_ ? AxisState::Ready : AxisState::Reset);
a.deviation = deviation_;
}

View File

@ -16,6 +16,10 @@ const char* stateName(int s) {
case 2: return "HOMING";
case 3: return "READY";
case 4: return "ERROR";
// Appended in the firmware enum rather than placed in lifecycle order,
// because these numbers are on the wire: inserting it would renumber
// every later state and a READY axis would read back as ERROR.
case 5: return "NEEDCFG";
default: return "?";
}
}

View File

@ -29,6 +29,7 @@ AxisState stateFromChar(char c) {
case 'H': case 'h': return AxisState::Homing;
case 'A': case 'a': return AxisState::Ready;
case 'E': case 'e': return AxisState::Error;
case 'C': case 'c': return AxisState::NeedConfig;
default: return AxisState::Unknown;
}
}

View File

@ -867,6 +867,9 @@ void TestRunner::testHwAxes(TestReport& r) {
// its own segment.
MotorTelemetry t = motor_.telemetry();
FoundHardware f;
// NEEDCFG counts as telemetry arriving: the link is fine and the axis is
// talking, it just has not been told what it is driving. Treating it as "no
// telemetry" would blame the serial port for a configuration gap.
f.axes_known = t.yaw.state != AxisState::Unknown;
if (f.axes_known) {
f.axes.push_back("yaw");

View File

@ -13,6 +13,7 @@ const char* axisStateLabel(AxisState s) {
case AxisState::Homing: return "HOMING";
case AxisState::Ready: return "READY";
case AxisState::Error: return "ERROR";
case AxisState::NeedConfig: return "NEEDCFG";
case AxisState::Unknown:
default: return "----";
}
@ -23,6 +24,9 @@ UiColor axisStateColor(AxisState s) {
case AxisState::Ready:
case AxisState::Homing: return UiColor::Cyan;
case AxisState::Error: return UiColor::Red;
// Not red: nothing is broken, the firmware is waiting to be told what it
// is driving. Yellow so it reads as "needs you", not as a fault.
case AxisState::NeedConfig: return UiColor::Yellow;
case AxisState::Boot:
case AxisState::Reset: return UiColor::Dim;
case AxisState::Unknown:

View File

@ -201,3 +201,25 @@ TEST_CASE("a firmware predating the handshake leaves those fields absent, not ze
CHECK(d.axes[0].sw_mode == -1);
CHECK(d.axes[0].cfg_received == -1);
}
TEST_CASE("the numeric state codes did not shift when NEEDCFG was added") {
// NEEDCFG is appended (5), not inserted in lifecycle order, because these
// numbers are on the wire. Had it gone in at 1, a READY axis (which would
// have become 4) would read back as ERROR - a healthy tower reporting a
// fault in every dump, and an old firmware misread by a new host.
auto one = [](int n) {
const std::string text =
"DUMP BEGIN build=x uptime=1 mcusr=0x01 free_ram=1\n"
"DUMP Y state=" + std::to_string(n) +
" hsub=0 enabled=1 lim_neg=0 lim_pos=1 hold_target=0 speed=1 center=0 "
"soft_margin=0 hold_corr=0 hold_peak=0 hold_cumul=0 has_encoder=1\n"
"DUMP END\n";
return fgc::parseDump(text).axes.at(0).state_name;
};
CHECK(one(0) == "BOOT");
CHECK(one(1) == "RESET");
CHECK(one(2) == "HOMING");
CHECK(one(3) == "READY");
CHECK(one(4) == "ERROR");
CHECK(one(5) == "NEEDCFG");
}

View File

@ -82,3 +82,24 @@ TEST_CASE("parseAxisSegment reads the encoder-deviation flag 'd'") {
CHECK(e->stall == true);
CHECK(e->deviation == true);
}
TEST_CASE("the NEEDCFG state char is understood, not read as unknown") {
// The firmware refuses to move until the host answers its NEED lines, and
// says so with 'C'. A host that did not know the code would map it to
// Unknown, which downstream reads as "no telemetry arrived" - blaming the
// serial link for a configuration gap.
auto t = fgc::parseTelemetryLine("ST Y:C,0,0,00000000,0,0,0,S");
REQUIRE(t);
CHECK(t->yaw.state == fgc::AxisState::NeedConfig);
CHECK_FALSE(t->yaw.state == fgc::AxisState::Unknown);
auto lower = fgc::parseTelemetryLine("ST Y:c,0,0,00000000,0,0,0,S");
REQUIRE(lower);
CHECK(lower->yaw.state == fgc::AxisState::NeedConfig);
}
TEST_CASE("an axis in NEEDCFG is not ready") {
auto t = fgc::parseTelemetryLine("ST Y:C,0,0,00000000,0,0,0,S");
REQUIRE(t);
CHECK_FALSE(t->yaw.ready());
}