From 194ef34080f77d2b14f964279704483c898e15ee Mon Sep 17 00:00:00 2001 From: SoftFever Date: Wed, 5 Aug 2026 17:09:49 +0800 Subject: [PATCH] Wait in the wipe tower with a millisecond dwell on Klipper The wipe tower's "Delay after unloading" never happened on Klipper. It was emitted as G4 S, and Klipper's G4 reads only the P parameter, in milliseconds, so the pause was silently skipped. The option now produces a dwell Klipper actually performs. Also corrects the planner flush rationale, which cited an extruder position reset that Klipper resolves at parse time and does not need synchronized, and adds end-to-end coverage that slices a two-filament print and checks the emitted wipe tower G-code on both a Klipper and a non-Klipper flavor. No change to any other firmware flavor's output, and no shipped profile sets a non-zero delay, so no shipped profile's output moves either. --- src/libslic3r/GCode/WipeTower.cpp | 9 ++- src/libslic3r/GCode/WipeTower.hpp | 15 ++-- src/libslic3r/GCode/WipeTower2.cpp | 2 +- tests/fff_print/test_wipe_tower.cpp | 111 +++++++++++++++++++++++++++- 4 files changed, 125 insertions(+), 12 deletions(-) diff --git a/src/libslic3r/GCode/WipeTower.cpp b/src/libslic3r/GCode/WipeTower.cpp index cfa407ea73..35d99498eb 100644 --- a/src/libslic3r/GCode/WipeTower.cpp +++ b/src/libslic3r/GCode/WipeTower.cpp @@ -622,6 +622,13 @@ const char* flush_planner_queue_command(GCodeFlavor flavor) return flavor == gcfKlipper ? "M400\n" : "G4 S0\n"; } +std::string wait_command(GCodeFlavor flavor, float seconds) +{ + if (flavor == gcfKlipper) + return "G4 P" + std::to_string(std::lround(seconds * 1000.f)) + "\n"; + return "G4 S" + Slic3r::float_to_string_decimal_point(seconds, 3) + "\n"; +} + class WipeTowerWriter { public: @@ -1150,7 +1157,7 @@ public: { if (time==0.f) return *this; - m_gcode += "G4 S" + Slic3r::float_to_string_decimal_point(time, 3) + "\n"; + m_gcode += wait_command(m_gcode_flavor, time); return *this; } diff --git a/src/libslic3r/GCode/WipeTower.hpp b/src/libslic3r/GCode/WipeTower.hpp index a083da1fb2..e6493378d6 100644 --- a/src/libslic3r/GCode/WipeTower.hpp +++ b/src/libslic3r/GCode/WipeTower.hpp @@ -26,14 +26,17 @@ enum GCodeFlavor : unsigned char; Polylines construct_gap_for_skip_points( const Polygon& polygon, const std::vector& skip_points, float wt_width, float gap_length, Polygon& insert_skip_polygon); -// Returns the command that makes the firmware finish its queued moves, so a command or -// custom-G-code boundary right after (resetting the extruder position, entering -// [change_filament_gcode] / [filament_start_gcode]) is not reached early. Klipper acts on -// such commands the moment it parses them, and its G4 reads only P, so the zero dwell the -// other flavors use synchronizes nothing there — M400 does. Defined in WipeTower.cpp, shared -// by WipeTower and WipeTower2. +// Returns the command that makes the firmware finish its queued moves around an M104/M109 +// or custom-G-code boundary. Klipper acts on commands the instant it parses them, and its G4 +// reads only P, so the zero dwell other flavors use synchronizes nothing there — M400 does. +// Defined in WipeTower.cpp, shared by WipeTower and WipeTower2. const char* flush_planner_queue_command(GCodeFlavor flavor); +// Returns the command that pauses for `seconds`. Klipper's G4 reads only P, in +// milliseconds, and ignores S, so the seconds form the other flavors use would dwell zero +// there. Defined in WipeTower.cpp, shared by WipeTower and WipeTower2. +std::string wait_command(GCodeFlavor flavor, float seconds); + class WipeTower { public: diff --git a/src/libslic3r/GCode/WipeTower2.cpp b/src/libslic3r/GCode/WipeTower2.cpp index 5dc78d4785..d2abf0a155 100644 --- a/src/libslic3r/GCode/WipeTower2.cpp +++ b/src/libslic3r/GCode/WipeTower2.cpp @@ -636,7 +636,7 @@ public: { if (time==0.f) return *this; - m_gcode += "G4 S" + Slic3r::float_to_string_decimal_point(time, 3) + "\n"; + m_gcode += wait_command(m_gcode_flavor, time); return *this; } diff --git a/tests/fff_print/test_wipe_tower.cpp b/tests/fff_print/test_wipe_tower.cpp index de5836ba7a..4ea11f7a2a 100644 --- a/tests/fff_print/test_wipe_tower.cpp +++ b/tests/fff_print/test_wipe_tower.cpp @@ -5,11 +5,17 @@ #include "libslic3r/GCode/WipeTower.hpp" #include "libslic3r/PrintConfig.hpp" -using namespace Slic3r; +#include "test_helpers.hpp" -// The wipe tower flushes the firmware's motion queue before a command or custom-G-code -// boundary that must not be reached early (an extruder-position reset, entering custom -// G-code). Klipper acts on those the moment it parses them, and its G4 reads only P, so the +using namespace Slic3r; +using namespace Slic3r::Test; + +// Pins the enum's size: the two GENERATE lists below hand-list every non-Klipper flavor, so a +// 14th `GCodeFlavor` value would silently go untested unless this fails the build first. +static_assert(int(gcfNoExtrusion) == 12, "GCodeFlavor grew: add the new value to the GENERATE lists in this file"); + +// The wipe tower flushes the firmware's motion queue around an M104/M109 or custom-G-code +// boundary. Klipper acts on those the moment it parses them, and its G4 reads only P, so the // zero dwell every other flavor uses is not a flush there. TEST_CASE("Klipper flushes the wipe tower planner queue with M400", "[WipeTower]") { @@ -25,3 +31,100 @@ TEST_CASE("Other flavors flush the wipe tower planner queue with a zero dwell", INFO("gcode flavor enum value: " << int(flavor)); CHECK(std::string(flush_planner_queue_command(flavor)) == "G4 S0\n"); } + +// A timed pause is emitted in seconds for most firmware. Klipper's G4 reads only P, in +// milliseconds, and ignores S, so the seconds form would pause for no time at all there. +// 1.5s is exactly representable as a float, so neither form can drift when rounded. +TEST_CASE("Klipper waits in the wipe tower with a millisecond dwell", "[WipeTower]") +{ + CHECK(wait_command(gcfKlipper, 1.5f) == "G4 P1500\n"); +} + +TEST_CASE("Other flavors wait in the wipe tower with a seconds dwell", "[WipeTower]") +{ + const GCodeFlavor flavor = GENERATE(gcfMarlinLegacy, gcfRepRapFirmware, gcfRepetier, + gcfMarlinFirmware, gcfRepRapSprinter, gcfTeacup, + gcfMakerWare, gcfSailfish, gcfMach3, gcfMachinekit, + gcfSmoothie, gcfNoExtrusion); + INFO("gcode flavor enum value: " << int(flavor)); + CHECK(wait_command(flavor, 1.5f) == "G4 S1.500\n"); +} + +// The two helpers above are only unit-tested in isolation. Nothing yet confirms that a +// Klipper `gcode_flavor` actually reaches the wipe tower writer and lands in the exported +// G-code, which is the binding constraint of both changes above ("only gcfKlipper changes"). +// These slice a real two-filament print and check that. + +// The G-code between each "WIPE_TOWER_START"/"WIPE_TOWER_END" tag pair the wipe tower writes +// around its toolchange chunks, concatenated. Isolates the region the flush/dwell helpers can +// emit into from ordinary object G-code, where an unrelated M400 (e.g. GCodeProcessor's +// pre-heat injector, gated off here since neither test sets enable_pre_heating) would +// otherwise create a false match. +static std::string wipe_tower_regions(const std::string &gcode) +{ + std::string regions; + size_t pos = 0; + while (true) { + size_t start = gcode.find("WIPE_TOWER_START", pos); + if (start == std::string::npos) + break; + size_t end = gcode.find("WIPE_TOWER_END", start); + if (end == std::string::npos) + break; + regions += gcode.substr(start, end - start); + pos = end + 1; + } + return regions; +} + +// A per-layer toolchange between the wall and infill filaments, same shape as +// test_multifilament.cpp's "Each feature prints with its assigned filament", so the wipe +// tower actually runs its toolchange path (and so `flush_planner_queue()`) on every layer. +static DynamicPrintConfig wipe_tower_toolchange_config(const std::string &gcode_flavor) +{ + return multifilament_config(2, { + { "sparse_infill_filament_id", 1 }, + { "internal_solid_filament_id", 1 }, + { "top_surface_filament_id", 1 }, + { "bottom_surface_filament_id", 1 }, + { "outer_wall_filament_id", 2 }, + { "inner_wall_filament_id", 2 }, + { "enable_prime_tower", true }, + { "gcode_flavor", gcode_flavor }, + }); +} + +// Slices a 20mm cube under `config`. Not just `Test::slice(...)`: a brand-new Print's first +// `apply()` call still has no per-feature regions built, so it undercounts the filaments in +// use and lets DynamicPrintConfig::normalize_fdm_2's "single filament" rule turn +// `enable_prime_tower` back off before the wipe tower ever runs. Applying the same config a +// second time, once init_print's first apply has settled those regions, lets that count see +// both filaments so the prime tower stays on. +static std::string slice_with_prime_tower(const DynamicPrintConfig &config) +{ + Print print; + Model model; + init_print({ cube(20) }, print, model, config); + print.apply(model, config); + return gcode(print); +} + +TEST_CASE("Klipper's wipe tower toolchanges flush the planner queue with M400 in exported G-code", "[WipeTower]") +{ + const std::string gcode = slice_with_prime_tower(wipe_tower_toolchange_config("klipper")); + REQUIRE_THAT(gcode, Catch::Matchers::ContainsSubstring("WIPE_TOWER_START")); + + const std::string tower = wipe_tower_regions(gcode); + CHECK_THAT(tower, Catch::Matchers::ContainsSubstring("M400")); + CHECK_THAT(tower, !Catch::Matchers::ContainsSubstring("G4 S0")); +} + +TEST_CASE("Marlin's wipe tower toolchanges keep the zero-dwell flush in exported G-code", "[WipeTower]") +{ + const std::string gcode = slice_with_prime_tower(wipe_tower_toolchange_config("marlin")); + REQUIRE_THAT(gcode, Catch::Matchers::ContainsSubstring("WIPE_TOWER_START")); + + const std::string tower = wipe_tower_regions(gcode); + CHECK_THAT(tower, Catch::Matchers::ContainsSubstring("G4 S0")); + CHECK_THAT(tower, !Catch::Matchers::ContainsSubstring("M400")); +}