Send the whole drivetrain on connect, so a restart is enough to retune

This commit is contained in:
pgdalmeida 2026-09-23 07:36:23 +02:00
parent 00c998301a
commit bd3a568508
Signed by: pedro.almeida
GPG Key ID: D4A6C394DF13F1D7
3 changed files with 124 additions and 56 deletions

View File

@ -731,10 +731,10 @@ full torque — the fire it is there to prevent.
### The config handshake — the firmware asks, `hardware.ini` answers ### The config handshake — the firmware asks, `hardware.ini` answers
Today the firmware carries a full parameter table and **guesses** when the host The firmware used to carry a full parameter table and **guess** when the host
says nothing, so every drivetrain value exists in two places and the two can said nothing, so every drivetrain value existed in two places and the two could
disagree. Section 6 of the config plan replaces that: the firmware declares what disagree. It no longer guesses: it declares what it needs and refuses to move
it needs and refuses to move until it has it. until it has it.
``` ```
firmware -> BOOT build=abc123 firmware -> BOOT build=abc123
@ -767,11 +767,23 @@ Three rules, each protecting against a different silent failure:
> `ST` line — deliberately not red, and deliberately not "no telemetry": the link > `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. > 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 > **Retuning needs only a restart.** The firmware asks exactly once, at boot,
> verified end to end, but the firmware that *asks* is not written yet. Set > and only while it is in `NEEDCFG` — so the handshake alone would mean that
> `[Features] mock_needcfg = 1` to make the simulated controller run the > editing `hardware.ini` and restarting the *host* changed nothing, because the
> handshake — emit `NEED` at start, reject `HOME` / `MOVE` / `ENABLE` until every > board is already configured and says nothing to reply to. The host therefore
> requested key is answered — which is how this is exercised on a dev box. > re-sends the whole declared block on every connect, whether or not it was
> asked. Edit, restart, done; no re-flash.
>
> It also makes the handshake robust to a `NEED` line lost on the wire: an axis
> still sitting in `NEEDCFG` is completed by that re-send rather than refusing to
> move until someone notices.
>
> **Both halves are live**, and validated on tower 121: the firmware emits `NEED`
> at boot and refuses `HOME` / `MOVE` / `ENABLE` until every required key is
> answered, and the host answers it. On a dev box with no board attached, set
> `[Features] mock_needcfg = 1` to make the simulated controller run the same
> handshake, or `0` to have it stand in for a board that is already configured —
> which is what exercises the unprompted re-send.
### Why speed is declared in degrees per second ### Why speed is declared in degrees per second

View File

@ -361,8 +361,12 @@ The fix is to make the search weak enough that the screw wins:
Declared per tower in `hardware.ini` as `[Motor.yaw] homing_current_ma`, converted Declared per tower in `hardware.ini` as `[Motor.yaw] homing_current_ma`, converted
to registers by the host and sent as `CURRENT` before every home — so retuning it to registers by the host and sent as `CURRENT` before every home — so retuning it
is a restart, not a reflash. See [configuration.md](configuration.md) for the keys is a restart, not a reflash. **That is now true of the whole drivetrain block**:
and `firmware/docs/hardware-notes.md` for the mechanism. the host re-sends every declared value on connect, not only the ones with their
own command. It was briefly true of the current alone, which made half a
`hardware.ini` edit take effect on a restart and half silently not. See
[configuration.md](configuration.md) for the keys and
`firmware/docs/hardware-notes.md` for the mechanism.
Three things this exposed, all since fixed: Three things this exposed, all since fixed:

View File

@ -1243,54 +1243,100 @@ struct Application::Impl {
LOG_WARN << "unparseable NEED line from the firmware: " << line; LOG_WARN << "unparseable NEED line from the firmware: " << line;
continue; continue;
} }
const bool is_yaw = need->axis == 'Y'; answerNeed(*need, /*asked=*/true);
// An axis the tower says it does not have. That is not a missing
// declaration - it is a firmware built for more axes than this rig,
// and answering would configure a driver that is not there. Same
// class of fault as presence reconciliation catches, so it is named
// that way rather than as a hole in hardware.ini.
const char* axis_name = is_yaw ? "yaw" : "pitch";
if (cfg.hardware.declared && !cfg.hardware.hasAxis(axis_name)) {
LOG_ERROR << "firmware asked for axis " << need->axis << " (" << axis_name
<< "), which this tower does not declare (axes "
<< cfg.hardware.axesLabel()
<< ") -- the firmware was built for more axes than the rig has; "
"rebuild it for this board rather than declaring an axis that is "
"not fitted";
continue;
}
const auto src = is_yaw
? axisConfigSource("yaw", cfg.geometry.yaw, cfg.drivetrain.yaw,
yaw_current_, cfg.yaw_soft_margin_counts)
: axisConfigSource("pitch", cfg.geometry.pitch,
cfg.drivetrain.pitch, pitch_current_,
cfg.pitch_soft_margin_counts);
const auto plan = planFirmwareConfig(*need, src);
for (const auto& cmd : plan.sets) motor->sendCommand(cmd);
if (!plan.unanswered.empty()) {
std::string keys;
for (const auto& k : plan.unanswered) keys += (keys.empty() ? "" : ", ") + k;
LOG_ERROR << "firmware asked axis " << need->axis << " for " << keys
<< " and hardware.ini does not declare "
<< (plan.unanswered.size() == 1 ? "it" : "them")
<< "; the axis stays in NEEDCFG and will not move";
}
if (!plan.unknown.empty()) {
std::string keys;
for (const auto& k : plan.unknown) keys += (keys.empty() ? "" : ", ") + k;
LOG_ERROR << "firmware asked axis " << need->axis << " for " << keys
<< ", which this host does not know -- the firmware is newer than the "
"host; upgrade the host rather than editing hardware.ini";
}
if (plan.complete())
LOG_INFO << "firmware config: answered " << plan.sets.size() << " keys for axis "
<< need->axis;
} }
} }
// Every key the host can answer, by its canonical name. Used to re-send the
// whole declared set on connect - see resendDeclaredConfig().
static NeedRequest fullRequest(char axis) {
NeedRequest r;
r.axis = axis;
r.keys = {"steps_per_rev", "global_scaler", "run_irun", "homing_irun",
"center_count", "soft_margin", "enc_deviation", "normal_speed",
"homing_speed", "center_speed", "sw_mode", "has_encoder"};
return r;
}
// Push the declared drivetrain at the firmware whether or not it asked.
//
// The firmware asks exactly once, at boot, and only while it is in NEEDCFG.
// Without this, editing hardware.ini and restarting the HOST changes nothing
// - the board is already configured, so it says nothing, and the host has
// nothing to reply to. The tower keeps running the old values silently.
// Re-tuning would need a re-flash, which is seven seconds on a bench and a
// trip up a mast in the field.
//
// CENTER and MARGIN are sent again as SET here, having just gone out as
// their own commands. That is idempotent and costs two lines; keeping the
// set uniform is worth more than saving them.
//
// It also makes the handshake robust to a NEED line lost on the wire: an
// axis still in NEEDCFG is completed by this re-send rather than sitting
// there refusing to move.
void resendDeclaredConfig() {
for (const char* name : {"yaw", "pitch"}) {
if (!cfg.hardware.hasAxis(name)) continue;
answerNeed(fullRequest(name[0] == 'y' ? 'Y' : 'P'), /*asked=*/false);
}
}
// `asked` distinguishes the two callers. When the firmware ASKED, a key the
// host cannot supply is a startup failure - the axis stays in NEEDCFG and
// will not move, so it is an error. On a proactive re-send it means only
// "hardware.ini does not declare this one", and the firmware simply keeps
// what it already had; saying ERROR there would cry wolf on every start.
void answerNeed(const NeedRequest& need, bool asked) {
const bool is_yaw = need.axis == 'Y';
// An axis the tower says it does not have. That is not a missing
// declaration - it is a firmware built for more axes than this rig,
// and answering would configure a driver that is not there. Same
// class of fault as presence reconciliation catches, so it is named
// that way rather than as a hole in hardware.ini.
const char* axis_name = is_yaw ? "yaw" : "pitch";
if (cfg.hardware.declared && !cfg.hardware.hasAxis(axis_name)) {
LOG_ERROR << "firmware asked for axis " << need.axis << " (" << axis_name
<< "), which this tower does not declare (axes "
<< cfg.hardware.axesLabel()
<< ") -- the firmware was built for more axes than the rig has; "
"rebuild it for this board rather than declaring an axis that is "
"not fitted";
return;
}
const auto src = is_yaw
? axisConfigSource("yaw", cfg.geometry.yaw, cfg.drivetrain.yaw,
yaw_current_, cfg.yaw_soft_margin_counts)
: axisConfigSource("pitch", cfg.geometry.pitch,
cfg.drivetrain.pitch, pitch_current_,
cfg.pitch_soft_margin_counts);
const auto plan = planFirmwareConfig(need, src);
for (const auto& cmd : plan.sets) motor->sendCommand(cmd);
if (!plan.unanswered.empty() && asked) {
std::string keys;
for (const auto& k : plan.unanswered) keys += (keys.empty() ? "" : ", ") + k;
LOG_ERROR << "firmware asked axis " << need.axis << " for " << keys
<< " and hardware.ini does not declare "
<< (plan.unanswered.size() == 1 ? "it" : "them")
<< "; the axis stays in NEEDCFG and will not move";
}
if (!plan.unknown.empty() && asked) {
std::string keys;
for (const auto& k : plan.unknown) keys += (keys.empty() ? "" : ", ") + k;
LOG_ERROR << "firmware asked axis " << need.axis << " for " << keys
<< ", which this host does not know -- the firmware is newer than the "
"host; upgrade the host rather than editing hardware.ini";
}
if (asked && plan.complete())
LOG_INFO << "firmware config: answered " << plan.sets.size() << " keys for axis "
<< need.axis;
if (!asked && !plan.sets.empty())
LOG_INFO << "drivetrain re-sent to axis " << need.axis << ": " << plan.sets.size()
<< " values from hardware.ini";
}
// Convert this tower's declared milliamps into TMC register fields, once. // Convert this tower's declared milliamps into TMC register fields, once.
// The host owns this because hardware.ini is where the sense resistor lives; // The host owns this because hardware.ini is where the sense resistor lives;
// the firmware is told registers and keeps working compile-time defaults. // the firmware is told registers and keeps working compile-time defaults.
@ -1389,6 +1435,12 @@ struct Application::Impl {
}; };
sendCurrent('Y', yaw_current_); sendCurrent('Y', yaw_current_);
sendCurrent('P', pitch_current_); sendCurrent('P', pitch_current_);
// And the rest of the declared drivetrain. CENTER/MARGIN/CURRENT above
// have always been re-sent on every start; the keys the config handshake
// added were not, so half the tuning took effect on a restart and half
// silently did not. This makes all of it behave the same way.
resendDeclaredConfig();
} }
// Derive each axis's reachable degree window from the freshly homed soft // Derive each axis's reachable degree window from the freshly homed soft