From e98750e713a6d6748a4acf12c06a73bbd2f50b2d Mon Sep 17 00:00:00 2001 From: SoftFever Date: Sun, 12 Jul 2026 21:04:19 +0800 Subject: [PATCH] fix(engine): real-change-only toolchange ordinals and a single M1020 toolchange command A dual-nozzle H2C print with support filament hangs at its first nozzle switch. The emitted file shows the change-filament block's M620 O ordinal jumping from O1 straight to O230, plus a duplicate "M1020 S" toolchange command right after every change block. Two causes, fixed together because they interlock (the ordinal check keys off the same toolchange detection that suppresses the duplicate): - append_tcr incremented m_toolchange_count once per prime-tower visit (roughly once per layer), while the change-filament template only emits its M620 O{toolchange_count + 1} line on real filament changes. With 229 change-less sparse tower layers below the first support layer, the first real change reported ordinal 230. The counter now advances only when the expanded change block really contains a toolchange command, and the placeholder exposes the upcoming change's ordinal (count + 1). The set_extruder path already counted per real change and is unchanged. - toolchange_prefix() returned "M1020 S" for BBL printers, so the custom_gcode_changes_tool() dedup could never match the stock profiles' line-leading "T[next_filament_id] ..." commands and the writer's own toolchange was appended after every change block on dual-extruder machines. The prefix is now the plain "T" (the manual-filament-change tag branch stays first), and the M1020 form moved into GCodeWriter::toolchange() as an explicit branch that also carries the nozzle: "M1020 S H". The nozzle parameter is signed on purpose: the null-safe nozzle lookup legitimately yields -1, matching the stock templates' own H-1 convention. The prefix change also lets the CoolingBuffer recognize the change blocks' T commands as tool boundaries on BBL printers (its per-filament attribution previously keyed off the duplicate M1020, or nothing at all on single-extruder models); its existing out-of-range guard ignores T1000-class machine commands. Verification: full suites green (libslic3r 48998 assertions / 169 cases; fff_print 692 / 65 including three new scenarios - writer emission per printer kind, dedup + ordinal progression on sequential prints, and a prime-tower regression scenario verified to fail against the old per-visit counting). Byte gate: 18 of 20 fixtures bit-identical; the sequential repro differs by exactly its 3 removed duplicate M1020 lines, deterministic across two runs. Reslicing the field project that exposed the hang yields M620 O1 followed by a gapless O2..O59 and zero duplicate M1020 lines. Co-authored-by: songwei.li --- src/libslic3r/GCode.cpp | 18 ++- src/libslic3r/GCodeWriter.cpp | 36 ++--- src/libslic3r/GCodeWriter.hpp | 2 +- tests/fff_print/test_gcodewriter.cpp | 217 ++++++++++++++++++++++++++- 4 files changed, 246 insertions(+), 27 deletions(-) diff --git a/src/libslic3r/GCode.cpp b/src/libslic3r/GCode.cpp index 78bb80dbf9..4ce936f9b7 100644 --- a/src/libslic3r/GCode.cpp +++ b/src/libslic3r/GCode.cpp @@ -927,9 +927,6 @@ static std::vector get_path_of_change_filament(const Print& print) } } - //BBS: increase toolchange count - gcodegen.m_toolchange_count++; - std::string toolchange_gcode_str; ZHopType z_hope_type = ZHopType(gcodegen.config().z_hop_types.get_at(gcodegen.get_filament_config_index((int)gcodegen.writer().filament()->id()))); @@ -1058,7 +1055,7 @@ static std::vector get_path_of_change_filament(const Print& print) config.set_key_value("max_layer_z", new ConfigOptionFloat(gcodegen.m_max_layer_z)); config.set_key_value("relative_e_axis", new ConfigOptionBool(full_config.use_relative_e_distances)); - config.set_key_value("toolchange_count", new ConfigOptionInt((int) gcodegen.m_toolchange_count)); + config.set_key_value("toolchange_count", new ConfigOptionInt((int) gcodegen.m_toolchange_count + 1)); // BBS: fan speed is useless placeholer now, but we don't remove it to avoid // slicing error in old change_filament_gcode in old 3MF config.set_key_value("fan_speed", new ConfigOptionInt((int) 0)); @@ -1180,7 +1177,10 @@ static std::vector get_path_of_change_filament(const Print& print) std::string toolchange_command; if (tcr.priming || (new_filament_id >= 0 && gcodegen.writer().need_toolchange(new_filament_id))) - toolchange_command = gcodegen.writer().toolchange(new_filament_id); + // Orca: null-safe, layer-aware nozzle lookup — group_result may be null on + // non-multi-nozzle paths (the helper falls back to the extruder id). + toolchange_command = gcodegen.writer().toolchange(new_filament_id, + nozzle_id_for_gcode_placeholder(group_result, new_filament_id, new_extruder_id, m_layer_idx)); if (!custom_gcode_changes_tool(toolchange_gcode_str, gcodegen.writer().toolchange_prefix(), new_filament_id)) toolchange_gcode_str += toolchange_command; else { @@ -1302,6 +1302,10 @@ static std::vector get_path_of_change_filament(const Print& print) std::string tcr_gcode, tcr_escaped_gcode = gcodegen.placeholder_parser_process("tcr_rotated_gcode", tcr_rotated_gcode, new_filament_id, &config); unescape_string_cstyle(tcr_escaped_gcode, tcr_gcode); gcode += tcr_gcode; + // Count the toolchange only when the emitted block really changed the tool — + // tower visits without a filament change must not advance the ordinal. + if (custom_gcode_changes_tool(tcr_gcode, gcodegen.writer().toolchange_prefix(), new_filament_id)) + gcodegen.m_toolchange_count++; check_add_eol(toolchange_gcode_str); // SoftFever: set new PA for new filament @@ -8648,7 +8652,7 @@ std::string GCode::set_extruder(unsigned int new_filament_id, double print_z, bo m_pa_processor->resetPreviousPA(m_config.pressure_advance.get_at(new_filament_id)); } - gcode += m_writer.toolchange(new_filament_id); + gcode += m_writer.toolchange(new_filament_id, new_extruder_id); if (Extruder *fil = m_writer.filament()) fil->set_config_index((int)get_filament_config_index((int)fil->id())); return gcode; @@ -8933,7 +8937,7 @@ std::string GCode::set_extruder(unsigned int new_filament_id, double print_z, bo //BBS: don't add T[next extruder] if there is no T cmd on filament change //We inform the writer about what is happening, but we may not use the resulting gcode. - std::string toolchange_command = m_writer.toolchange(new_filament_id); + std::string toolchange_command = m_writer.toolchange(new_filament_id, next_nozzle_id); if (Extruder *fil = m_writer.filament()) fil->set_config_index((int)get_filament_config_index((int)fil->id())); if (!custom_gcode_changes_tool(toolchange_gcode_parsed, m_writer.toolchange_prefix(), new_filament_id)) diff --git a/src/libslic3r/GCodeWriter.cpp b/src/libslic3r/GCodeWriter.cpp index f32840f614..bbf69fe181 100644 --- a/src/libslic3r/GCodeWriter.cpp +++ b/src/libslic3r/GCodeWriter.cpp @@ -594,23 +594,15 @@ std::string GCodeWriter::update_progress(unsigned int num, unsigned int tot, boo std::string GCodeWriter::toolchange_prefix() const { - std::string gcode = "T"; + // Orca: the manual-filament-change tag must stay ahead of the flavor selection so + // MMU manual-change handling keeps working. if (config.manual_filament_change) - gcode = ";" + GCodeProcessor::reserved_tag(GCodeProcessor::ETags::Manual_Tool_Change) + "T"; - else { - if (m_is_bbl_printers) - gcode = "M1020 S"; - else { - if (FLAVOR_IS(gcfMakerWare)) - gcode = "M135 T"; - else if (FLAVOR_IS(gcfSailfish)) - gcode = "M108 T"; - } - } - return gcode; + return ";" + GCodeProcessor::reserved_tag(GCodeProcessor::ETags::Manual_Tool_Change) + "T"; + return FLAVOR_IS(gcfMakerWare) ? "M135 T" : + FLAVOR_IS(gcfSailfish) ? "M108 T" : "T"; } -std::string GCodeWriter::toolchange(unsigned int filament_id) +std::string GCodeWriter::toolchange(unsigned int filament_id, int nozzle_id) { // set the new extruder auto filament_extruder_iter = Slic3r::lower_bound_by_predicate(m_filament_extruders.begin(), m_filament_extruders.end(), [filament_id](const Extruder &e) { return e.id() < filament_id; }); @@ -621,9 +613,16 @@ std::string GCodeWriter::toolchange(unsigned int filament_id) // return the toolchange command // if we are running a single-extruder setup, just set the extruder and return nothing std::ostringstream gcode; + // Orca: also emit for non-BBL single-extruder multi-filament setups (MMU-style). if (this->multiple_extruders || (this->config.filament_diameter.values.size() > 1 && !is_bbl_printers())) { - // Orca: call toolchange_prefix() to get the correct command prefix based on the configuration and flavor. - gcode << this->toolchange_prefix() << filament_id; + // Orca: manual filament change keeps its tag line even on BBL machines, so the + // M1020 form must not shadow it. nozzle_id is signed: the null-safe nozzle + // lookup legitimately yields -1 ("no specific nozzle"), matching the literal + // H-1 the stock change templates emit; an unsigned would wrap. + if (m_is_bbl_printers && !config.manual_filament_change) + gcode << "M1020 S" << filament_id << " H" << nozzle_id; + else + gcode << this->toolchange_prefix() << filament_id; if (GCodeWriter::full_gcode_comment) gcode << " ; change extruder"; gcode << "\n"; @@ -1271,8 +1270,9 @@ std::string GCodeWriter::set_extruder(unsigned int filament_id) auto filament_ext_it = Slic3r::lower_bound_by_predicate(m_filament_extruders.begin(), m_filament_extruders.end(), [filament_id](const Extruder &e) { return e.id() < filament_id; }); unsigned int extruder_id = filament_ext_it->extruder_id(); assert(filament_ext_it != m_filament_extruders.end() && filament_ext_it->id() == filament_id); - //TODO: optmize here, pass extruder_id to toolchange - return this->need_toolchange(filament_id) ? this->toolchange(filament_id) : ""; + // Orca: writer-only context (calibration paths) has no nozzle grouping; the + // filament's own extruder id is the correct degenerate nozzle value. + return this->need_toolchange(filament_id) ? this->toolchange(filament_id, (int) extruder_id) : ""; } void GCodeWriter::init_extruder(unsigned int filament_id) diff --git a/src/libslic3r/GCodeWriter.hpp b/src/libslic3r/GCodeWriter.hpp index 205ad0cfbb..f387bfc868 100644 --- a/src/libslic3r/GCodeWriter.hpp +++ b/src/libslic3r/GCodeWriter.hpp @@ -72,7 +72,7 @@ public: // Prefix of the toolchange G-code line, to be used by the CoolingBuffer to separate sections of the G-code // printed with the same extruder. std::string toolchange_prefix() const; - std::string toolchange(unsigned int filament_id); + std::string toolchange(unsigned int filament_id, int nozzle_id); std::string set_speed(double F, const std::string &comment = std::string(), const std::string &cooling_marker = std::string()); // SoftFever NOTE: the returned speed is mm/minute double get_current_speed() const { return m_current_speed;} diff --git a/tests/fff_print/test_gcodewriter.cpp b/tests/fff_print/test_gcodewriter.cpp index df8cfb33f1..401c86cd01 100644 --- a/tests/fff_print/test_gcodewriter.cpp +++ b/tests/fff_print/test_gcodewriter.cpp @@ -1,6 +1,10 @@ #include +#include #include +#include +#include +#include #include "libslic3r/GCodeWriter.hpp" #include "libslic3r/GCode.hpp" @@ -417,7 +421,7 @@ SCENARIO("Extruder reads the injected config column", "[GCodeWriter][H2C]") { // Filament-indexed arrays keep one entry per filament. writer.config.filament_diameter.values = {1.75, 1.75}; writer.set_extruders({0, 1}); - writer.toolchange(1); + writer.toolchange(1, 1); Extruder *fil = writer.filament(); REQUIRE(fil != nullptr); REQUIRE(fil->id() == 1); @@ -456,3 +460,214 @@ SCENARIO("Extruder reads the injected config column", "[GCodeWriter][H2C]") { } } } + +// Numeric argument of every line starting with `prefix`, in file order. +static std::vector collect_line_args(const std::string &gcode, const std::string &prefix) +{ + std::vector values; + std::istringstream stream(gcode); + std::string line; + while (std::getline(stream, line)) + if (line.compare(0, prefix.size(), prefix) == 0) + values.push_back(std::atoi(line.c_str() + int(prefix.size()))); + return values; +} + +static int count_lines_with_prefix(const std::string &gcode, const std::string &prefix) +{ + return (int) collect_line_args(gcode, prefix).size(); +} + +// A toolchange ordinal sequence is healthy when it advances by exactly one per +// change block; a change-less prime-tower visit must not consume an ordinal. +static bool ordinals_consecutive(const std::vector &values) +{ + for (size_t i = 1; i < values.size(); ++i) + if (values[i] != values[i - 1] + 1) + return false; + return true; +} + +SCENARIO("Toolchange emission and prefix per printer kind", "[GCodeWriter][H2C]") { + GIVEN("A dual-extruder writer with two filaments") { + GCodeWriter writer; + writer.config.filament_diameter.values = {1.75, 1.75}; + writer.set_extruders({0, 1}); + + WHEN("the printer is a BBL machine") { + writer.set_is_bbl_machine(true); + THEN("the toolchange prefix is the plain T command") { + REQUIRE_THAT(writer.toolchange_prefix(), Catch::Matchers::Equals("T")); + } + THEN("toolchange emits a single M1020 with the nozzle id") { + const std::string gcode = writer.toolchange(1, 0); + REQUIRE_THAT(gcode, Catch::Matchers::ContainsSubstring("M1020 S1 H0")); + REQUIRE_THAT(gcode, !Catch::Matchers::StartsWith("T1")); + } + THEN("the other filament and nozzle emit their own ids") { + REQUIRE_THAT(writer.toolchange(0, 1), Catch::Matchers::ContainsSubstring("M1020 S0 H1")); + } + THEN("an unresolved nozzle keeps the literal -1 convention") { + REQUIRE_THAT(writer.toolchange(1, -1), Catch::Matchers::ContainsSubstring("M1020 S1 H-1")); + } + } + WHEN("the printer is a BBL machine with manual filament change") { + writer.set_is_bbl_machine(true); + writer.config.manual_filament_change.value = true; + THEN("the manual tag wins over the M1020 form") { + REQUIRE_THAT(writer.toolchange_prefix(), Catch::Matchers::StartsWith(";")); + const std::string gcode = writer.toolchange(1, 0); + REQUIRE_THAT(gcode, Catch::Matchers::ContainsSubstring(writer.toolchange_prefix() + "1")); + REQUIRE_THAT(gcode, !Catch::Matchers::ContainsSubstring("M1020")); + } + } + WHEN("the printer is not a BBL machine") { + THEN("toolchange keeps the plain T command") { + REQUIRE_THAT(writer.toolchange_prefix(), Catch::Matchers::Equals("T")); + const std::string gcode = writer.toolchange(1, 0); + REQUIRE_THAT(gcode, Catch::Matchers::StartsWith("T1")); + REQUIRE_THAT(gcode, !Catch::Matchers::ContainsSubstring("M1020")); + } + } + } +} + +// Shared dual-extruder printer config for the toolchange-count scenarios below. +static DynamicPrintConfig dual_extruder_toolchange_config() +{ + DynamicPrintConfig config = DynamicPrintConfig::full_print_config(); + config.set_key_value("gcode_flavor", new ConfigOptionEnum(gcfMarlinFirmware)); + config.set_key_value("emit_machine_limits_to_gcode", new ConfigOptionBool(false)); + config.set_key_value("machine_start_gcode", new ConfigOptionString("")); + config.set_key_value("layer_height", new ConfigOptionFloat(0.2)); + config.set_key_value("initial_layer_print_height", new ConfigOptionFloat(0.2)); + config.set_key_value("initial_layer_line_width", new ConfigOptionFloatOrPercent(0, false)); + config.set_key_value("z_hop", new ConfigOptionFloats({0., 0.})); + // The change block carries both a real toolchange command and the ordinal + // placeholder the stock profiles feed to the firmware. + config.set_key_value("change_filament_gcode", + new ConfigOptionString("T[next_filament_id]\nM620 O{toolchange_count + 1}\n")); + + // 2 extruders, one filament each (manual map so nothing regroups them). + config.set_key_value("nozzle_diameter", new ConfigOptionFloats({0.4, 0.4})); + config.set_key_value("printer_extruder_id", new ConfigOptionInts({1, 2})); + config.set_key_value("printer_extruder_variant", new ConfigOptionStrings({"Direct Drive Standard", "Direct Drive Standard"})); + config.set_key_value("filament_diameter", new ConfigOptionFloats({1.75, 1.75})); + config.set_key_value("filament_colour", new ConfigOptionStrings({"#FF0000", "#00FF00"})); + config.set_key_value("default_filament_colour", new ConfigOptionStrings({"#FF0000", "#00FF00"})); + config.set_key_value("filament_type", new ConfigOptionStrings({"PLA", "PLA"})); + config.option>("filament_map_mode", true)->value = fmmManual; + config.set_key_value("filament_map", new ConfigOptionInts({1, 2})); + config.set_key_value("nozzle_temperature", new ConfigOptionInts({210, 210})); + config.set_key_value("nozzle_temperature_range_low", new ConfigOptionInts({190, 190})); + config.set_key_value("nozzle_temperature_range_high", new ConfigOptionInts({240, 240})); + config.set_key_value("flush_multiplier", new ConfigOptionFloats({1})); + config.set_key_value("flush_volumes_matrix", new ConfigOptionFloats({0, 140, 140, 0})); + return config; +} + +SCENARIO("Change blocks carry consecutive toolchange ordinals without a duplicate command", "[GCodeWriter][H2C]") { + GIVEN("Two sequentially printed objects on different extruders of a BBL machine") { + DynamicPrintConfig config = dual_extruder_toolchange_config(); + config.set_key_value("print_sequence", new ConfigOptionEnum(PrintSequence::ByObject)); + + Model model; + auto *obj1 = model.add_object(); + obj1->add_volume(cube(20)); + obj1->add_instance(); + auto *obj2 = model.add_object(); + obj2->add_volume(cube(20)); + obj2->add_instance(); + obj2->config.set_key_value("extruder", new ConfigOptionInt(2)); + + auto slice_to_gcode = [&]() { + Print print; + print.is_BBL_printer() = true; + arrange_objects(model, InfiniteBed{}, ArrangeParams{scaled(min_object_distance(config))}); + for (auto *mo : model.objects) { + mo->ensure_on_bed(); + print.auto_assign_extruders(mo); + } + print.apply(model, config); + print.validate(); + print.set_status_silent(); + print.process(); + return Slic3r::Test::gcode(print); + }; + + WHEN("the change block already changes the tool") { + const std::string gcode = slice_to_gcode(); + const std::vector ordinals = collect_line_args(gcode, "M620 O"); + THEN("each change block advances the ordinal by exactly one, without inflation") { + REQUIRE(!ordinals.empty()); + REQUIRE(ordinals_consecutive(ordinals)); + REQUIRE(ordinals.front() <= 3); + } + THEN("the writer's own command is suppressed as a duplicate") { + REQUIRE(count_lines_with_prefix(gcode, "M1020") == 0); + REQUIRE(count_lines_with_prefix(gcode, "T1") >= 1); + } + } + WHEN("the change block does not change the tool itself") { + config.set_key_value("change_filament_gcode", + new ConfigOptionString("M620 O{toolchange_count + 1}\n")); + const std::string gcode = slice_to_gcode(); + const std::vector ordinals = collect_line_args(gcode, "M620 O"); + THEN("the writer's toolchange survives and carries a nozzle id") { + REQUIRE(count_lines_with_prefix(gcode, "M1020 S1 H") >= 1); + } + THEN("the ordinal sequence stays consecutive") { + REQUIRE(!ordinals.empty()); + REQUIRE(ordinals_consecutive(ordinals)); + REQUIRE(ordinals.front() <= 3); + } + } + } +} + +SCENARIO("Prime-tower visits without a filament change do not advance the toolchange ordinal", "[GCodeWriter][H2C]") { + GIVEN("A print whose only filament change happens far above the bed") { + DynamicPrintConfig config = dual_extruder_toolchange_config(); + config.set_key_value("enable_prime_tower", new ConfigOptionBool(true)); + + // Filament 2 is used only above z=6, so every tower layer below it is a + // change-less visit — the exact geometry that used to inflate the ordinal. + Model model; + auto *obj = model.add_object(); + obj->add_volume(cube(10)); + obj->add_instance(); + DynamicPrintConfig range_config; + range_config.set_key_value("extruder", new ConfigOptionInt(2)); + // Every layer range must carry a layer_height (see layer_height_profile_from_ranges). + range_config.set_key_value("layer_height", new ConfigOptionFloat(0.2)); + obj->layer_config_ranges[{6.0, 10.0}].assign_config(std::move(range_config)); + + Print print; + print.is_BBL_printer() = true; + arrange_objects(model, InfiniteBed{}, ArrangeParams{scaled(min_object_distance(config))}); + for (auto *mo : model.objects) { + mo->ensure_on_bed(); + print.auto_assign_extruders(mo); + } + print.apply(model, config); + print.validate(); + print.set_status_silent(); + print.process(); + const std::string gcode = Slic3r::Test::gcode(print); + + WHEN("the print is exported") { + const std::vector ordinals = collect_line_args(gcode, "M620 O"); + THEN("the prime-tower toolchange path was exercised") { + REQUIRE_THAT(gcode, Catch::Matchers::ContainsSubstring("CP TOOLCHANGE START")); + } + THEN("dozens of change-less tower layers consume no ordinal") { + REQUIRE(!ordinals.empty()); + REQUIRE(ordinals_consecutive(ordinals)); + REQUIRE(ordinals.front() <= 3); + } + THEN("no duplicate toolchange command follows the change block") { + REQUIRE(count_lines_with_prefix(gcode, "M1020") == 0); + } + } + } +}