GGS
/
fwt_software
8
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 1 Wiki Activity

Trust the firmware's reset-cause name over a decode that cannot be right for both boards

This commit is contained in:
pgdalmeida 2026-09-21 19:03:55 +02:00
parent 816242725a
commit e2bdc8debf
Signed by: pedro.almeida
GPG Key ID: D4A6C394DF13F1D7
4 changed files with 62 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
docs/modules-reference.md
View File

@ -19,7 +19,7 @@ Per-file reference for the refactored tree, plus the shared data structures.
| [include/fgc/HardwareChecks.h](../include/fgc/HardwareChecks.h), [src/core/HardwareChecks.cpp](../src/core/HardwareChecks.cpp) | The verdicts behind `test hardware`: `checkUsbfsMemory`, `checkUsb3Lpm`, `checkI2cFunctionality`, `checkDrivetrainAdopted`. Pure, so the thresholds that encode known-issues #2 and #19 cannot drift unnoticed |
| [include/fgc/AxisSpeed.h](../include/fgc/AxisSpeed.h), [src/core/AxisSpeed.cpp](../src/core/AxisSpeed.cpp) | `planAxisSpeed` (deg/s -> TMC `VMAX` through `counts_per_deg`) + `vmaxToDegPerSec`. Pure arithmetic; refuses an uncalibrated scale rather than commanding a near-zero speed |
| [include/fgc/FirmwareConfig.h](../include/fgc/FirmwareConfig.h), [src/core/FirmwareConfig.cpp](../src/core/FirmwareConfig.cpp) | `parseNeedLine` (firmware `NEED` line -> axis + keys) + `planFirmwareConfig` (-> the `SET` lines that answer it). Reports a key it cannot answer separately from one it does not recognise: incomplete `hardware.ini` vs. a firmware newer than the host |
| [include/fgc/DumpParser.h](../include/fgc/DumpParser.h), [src/core/DumpParser.cpp](../src/core/DumpParser.cpp) | `parseDump` firmware `DUMP` block → `DumpData` (per-axis state + decoded TMC registers/flags); `formatDump` |
| [include/fgc/DumpParser.h](../include/fgc/DumpParser.h), [src/core/DumpParser.cpp](../src/core/DumpParser.cpp) | `parseDump` firmware `DUMP` block → `DumpData` (per-axis state + decoded TMC registers/flags); `formatDump`. Prefers the firmware's own `reset=` name over any host-side decode - `mcusr` is an AVR bitmask on one board and a plain enum on the other, so a single table cannot be right for both |
| [include/fgc/DiagParser.h](../include/fgc/DiagParser.h), [src/core/DiagParser.cpp](../src/core/DiagParser.cpp) | `parseDiag` firmware `DG` self-test stream → `DiagResult` (per-axis tests, PASS/FAIL); `formatDiag` |
| [include/fgc/Calibration.h](../include/fgc/Calibration.h), [src/core/Calibration.cpp](../src/core/Calibration.cpp) | `linearFit` (least-squares, R²) + `circularMeanDeg` for the IMU-referenced calibration |
| [include/fgc/PitchSetup.h](../include/fgc/PitchSetup.h), [src/core/PitchSetup.cpp](../src/core/PitchSetup.cpp) | `gimbal pitch setup` worker thread: home-if-needed → pitch sweep dwelling on the gravity-referenced IMU pitch → least-squares fit → park level; tunable via `CalibParams`; exposes `progress()`/`report()`/`takeResult()`. Yaw is not touched |

12
include/fgc/DumpParser.h
View File

@ -66,7 +66,19 @@ struct DumpData {
bool valid = false;
std::string build;
long uptime_ms = 0;
// Why the MCU last restarted, as the firmware reported it.
//
// `mcusr` is the RAW value and means DIFFERENT THINGS per board: on AVR it is
// the MCUSR bitmask (bit 0 power-on, bit 1 external, ...); on RP2040 it is
// rp2040.getResetReason(), a plain enum where 3 is one value, not two bits.
// Decoding one as the other is silently wrong - it reported an RP2040 soft
// reset as "PORF (power-on), EXTRF (external)" on tower 121.
//
// So the firmware now NAMES it in the dump (`reset=<text>`), and that text is
// authoritative because only the board knows its own encoding. reset_flags is
// the legacy AVR bit decode, kept for firmware that predates the field.
unsigned mcusr = 0;
std::string reset_text; // empty => firmware did not name it
long free_ram = 0;
std::vector<std::string> reset_flags;
std::vector<DumpAxis> axes;

14
src/core/DumpParser.cpp
View File

@ -119,6 +119,7 @@ DumpData parseDump(const std::string& block) {
d.uptime_ms = asInt(get(hdr, "uptime", "0"));
d.mcusr = static_cast<unsigned>(asInt(get(hdr, "mcusr", "0")));
d.free_ram = asInt(get(hdr, "free_ram", "0"));
d.reset_text = get(hdr, "reset");
d.reset_flags = bitsSet(d.mcusr, kMcusr);
// Per-axis lines: "DUMP <Y|P> state=.." and "DUMP <Y|P> TMC ..".
@ -199,8 +200,17 @@ std::vector<std::string> formatDump(const DumpData& d) {
}
{
std::ostringstream os;
os << "reset cause (mcusr=0x" << std::hex << d.mcusr << std::dec << "): "
<< join(d.reset_flags);
os << "reset cause (0x" << std::hex << d.mcusr << std::dec << "): ";
if (!d.reset_text.empty()) {
// The firmware named it, so no guessing is involved.
os << d.reset_text;
} else {
// Firmware predating the `reset=` field. The only decode available is
// the AVR one, and on a non-AVR board it is wrong - so say that it is
// an assumption rather than stating it as fact.
os << join(d.reset_flags) << " [assumed AVR MCUSR bits; firmware does "
"not name it]";
}
out.push_back(os.str());
}
for (const auto& ax : d.axes) {

37
tests/test_dumpparser.cpp
View File

@ -223,3 +223,40 @@ TEST_CASE("the numeric state codes did not shift when NEEDCFG was added") {
CHECK(one(4) == "ERROR");
CHECK(one(5) == "NEEDCFG");
}
TEST_CASE("the firmware's own name for the reset cause wins over any decode") {
// mcusr means different things per board: an AVR MCUSR bitmask, or the
// RP2040's getResetReason() enum. 0x3 is "two bits set" on one and a single
// enum value on the other, so a host-side table cannot be right for both.
const std::string text =
"DUMP BEGIN build=x uptime=1 mcusr=0x3 reset=SOFT_RESET free_ram=4096\n"
"DUMP Y state=1 hsub=0 enabled=0 lim_neg=0 lim_pos=0 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";
auto d = fgc::parseDump(text);
REQUIRE(d.valid);
CHECK(d.reset_text == "SOFT_RESET");
std::string out;
for (const auto& l : fgc::formatDump(d)) out += l + "\n";
CHECK(out.find("SOFT_RESET") != std::string::npos);
// The AVR names must not appear when the firmware said what it was.
CHECK(out.find("PORF") == std::string::npos);
CHECK(out.find("EXTRF") == std::string::npos);
}
TEST_CASE("without that field the AVR decode is labelled as an assumption") {
const std::string text =
"DUMP BEGIN build=old uptime=1 mcusr=0x3 free_ram=4096\n"
"DUMP Y state=1 hsub=0 enabled=0 lim_neg=0 lim_pos=0 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";
auto d = fgc::parseDump(text);
REQUIRE(d.valid);
CHECK(d.reset_text.empty());
std::string out;
for (const auto& l : fgc::formatDump(d)) out += l + "\n";
CHECK(out.find("PORF") != std::string::npos); // still decoded
CHECK(out.find("assumed AVR") != std::string::npos); // but not stated as fact
}