From 1d023216f226714b7dd95a41db8bac974e299e36 Mon Sep 17 00:00:00 2001 From: SoftFever Date: Wed, 5 Aug 2026 18:09:36 +0800 Subject: [PATCH] clean up --- src/libslic3r/GCode/WipeTower.cpp | 2 + src/libslic3r/GCode/WipeTower.hpp | 15 ++-- src/libslic3r/GCode/WipeTower2.cpp | 6 +- tests/fff_print/test_wipe_tower.cpp | 105 +++++++++++++--------------- 4 files changed, 59 insertions(+), 69 deletions(-) diff --git a/src/libslic3r/GCode/WipeTower.cpp b/src/libslic3r/GCode/WipeTower.cpp index 35d99498eb..b97e773e63 100644 --- a/src/libslic3r/GCode/WipeTower.cpp +++ b/src/libslic3r/GCode/WipeTower.cpp @@ -1345,6 +1345,8 @@ public: { std::string buffer; if (wait_for_moves) + // Not flush_planner_queue_command(): this BBL precool path wants M400, which every + // flavor it reaches understands, not the zero dwell the other flavors flush with. buffer += "M400\n"; buffer += "M104"; if (target_extruder != -1) diff --git a/src/libslic3r/GCode/WipeTower.hpp b/src/libslic3r/GCode/WipeTower.hpp index e6493378d6..0819a04f10 100644 --- a/src/libslic3r/GCode/WipeTower.hpp +++ b/src/libslic3r/GCode/WipeTower.hpp @@ -26,16 +26,11 @@ 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 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); +// Klipper acts on commands the instant it parses them, and its G4 reads only P (milliseconds), +// so the zero-second and seconds-valued dwells every other flavor uses neither synchronize nor +// pause there. Both defined in WipeTower.cpp, shared by WipeTower and WipeTower2. +const char* flush_planner_queue_command(GCodeFlavor flavor); // finish queued moves, e.g. around M104/M109 +std::string wait_command(GCodeFlavor flavor, float seconds); // pause for `seconds` class WipeTower { diff --git a/src/libslic3r/GCode/WipeTower2.cpp b/src/libslic3r/GCode/WipeTower2.cpp index d2abf0a155..0837bfa908 100644 --- a/src/libslic3r/GCode/WipeTower2.cpp +++ b/src/libslic3r/GCode/WipeTower2.cpp @@ -386,8 +386,8 @@ public: } WipeTowerWriter2& switch_filament_monitoring(bool enable) { - m_gcode += flush_planner_queue_command(m_gcode_flavor); - m_gcode += std::string("M591 ") + (enable ? "R" : "S0") + "\n"; + flush_planner_queue(); + m_gcode += enable ? "M591 R\n" : "M591 S0\n"; return *this; } @@ -626,7 +626,7 @@ public: // Set extruder temperature, don't wait by default. WipeTowerWriter2& set_extruder_temp(int temperature, bool wait = false) { - m_gcode += flush_planner_queue_command(m_gcode_flavor); + flush_planner_queue(); m_gcode += "M" + std::to_string(wait ? 109 : 104) + " S" + std::to_string(temperature) + "\n"; return *this; } diff --git a/tests/fff_print/test_wipe_tower.cpp b/tests/fff_print/test_wipe_tower.cpp index 4ea11f7a2a..bb9e4781d1 100644 --- a/tests/fff_print/test_wipe_tower.cpp +++ b/tests/fff_print/test_wipe_tower.cpp @@ -1,7 +1,9 @@ #include #include +#include +#include "libslic3r/GCode/GCodeProcessor.hpp" #include "libslic3r/GCode/WipeTower.hpp" #include "libslic3r/PrintConfig.hpp" @@ -10,13 +12,22 @@ 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"); +// Taken from the config enum map rather than hand-listed, so a flavor added to GCodeFlavor later +// is covered here without editing this file. +static std::vector non_klipper_flavors() +{ + std::vector flavors; + for (const auto &[name, value] : ConfigOptionEnum::get_enum_values()) + if (GCodeFlavor(value) != gcfKlipper) + flavors.push_back(GCodeFlavor(value)); + return flavors; +} + +static std::string flavor_name(GCodeFlavor flavor) +{ + return ConfigOptionEnum::get_enum_names()[int(flavor)]; +} -// 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]") { CHECK(std::string(flush_planner_queue_command(gcfKlipper)) == "M400\n"); @@ -24,16 +35,11 @@ TEST_CASE("Klipper flushes the wipe tower planner queue with M400", "[WipeTower] TEST_CASE("Other flavors flush the wipe tower planner queue with a zero 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)); + const GCodeFlavor flavor = GENERATE(from_range(non_klipper_flavors())); + INFO("gcode flavor: " << flavor_name(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]") { @@ -42,44 +48,39 @@ TEST_CASE("Klipper waits in the wipe tower with a millisecond dwell", "[WipeTowe 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)); + const GCodeFlavor flavor = GENERATE(from_range(non_klipper_flavors())); + INFO("gcode flavor: " << flavor_name(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 cases above only exercise the helpers in isolation. The one below slices a real +// two-filament print, so it also covers the binding constraint of both changes: that the +// configured `gcode_flavor` reaches the wipe tower writer and lands in the exported G-code. -// 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. +// The G-code inside each WIPE_TOWER_START/WIPE_TOWER_END pair, concatenated, so an M400 emitted +// outside the tower (e.g. GCodeProcessor's pre-heat injector) cannot create a false match. static std::string wipe_tower_regions(const std::string &gcode) { + const std::string &start_tag = GCodeProcessor::reserved_tag(GCodeProcessor::ETags::Wipe_Tower_Start); + const std::string &end_tag = GCodeProcessor::reserved_tag(GCodeProcessor::ETags::Wipe_Tower_End); std::string regions; size_t pos = 0; while (true) { - size_t start = gcode.find("WIPE_TOWER_START", pos); + size_t start = gcode.find(start_tag, pos); if (start == std::string::npos) break; - size_t end = gcode.find("WIPE_TOWER_END", start); + size_t end = gcode.find(end_tag, start); if (end == std::string::npos) break; - regions += gcode.substr(start, end - start); + regions.append(gcode, 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. +// test_multifilament.cpp's "Each feature prints with its assigned filament", so the wipe tower +// 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, { @@ -90,41 +91,33 @@ static DynamicPrintConfig wipe_tower_toolchange_config(const std::string &gcode_ { "outer_wall_filament_id", 2 }, { "inner_wall_filament_id", 2 }, { "enable_prime_tower", true }, + { "layer_height", 0.3 }, { "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. +// Slices a 10mm cube under `config`. Not plain Test::slice: a brand-new Print's first `apply()` +// counts one filament in use, and DynamicPrintConfig::normalize_fdm_2's single-filament rule then +// clears `enable_prime_tower`. A second apply, once init_print's regions have settled, sees both +// filaments and the tower survives. static std::string slice_with_prime_tower(const DynamicPrintConfig &config) { Print print; Model model; - init_print({ cube(20) }, print, model, config); + init_print({ cube(10) }, 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]") +TEST_CASE("The wipe tower's toolchange planner flush follows the gcode flavor", "[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")); + auto [flavor, expected, unexpected] = GENERATE(table({ + { "klipper", "M400", "G4 S0" }, + { "marlin", "G4 S0", "M400" } })); + DYNAMIC_SECTION(flavor) { + const std::string tower = wipe_tower_regions(slice_with_prime_tower(wipe_tower_toolchange_config(flavor))); + REQUIRE_FALSE(tower.empty()); + CHECK_THAT(tower, Catch::Matchers::ContainsSubstring(expected)); + CHECK_THAT(tower, !Catch::Matchers::ContainsSubstring(unexpected)); + } }