diff --git a/src/libslic3r/GCode.cpp b/src/libslic3r/GCode.cpp index c7c2721ed1..78b4300d87 100644 --- a/src/libslic3r/GCode.cpp +++ b/src/libslic3r/GCode.cpp @@ -4026,10 +4026,10 @@ void GCode::_do_export(Print& print, GCodeOutputStream &file, ThumbnailsGenerato (int) initial_extruder_id < (int) m_config.physical_extruder_map.values.size()) ? m_config.physical_extruder_map.values[(int) initial_extruder_id] : -1; - // enable_pressure_advance and pressure_advance are variant-expanded, so their - // length is columns, not filament slots. Bound in slot space -- filament_diameter - // is one entry per slot -- exactly as the second-layer temperature loop does, then - // translate the slot to its column with get_filament_config_index(). + // enable_pressure_advance and pressure_advance are indexed by COLUMN, not by + // filament slot. filament_diameter is one entry per slot and is never + // variant-expanded, so it is the slot-space bound; translate the slot to its column + // with get_filament_config_index(), as the second-layer temperature loop below does. const int num_pa_filament_slots = (int) print.config().filament_diameter.values.size(); for (int tool_idx : get_imex_active_tools(print)) { // Unlike the second-layer temperature loop, the primary is skipped here: @@ -6065,11 +6065,11 @@ LayerResult GCode::process_layer( // Mutually exclusive with the `else` below, so a head skipped here gets no // transition at all. `tool_idx` is physical; the printing head uses this layer's // own filament, the parallel carriages resolve through the head map. - // nozzle_temperature is variant-expanded, so its length is columns, not slots: - // bound in slot space, or an out-of-slot logical reaches get_filament_config_index - // and comes back as filament 0. - const int num_filament_columns = std::min((int) print.config().nozzle_temperature.values.size(), - (int) print.config().filament_diameter.values.size()); + // nozzle_temperature is indexed by column: bound in slot space, or an out-of-slot + // logical reaches get_filament_config_index and comes back as filament 0. + // filament_diameter is the slot-space yardstick -- one entry per filament slot, + // never variant-expanded -- and is the bound the pressure advance loop uses too. + const int num_filament_slots = (int) print.config().filament_diameter.values.size(); // Bounds-checked, not get_at() -- see IMEXHelpers.hpp. A clamp would hand the // "initial" branch below to whichever secondary sits on pem[0], giving it the wrong // filament's transition temperature and never its own. @@ -6083,7 +6083,7 @@ LayerResult GCode::process_layer( ? (int)first_extruder_id : resolve_filament_for_head( m_imex_head_filament_map, m_config.physical_extruder_map, tool_idx); - if (logical < 0 || logical >= num_filament_columns) continue; + if (logical < 0 || logical >= num_filament_slots) continue; // Variant-expanded printers column each filament; index as the `else` does. int temperature = print.config().nozzle_temperature.get_at(get_filament_config_index(logical)); if (temperature > 0) diff --git a/tests/fff_print/test_imex_mode_gcode.cpp b/tests/fff_print/test_imex_mode_gcode.cpp index d09feb93c4..04643864e7 100644 --- a/tests/fff_print/test_imex_mode_gcode.cpp +++ b/tests/fff_print/test_imex_mode_gcode.cpp @@ -50,6 +50,25 @@ static bool has_line(const std::string &gcode, const std::string &expected) return find_exact_line(gcode, expected) != std::string::npos; } +// Any command line -- not a comment -- containing `token`. Used where the assertion is that a +// carriage was addressed at all, rather than with a particular value: the config block the +// exporter appends restates every setting as "; key = value", so the comment lines have to be +// excluded or a search for a setting's own name matches itself. +static bool has_command_containing(const std::string &gcode, const std::string &token) +{ + std::size_t pos = 0; + while (pos < gcode.size()) { + const std::size_t eol = gcode.find('\n', pos); + const std::string line = gcode.substr(pos, (eol == std::string::npos ? gcode.size() : eol) - pos); + if (!line.empty() && line.front() != ';' && line.find(token) != std::string::npos) + return true; + if (eol == std::string::npos) + break; + pos = eol + 1; + } + return false; +} + // Mode scripts. Free of placeholders so that a test can tell "the script was emitted" apart // from "the script was expanded"; the expansion case below supplies its own template. static const char *kCopyScript = "SET_DUAL_CARRIAGE MODE=COPY"; @@ -79,6 +98,12 @@ static void imex_7x4_printer(DynamicPrintConfig &config) "Direct Drive Standard" }, { "extruder_printable_height", "0,0,0,0,0,0,0" }, { "physical_extruder_map", "0,0,0,0,1,2,3" }, + // Authored 1..n, as every production path does: PresetBundle writes it in + // full_fff_config(), and config load synthesizes it when absent. The all-1s default is + // not a neutral placeholder -- get_config_index_base() keys the slot-to-column lookup + // on it, and with every entry equal each per-filament vector collapses to filament 1's + // value, leaving a per-slot assertion comparing a value against itself. + { "filament_self_index", "1,2,3,4,5,6,7" }, { "is_imex", "1" }, { "imex_mode_names", "primary;copy;iq-copy" }, { "imex_mode_active_tools", "0:P;0:P,1:C;0:P,1:C,2:C,3:C" }, @@ -315,3 +340,47 @@ TEST_CASE("IMEX mode placeholders are defined on an ordinary single-extruder pri CHECK(has_line(gcode, ";IMEX_MODE_INDEX:0")); CHECK(has_line(gcode, ";IMEX_MODE_GCODE:")); } + +// In a parallel mode no tool changes occur, so every active carriage has to be addressed +// explicitly: once before the print for pressure advance, and again at the second layer for the +// drop from the initial-layer temperature. Nothing else in the suite covers either emission, and +// a carriage that falls out of one of those loops emits nothing at all rather than emitting +// something wrong -- it silently holds the initial-layer temperature, or whatever pressure +// advance the firmware was last told, for the whole job. So the property worth pinning is that +// the set of carriages addressed is exactly the set the mode declares active. +TEST_CASE("Every carriage a parallel mode declares active is addressed, and no other", + "[ImexModeGcode][IMEX]") +{ + DynamicPrintConfig config = multifilament_config(7); + imex_7x4_printer(config); + all_regions_on_filament(config, 1); + set_mode_gcodes(config, { "", kCopyScript, kQuadScript }); + config.set_deserialize_strict({ + { "imex_parallel_mode", "copy" }, + { "enable_pressure_advance", "1,1,1,1,1,1,1" }, + // Distinct per slot, so each assertion below names one filament and no other. copy + // mode's secondary rides physical 1, which physical_extruder_map routes to the fifth + // slot -- 0.05 and 244, not the primary's 0.01 and 240. + { "pressure_advance", "0.010,0.020,0.030,0.040,0.050,0.060,0.070" }, + { "nozzle_temperature_initial_layer", "235,235,235,235,235,235,235" }, + { "nozzle_temperature", "240,241,242,243,244,245,246" }, + }); + + const std::string gcode = slice({ cube(20) }, config); + + // imex_mode_active_tools declares copy as "0:P,1:C", so exactly these two carriages run, and + // each transitions to the temperature of the filament its own head is routed to. The other + // two heads this printer has must stay untouched. + CHECK(has_command_containing(gcode, "M104 S240 T0 ; set IMEX tool temperature")); + CHECK(has_command_containing(gcode, "M104 S244 T1 ; set IMEX tool temperature")); + CHECK_FALSE(has_command_containing(gcode, "T2 ; set IMEX tool temperature")); + CHECK_FALSE(has_command_containing(gcode, "T3 ; set IMEX tool temperature")); + + // The companion loop, which skips the primary because set_extruder() has already emitted it + // on the ordinary path. The qualified line is therefore the one that belongs to this loop: + // Klipper names the first carriage as the unnumbered "extruder" and the rest by index, so + // dropping the secondary removes `extruder1` and leaves `extruder` in place. + CHECK(has_command_containing(gcode, "SET_PRESSURE_ADVANCE ADVANCE=0.05 EXTRUDER=extruder1;")); + CHECK(has_command_containing(gcode, "SET_PRESSURE_ADVANCE ADVANCE=0.01 EXTRUDER=extruder;")); + CHECK_FALSE(has_command_containing(gcode, "EXTRUDER=extruder2;")); +}