Restore the pre-IMEX pressure advance output for non-IMEX printers

Closes review comment 6.

The per-tool pressure advance work changed set_pressure_advance() for users who
are not using the feature. RepRapFirmware lost its D qualifier when no tool index
was supplied: upstream emits M572 D0 S<pa> unconditionally, and a bare M572
applies to whatever tool is currently selected and errors when there is none, so
PA started depending on tool-selection state for every RRF user. The D is back,
defaulting to 0, and D<tool> is reached only from the IMEX paths.

The same rewrite had also changed the comment separator from "<value>; Override"
to "<value> ; Override" on the Klipper, RRF, Marlin 2.x and Marlin Legacy
branches, so every non-IMEX print of those flavors carried a one-byte diff.
Restored. Upstream is internally inconsistent here -- BBL and Repetier do use the
spaced form -- and the point is to match it exactly rather than to tidy it.

Emitted output for all six flavors with no tool index is now byte-identical to
upstream. Verified on a real slice: a Klipper profile emits
"SET_PRESSURE_ADVANCE ADVANCE=0.02; Override pressure advance value", an exact
string match, with no EXTRUDER= qualifier. The tests were pinning the regressed
form and are inverted.

Also records at the imex key registrations why they are kept out of the g-code
config block, matching the house convention at the other banned keys.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Clifford Garwood
2026-09-03 00:55:35 -04:00
co-authored by Claude Opus 5
parent 612e0e3932
commit 5629dd29e9
3 changed files with 22 additions and 18 deletions
+7 -8
View File
@@ -845,14 +845,14 @@ SCENARIO("set_pressure_advance emits Klipper form with optional EXTRUDER=extrude
std::string out = writer.set_pressure_advance(0.05);
THEN("Output contains SET_PRESSURE_ADVANCE ADVANCE=0.05 with no EXTRUDER qualifier") {
REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("SET_PRESSURE_ADVANCE"));
REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("ADVANCE=0.05"));
REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("ADVANCE=0.05; Override"));
REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring("EXTRUDER="));
}
}
WHEN("set_pressure_advance is called with tool=0") {
std::string out = writer.set_pressure_advance(0.04, 0);
THEN("Output targets EXTRUDER=extruder (no trailing index)") {
REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("EXTRUDER=extruder "));
REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("EXTRUDER=extruder;"));
REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring("EXTRUDER=extruder0"));
}
}
@@ -865,21 +865,20 @@ SCENARIO("set_pressure_advance emits Klipper form with optional EXTRUDER=extrude
}
}
SCENARIO("set_pressure_advance emits RepRapFirmware form with optional D<N>", "[GCodeWriter][PressureAdvance]") {
SCENARIO("set_pressure_advance emits RepRapFirmware form with D<N>", "[GCodeWriter][PressureAdvance]") {
GIVEN("An RRF-flavored GCodeWriter") {
GCodeWriter writer;
writer.config.gcode_flavor.value = gcfRepRapFirmware;
WHEN("set_pressure_advance is called without a tool index") {
std::string out = writer.set_pressure_advance(0.07);
THEN("Output is bare M572 S... with no D qualifier") {
REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M572 S0.07"));
REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring(" D"));
THEN("Output keeps the historical D0 rather than depending on the selected tool") {
REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M572 D0 S0.07"));
}
}
WHEN("set_pressure_advance is called with tool=0") {
std::string out = writer.set_pressure_advance(0.08, 0);
THEN("Output contains D0 (explicit tool 0, not the current-tool fallback)") {
THEN("Output contains D0, same as the tool-less form") {
REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M572 D0 S0.08"));
}
}
@@ -900,7 +899,7 @@ SCENARIO("set_pressure_advance emits Marlin 2.x form with optional T<N>", "[GCod
WHEN("set_pressure_advance is called without a tool index") {
std::string out = writer.set_pressure_advance(0.10);
THEN("Output is bare M900 K... with no T qualifier") {
REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M900 K0.1 "));
REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M900 K0.1; Override"));
REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring(" T"));
}
}