From 5434a5217cd38e7e6776e39d4d3a60efc93cec9e Mon Sep 17 00:00:00 2001 From: Clifford Garwood Date: Fri, 24 Apr 2026 22:53:24 -0400 Subject: [PATCH] test(gcode): per-firmware coverage for GCodeWriter::set_pressure_advance(tool) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exercises the IMEX per-tool PA emission surface added in af59501f4a ("feat: firmware-agnostic per-tool PA emission for IMEX parallel modes"). Six scenarios cover the full routing matrix: - Negative PA returns empty across all flavors (early-exit guard). - Klipper: bare vs EXTRUDER=extruder vs EXTRUDER=extruderN. Asserts the tool=0 case emits the unsuffixed extruder name (first Klipper extruder is named "extruder", not "extruder0") — a subtle edge case easy to regress. - RRF: bare vs D0 vs DN. The D0 case matters: passing tool=0 explicitly must emit `D0`, not the current-tool fallback. - Marlin 2.x: bare vs T0 vs TN. - Marlin Legacy: tool index is silently dropped — verifies the fallback branch can't accidentally start emitting T qualifiers on firmware that doesn't support them. - BBL: flag wins over firmware flavor (Marlin 2 flavor + BBL flag emits the BBL-specific `M900 K... L1000 M10`) and BBL never emits a per-tool qualifier regardless of the tool argument. All 25 assertions across 6 cases pass under [PressureAdvance]. Co-Authored-By: Claude Opus 4.7 --- tests/fff_print/test_gcodewriter.cpp | 145 +++++++++++++++++++++++++++ 1 file changed, 145 insertions(+) diff --git a/tests/fff_print/test_gcodewriter.cpp b/tests/fff_print/test_gcodewriter.cpp index ef8fb58b41..e88009cfbe 100644 --- a/tests/fff_print/test_gcodewriter.cpp +++ b/tests/fff_print/test_gcodewriter.cpp @@ -68,6 +68,151 @@ SCENARIO("lift() is not ignored after unlift() at normal values of Z", "[GCodeWr } } +SCENARIO("set_pressure_advance emits nothing for negative PA", "[GCodeWriter][PressureAdvance]") { + GIVEN("A default GCodeWriter") { + GCodeWriter writer; + THEN("Negative PA returns empty regardless of firmware flavor") { + writer.config.gcode_flavor.value = gcfKlipper; + REQUIRE(writer.set_pressure_advance(-1.0).empty()); + writer.config.gcode_flavor.value = gcfRepRapFirmware; + REQUIRE(writer.set_pressure_advance(-0.001).empty()); + writer.config.gcode_flavor.value = gcfMarlinFirmware; + REQUIRE(writer.set_pressure_advance(-100.0).empty()); + } + } +} + +SCENARIO("set_pressure_advance emits Klipper form with optional EXTRUDER=extruder", "[GCodeWriter][PressureAdvance]") { + GIVEN("A Klipper-flavored GCodeWriter") { + GCodeWriter writer; + writer.config.gcode_flavor.value = gcfKlipper; + + WHEN("set_pressure_advance is called without a tool index") { + 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("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=extruder0")); + } + } + WHEN("set_pressure_advance is called with tool=3") { + std::string out = writer.set_pressure_advance(0.06, 3); + THEN("Output targets EXTRUDER=extruder3") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("EXTRUDER=extruder3")); + } + } + } +} + +SCENARIO("set_pressure_advance emits RepRapFirmware form with optional D", "[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")); + } + } + 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)") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M572 D0 S0.08")); + } + } + WHEN("set_pressure_advance is called with tool=2") { + std::string out = writer.set_pressure_advance(0.09, 2); + THEN("Output contains D2") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M572 D2 S0.09")); + } + } + } +} + +SCENARIO("set_pressure_advance emits Marlin 2.x form with optional T", "[GCodeWriter][PressureAdvance]") { + GIVEN("A Marlin 2-flavored GCodeWriter") { + GCodeWriter writer; + writer.config.gcode_flavor.value = gcfMarlinFirmware; + + 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(" T")); + } + } + WHEN("set_pressure_advance is called with tool=0") { + std::string out = writer.set_pressure_advance(0.11, 0); + THEN("Output contains T0") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M900 K0.11 T0")); + } + } + WHEN("set_pressure_advance is called with tool=1") { + std::string out = writer.set_pressure_advance(0.12, 1); + THEN("Output contains T1") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M900 K0.12 T1")); + } + } + } +} + +SCENARIO("set_pressure_advance emits Marlin Legacy form without tool qualifier even when tool index is supplied", + "[GCodeWriter][PressureAdvance]") { + GIVEN("A Marlin Legacy-flavored GCodeWriter") { + GCodeWriter writer; + writer.config.gcode_flavor.value = gcfMarlinLegacy; + + WHEN("set_pressure_advance is called without a tool index") { + std::string out = writer.set_pressure_advance(0.05); + THEN("Output is bare M900 K...") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M900 K0.05")); + REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring(" T")); + } + } + WHEN("set_pressure_advance is called with tool=2 (a hypothetical IMEX secondary)") { + std::string out = writer.set_pressure_advance(0.06, 2); + THEN("Output is still bare M900 — Marlin Legacy has no per-tool LA") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M900 K0.06")); + REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring(" T2")); + } + } + } +} + +SCENARIO("set_pressure_advance emits BBL M900 L1000 M10 regardless of tool index", + "[GCodeWriter][PressureAdvance]") { + GIVEN("A BBL-flagged GCodeWriter (the flag overrides firmware flavor routing)") { + GCodeWriter writer; + writer.set_is_bbl_machine(true); + // Flavor intentionally set to something other than the BBL branch to prove the flag wins. + writer.config.gcode_flavor.value = gcfMarlinFirmware; + + WHEN("set_pressure_advance is called without a tool index") { + std::string out = writer.set_pressure_advance(0.05); + THEN("Output is the BBL-specific M900 Kx L1000 M10 form") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M900 K0.05 L1000 M10")); + } + } + WHEN("set_pressure_advance is called with a tool index") { + std::string out = writer.set_pressure_advance(0.05, 2); + THEN("BBL output is unchanged — no per-tool qualifier is emitted on BBL printers") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M900 K0.05 L1000 M10")); + REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring(" T2")); + REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring("EXTRUDER=")); + } + } + } +} + SCENARIO("set_speed emits values with fixed-point output.", "[GCodeWriter]") { GIVEN("GCodeWriter instance") {