From bd8dfd6250e03a0bc2321bd1e36a3cbceb1e2a26 Mon Sep 17 00:00:00 2001 From: Clifford Garwood Date: Thu, 3 Sep 2026 00:57:14 -0400 Subject: [PATCH] Cover the IMEX slice offset, the mode G-code placeholders, and the non-IMEX heater guard Closes review comments 13, 14 and 15, and adds the test for a shipped-profile regression that nothing guarded. - 14 and 15: compute_imex_slice_offset had eight tests on the calculation and none on the result, which is the whole firmware-managed path. test_imex_slice_offset now covers the derivation end (which config produces a non-zero offset, and that it is plate-local rather than moving with the plate origin -- the bug that shifted every plate after the first) and the consumption end (emitted coordinates and first_layer_print_min/max both move by the derived amount). The first_layer case also cross-checks the two consumers against each other: the declared bounds must keep the same relationship to the emitted toolpaths in both frames, which fails if exactly one of them is shifted. It deliberately does not pin the size of that gap -- it is 2.225 mm here, set by the wall generator, the same with no offset at all, and pinning it would fail on an unrelated change. - 13: nothing exercised the imex_mode / imex_mode_index / imex_mode_gcode placeholders or the {global} flow into machine_start_gcode that their ordering exists to guarantee. Seven cases now do, including the ordering itself -- the mode script declares a global and machine_start_gcode reads it back, so moving the mode processing later leaves the variable undefined and fails the export -- plus the inert cases (Primary mode, and a printer with the table filled in but is_imex off). All matching is whole-line, because the config block the exporter appends repeats machine_start_gcode verbatim and would make substring checks meaningless. - New: GCodeWriter passes this->config.is_imex.value into the heater remap, and nothing tested that it passes the flag rather than a constant. Hardcode true there and the whole suite stays green while fdm_bbl_3dp_002_common, which ships physical_extruder_map [1,0], starts sending filament 0's M104/M109 to heater 1. The new case runs a two-nozzle non-IMEX printer with that map and asserts each filament's temperature reaches only its own tool. It uses idle_temperature via ooze prevention rather than nozzle_temperature: keys in filament_options_with_variant are re-indexed per filament by variant slot at apply time, and this harness pins nozzle_diameter to one value, so every filament resolves to the same slot and the temperatures stop telling the heads apart. Also fixes two weaknesses in tests added earlier in this branch: an assertion that would have been prefix-satisfied by the very routing it was meant to exclude, and a whole-file command comparison between two slices, which this slicer's output is not stable enough to support. Co-Authored-By: Claude Opus 5 (1M context) --- tests/fff_print/CMakeLists.txt | 2 + tests/fff_print/test_imex_mode_gcode.cpp | 317 +++++++++++++ tests/fff_print/test_imex_slice_offset.cpp | 489 +++++++++++++++++++++ tests/fff_print/test_multifilament.cpp | 160 +++++++ 4 files changed, 968 insertions(+) create mode 100644 tests/fff_print/test_imex_mode_gcode.cpp create mode 100644 tests/fff_print/test_imex_slice_offset.cpp diff --git a/tests/fff_print/CMakeLists.txt b/tests/fff_print/CMakeLists.txt index fc46bb8fdc..f81fdb239a 100644 --- a/tests/fff_print/CMakeLists.txt +++ b/tests/fff_print/CMakeLists.txt @@ -10,6 +10,8 @@ add_executable(${_TEST_NAME}_tests test_flow.cpp test_gcode_timing.cpp test_gcodewriter.cpp + test_imex_mode_gcode.cpp + test_imex_slice_offset.cpp test_model.cpp test_multifilament.cpp test_perimeters.cpp diff --git a/tests/fff_print/test_imex_mode_gcode.cpp b/tests/fff_print/test_imex_mode_gcode.cpp new file mode 100644 index 0000000000..d09feb93c4 --- /dev/null +++ b/tests/fff_print/test_imex_mode_gcode.cpp @@ -0,0 +1,317 @@ +#include + +#include "libslic3r/Config.hpp" +#include "libslic3r/PrintConfig.hpp" + +#include "test_helpers.hpp" +#include "test_utils.hpp" + +#include +#include +#include + +using namespace Slic3r; +using namespace Slic3r::Test; + +// The IMEX mode placeholders ({imex_mode}, {imex_mode_index}, {imex_mode_gcode}) and the mode +// script that consumes them are set up in GCode.cpp's _do_export, immediately before +// machine_start_gcode is processed. Two contracts live in that ordering: +// * the three placeholders exist for every G-code script the export runs, IMEX or not; +// * the mode script is itself run through the placeholder parser, and it runs FIRST, so a +// {global ...} it declares is in scope by the time machine_start_gcode is processed. +// Nothing else in the suite exercises them. + +// Offset of the first line of `gcode` that equals `expected` once trailing whitespace is +// stripped, or npos. Whole-line matching is deliberate on both counts: prefix matching would +// let ";IMEX_MODE_INDEX:2" pass for an emitted index of 20, and the config block the exporter +// appends to every file repeats machine_start_gcode and imex_mode_gcodes verbatim (as +// "; key = value"), which would make any substring search for the templates below trivially +// true and every "not emitted" assertion trivially false. +static std::size_t find_exact_line(const std::string &gcode, const std::string &expected) +{ + std::size_t pos = 0; + while (pos <= gcode.size()) { + const std::size_t eol = gcode.find('\n', pos); + const std::size_t end = (eol == std::string::npos) ? gcode.size() : eol; + std::string line = gcode.substr(pos, end - pos); + while (!line.empty() && (line.back() == '\r' || line.back() == ' ' || line.back() == '\t')) + line.pop_back(); + if (line == expected) + return pos; + if (eol == std::string::npos) + break; + pos = eol + 1; + } + return std::string::npos; +} + +static bool has_line(const std::string &gcode, const std::string &expected) +{ + return find_exact_line(gcode, expected) != std::string::npos; +} + +// 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"; +static const char *kQuadScript = "SET_QUAD_CARRIAGE MODE=COPY"; + +// Reports all three placeholders on their own lines so find_exact_line can pin each value. +static const char *kReportStartGcode = ";IMEX_MODE:{imex_mode}\n" + ";IMEX_MODE_INDEX:{imex_mode_index}\n" + ";IMEX_MODE_GCODE:{imex_mode_gcode}\n"; + +// Shared IMEX printer geometry: 7 logical extruders across 4 physical heads, three modes. +// physical_extruder_map is only honoured when its length matches the nozzle count +// (PrintApply feeds effective_physical_extruder_map the nozzle_diameter size), so the nozzle +// keys must be sized to 7 or the map is silently replaced with the identity and IMEX quietly +// stops happening at all. Mirrors imex_7x4_printer() in test_multifilament.cpp; `iq-copy` is +// carried over from the IQEX case there, which is a mode combination known to slice. +// +// imex_mode_gcodes is left to each test: the position a mode's script occupies in this list is +// exactly what {imex_mode_index} is asserted against. +static void imex_7x4_printer(DynamicPrintConfig &config) +{ + config.set_deserialize_strict({ + { "nozzle_diameter", "0.4,0.4,0.4,0.4,0.4,0.4,0.4" }, + { "printer_extruder_id", "1,2,3,4,5,6,7" }, + { "printer_extruder_variant", "Direct Drive Standard,Direct Drive Standard,Direct Drive Standard," + "Direct Drive Standard,Direct Drive Standard,Direct Drive Standard," + "Direct Drive Standard" }, + { "extruder_printable_height", "0,0,0,0,0,0,0" }, + { "physical_extruder_map", "0,0,0,0,1,2,3" }, + { "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" }, + { "skirt_loops", "0" }, + { "brim_type", "no_brim" }, + // Klipper skips the bed/extruder temperature block that would otherwise be written + // between the mode script and machine_start_gcode, so the ordering assertion below + // compares the two scripts and nothing else. + { "gcode_flavor", "klipper" }, + }); +} + +// Set the mode scripts positionally, one per name in imex_mode_names. Assigning the vector +// beats set_deserialize_strict here: a coStrings round trip through a ';'-joined string has to +// quote and escape empty entries and embedded braces, and the empty first entry (primary, which +// deliberately has no script) is exactly the entry that would be lost. +static void set_mode_gcodes(DynamicPrintConfig &config, const std::vector &gcodes) +{ + config.option("imex_mode_gcodes", true)->values = gcodes; +} + +// Route every region to one filament. An unset *_filament_id is not "inherit": +// clamp_feature_filament_to_valid rewrites <=0 to 1, which would drag tool 0 into +// tool_ordering. Filament 1 is logical slot 0, which pem routes to head 0 -- the head every +// mode here declares Primary -- so Print::validate() accepts the plate. +static void all_regions_on_filament(DynamicPrintConfig &config, int filament_1based) +{ + for (const char *key : { "outer_wall_filament_id", "inner_wall_filament_id", + "sparse_infill_filament_id", "internal_solid_filament_id", + "top_surface_filament_id", "bottom_surface_filament_id" }) + config.set_deserialize_strict({ { key, std::to_string(filament_1based) } }); +} + + +// {imex_mode} is the plate's active mode, {imex_mode_index} its position in imex_mode_names and +// {imex_mode_gcode} the raw script at that same position. Two modes are exercised because a +// single one cannot tell a resolved index apart from a constant: `copy` sits at 1 and `iq-copy` +// at 2, so an index that stopped tracking imex_mode_names fails at least one of them. +TEST_CASE("IMEX mode placeholders resolve to the active mode's name, index and script", + "[ImexModeGcode][IMEX]") +{ + auto [mode, mode_index, mode_script] = GENERATE(table({ + { "copy", 1, kCopyScript }, + { "iq-copy", 2, kQuadScript }, + })); + INFO("active mode: " << mode); + + 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", mode }, + { "machine_start_gcode", kReportStartGcode }, + }); + + const std::string gcode = slice({ cube(20) }, config); + + CHECK(has_line(gcode, ";IMEX_MODE:" + mode)); + CHECK(has_line(gcode, ";IMEX_MODE_INDEX:" + std::to_string(mode_index))); + CHECK(has_line(gcode, ";IMEX_MODE_GCODE:" + mode_script)); + // The script for the active mode -- and only that one -- reaches the file. + CHECK(has_line(gcode, mode_script)); + CHECK_FALSE(has_line(gcode, mode == "copy" ? kQuadScript : kCopyScript)); +} + +// The mode script is a G-code template, not a literal: it goes through +// placeholder_parser_process like machine_start_gcode does. Because the three IMEX placeholders +// are set before that call, the script can also read its own mode -- which is what +// distinguishes "the script is processed" from "the script is processed too early to see them". +TEST_CASE("A placeholder inside an IMEX mode's G-code script is expanded", + "[ImexModeGcode][IMEX]") +{ + const std::string templ = "SET_DUAL_CARRIAGE MODE={imex_mode} INDEX={imex_mode_index}" + " TEMP={nozzle_temperature_initial_layer[0]}"; + + DynamicPrintConfig config = multifilament_config(7); + imex_7x4_printer(config); + all_regions_on_filament(config, 1); + set_mode_gcodes(config, { "", templ, kQuadScript }); + config.set_deserialize_strict({ + { "imex_parallel_mode", "copy" }, + // The multi-extruder normalization collapses the per-filament temperature vector to a + // single value, so [0] is this literal 200 rather than a per-slot default. + { "nozzle_temperature_initial_layer", "200" }, + { "machine_start_gcode", ";START\n" }, + }); + + const std::string gcode = slice({ cube(20) }, config); + + CHECK(has_line(gcode, "SET_DUAL_CARRIAGE MODE=copy INDEX=1 TEMP=200")); + // If the script were written straight to the file the braces would survive verbatim. + CHECK_FALSE(has_line(gcode, templ)); +} + +// The ordering guarantee, and the reason the placeholder block sits where it does: the mode +// script is processed BEFORE machine_start_gcode, so a {global} the mode declares is in scope +// for machine_start_gcode. Reorder the two and machine_start_gcode references an undefined +// variable, which the placeholder parser raises as an error out of the export -- so a +// regression here fails this test whether the export throws or merely writes nothing useful. +TEST_CASE("A global declared in the IMEX mode G-code is visible to machine_start_gcode", + "[ImexModeGcode][IMEX]") +{ + DynamicPrintConfig config = multifilament_config(7); + imex_7x4_printer(config); + all_regions_on_filament(config, 1); + set_mode_gcodes(config, { "", std::string("{global imex_carriage_count = 2}") + kCopyScript, kQuadScript }); + config.set_deserialize_strict({ + { "imex_parallel_mode", "copy" }, + { "machine_start_gcode", ";IMEX_CARRIAGES:{imex_carriage_count}\n" }, + }); + + const std::string gcode = slice({ cube(20) }, config); + + // The value is the mode script's, not a default: nothing else in this config defines it. + const std::size_t start_gcode_at = find_exact_line(gcode, ";IMEX_CARRIAGES:2"); + REQUIRE(start_gcode_at != std::string::npos); + + // ...and the two land in the file in that same order. Ordering is the contract here, so it + // is asserted directly rather than left implicit in the global resolving. + const std::size_t mode_script_at = find_exact_line(gcode, kCopyScript); + REQUIRE(mode_script_at != std::string::npos); + CHECK(mode_script_at < start_gcode_at); +} + +// Primary is single-carriage printing: it names a real mode (position 0 of imex_mode_names, not +// the not-found fallback) but carries no script, so the export must add nothing. This is what +// keeps the mode machinery out of the way of an IMEX printer that is not printing in parallel. +TEST_CASE("Primary-mode IMEX prints emit no mode G-code", "[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", "primary" }, + { "machine_start_gcode", kReportStartGcode }, + }); + + const std::string gcode = slice({ cube(20) }, config); + + CHECK(has_line(gcode, ";IMEX_MODE:primary")); + CHECK(has_line(gcode, ";IMEX_MODE_INDEX:0")); + CHECK(has_line(gcode, ";IMEX_MODE_GCODE:")); + CHECK_FALSE(has_line(gcode, kCopyScript)); + CHECK_FALSE(has_line(gcode, kQuadScript)); +} + +// A plate stores its IMEX mode as the mode's *name* and the exporter resolves that name against +// the printer's imex_mode_names at slice time, so the two drift apart whenever the mode is +// renamed or deleted after a plate was set to it, or the project is opened against a printer +// preset that names its modes differently. A name matching no row must be treated as Primary -- +// not as a parallel mode whose every name-keyed lookup happens to come back empty, which is how +// it used to behave: no mode script (there is none to find), no active-tool roster, and yet the +// parallel branches taken all the way through the export. +// +// Primary is a row like any other, so falling back to it means running its script too; a +// non-empty script is used here so "fell back to Primary" is distinguishable from "resolved +// nothing and emitted nothing". +TEST_CASE("An IMEX mode the printer no longer defines falls back to Primary", + "[ImexModeGcode][IMEX]") +{ + static const char *kPrimaryScript = "SET_DUAL_CARRIAGE MODE=PRIMARY"; + + DynamicPrintConfig config = multifilament_config(7); + imex_7x4_printer(config); + all_regions_on_filament(config, 1); + set_mode_gcodes(config, { kPrimaryScript, kCopyScript, kQuadScript }); + config.set_deserialize_strict({ + // A name no row in imex_mode_names carries -- what `copy` becomes once it is renamed. + { "imex_parallel_mode", "copy-renamed" }, + { "machine_start_gcode", kReportStartGcode }, + }); + + const std::string gcode = slice({ cube(20) }, config); + + // The placeholders report the mode the export actually ran, so the stale name must not + // survive into them -- a mode script keyed on {imex_mode} would otherwise set the printer + // up for a mode the slicer did not slice for. + CHECK(has_line(gcode, ";IMEX_MODE:primary")); + CHECK(has_line(gcode, ";IMEX_MODE_INDEX:0")); + CHECK(has_line(gcode, ";IMEX_MODE_GCODE:" + std::string(kPrimaryScript))); + + // Primary's own script runs; neither parallel mode's does. + CHECK(has_line(gcode, kPrimaryScript)); + CHECK_FALSE(has_line(gcode, kCopyScript)); + CHECK_FALSE(has_line(gcode, kQuadScript)); +} + +// The whole block is gated on is_imex. A printer preset can carry a filled-in mode table and +// still not be an IMEX machine -- and a plate config can still carry a stale imex_parallel_mode +// -- so with the flag off the placeholders stay empty and no mode script is injected. Run on an +// ordinary single-extruder printer on purpose: the guard is one boolean, and the multi-head +// geometry the other cases need would only add ways for this one to fail for another reason. +TEST_CASE("A printer with IMEX disabled resolves the mode placeholders to nothing", + "[ImexModeGcode][IMEX]") +{ + DynamicPrintConfig config = DynamicPrintConfig::full_print_config(); + set_mode_gcodes(config, { "", kCopyScript, kQuadScript }); + config.set_deserialize_strict({ + { "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" }, + { "is_imex", "0" }, + { "imex_parallel_mode", "copy" }, + { "skirt_loops", "0" }, + { "brim_type", "no_brim" }, + { "gcode_flavor", "klipper" }, + { "machine_start_gcode", kReportStartGcode }, + }); + + const std::string gcode = slice({ cube(20) }, config); + + CHECK(has_line(gcode, ";IMEX_MODE:")); + CHECK(has_line(gcode, ";IMEX_MODE_INDEX:0")); + CHECK(has_line(gcode, ";IMEX_MODE_GCODE:")); + CHECK_FALSE(has_line(gcode, kCopyScript)); + CHECK_FALSE(has_line(gcode, kQuadScript)); +} + +// The placeholders are set unconditionally, ahead of the is_imex check, so they are defined for +// every printer. A single-extruder preset that mentions {imex_mode} must expand it to an empty +// string rather than fail the export on an unknown variable. +TEST_CASE("IMEX mode placeholders are defined on an ordinary single-extruder printer", + "[ImexModeGcode][IMEX]") +{ + const std::string gcode = slice({ cube(20) }, { + { "skirt_loops", "0" }, + { "brim_type", "no_brim" }, + { "gcode_flavor", "klipper" }, + { "machine_start_gcode", kReportStartGcode }, + }); + + CHECK(has_line(gcode, ";IMEX_MODE:")); + CHECK(has_line(gcode, ";IMEX_MODE_INDEX:0")); + CHECK(has_line(gcode, ";IMEX_MODE_GCODE:")); +} diff --git a/tests/fff_print/test_imex_slice_offset.cpp b/tests/fff_print/test_imex_slice_offset.cpp new file mode 100644 index 0000000000..c12c0f20cb --- /dev/null +++ b/tests/fff_print/test_imex_slice_offset.cpp @@ -0,0 +1,489 @@ +#include + +#include "libslic3r/GCodeReader.hpp" +#include "libslic3r/Model.hpp" +#include "libslic3r/Point.hpp" +#include "libslic3r/Print.hpp" + +#include "test_helpers.hpp" +#include "test_utils.hpp" + +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace Slic3r; +using namespace Slic3r::Test; +using Catch::Matchers::WithinAbs; + +// The IMEX slice offset is the firmware-managed handoff: an extra XY shift applied at emission +// so the primary tool's zone lands centred on the bed origin and the firmware can fan copies / +// mirrors out from there. `Print::update_imex_slice_offset()` DERIVES it from the applied +// config -- nobody hands it in. That matters because the only thing that used to hand it in was +// the plater, so a headless `orca-slicer --slice` produced slicer-frame coordinates while the +// mode G-code told the firmware to apply its own offsets on top. +// +// This file covers both halves: +// +// * Derivation -- which config makes the offset non-zero, what value it takes, and (the part +// the GUI push got wrong) that it is in PLATE-LOCAL mm and therefore does not move when the +// plate origin does. `compute_imex_slice_offset()` and `compute_imex_zone_layout()` are +// covered as pure functions in tests/libslic3r/; what is covered here is the Print reaching +// them with the right inputs. +// * Consumption -- that a slice actually comes out shifted. Two sites consume the offset: +// GCode::set_gcode_offset_with_imex_shift() (GCode.hpp), which augments the writer offset +// so every emitted coordinate moves, and Print::translate_to_print_space() (Print.cpp), +// which feeds the first_layer_print_min/max placeholders a start-G-code template uses to +// declare the print area to firmware. The two must stay in one frame, so the last test +// cross-checks them against each other. +// +// The offset is applied at emission, after slicing, so it is a pure translation: assertions are +// on the SHIFT between two slices of the same plate, never on absolute coordinates or on a +// golden file. That keeps them independent of where the arranger puts the cube, and immune to +// the run-to-run variation in this slicer's parallel infill generation. + +// --------------------------------------------------------------------------------------------- +// Helpers (unnamed namespace: every suite links into one binary, so nothing here may collide +// with a same-named helper in a sibling test file) +// --------------------------------------------------------------------------------------------- +namespace { + +// The bed the derivation divides up, and the answer it must reach. +// +// With `imex_gantry_count` 1, `imex_tools_per_gantry` 2 and `imex_tool_layout` front-left, T0 +// owns the left column and T1 the right one, so a mode declaring `0:P,1:C` gives the primary +// the left half of the bed: x [0, 150], y [0, 200]. Its centre is the offset. Both numbers are +// set by the config below, not inherited from a default. +constexpr double kBedWidth = 300.0; +constexpr double kBedDepth = 200.0; +const Vec2d kPrimaryZoneCentre(kBedWidth / 4.0, kBedDepth / 2.0); + +// XY extent of a set of emitted moves. `empty()` means nothing matched. +struct XYBounds +{ + double min_x = std::numeric_limits::max(); + double min_y = std::numeric_limits::max(); + double max_x = std::numeric_limits::lowest(); + double max_y = std::numeric_limits::lowest(); + + bool empty() const { return min_x > max_x; } + double size_x() const { return max_x - min_x; } + double size_y() const { return max_y - min_y; } +}; + +// Extent of the END points of every extruding move commented "perimeter", up to `z_max`. +// +// Perimeters only: they are the outermost extrusions, so they carry the plate's extent, and +// unlike infill their geometry and ordering are stable run to run. End points only: the parse +// callback runs BEFORE the reader advances, so the start point of the first extrusion of a +// polyline is whatever the preceding travel left behind -- and a cube's perimeters are closed +// loops, whose last point is their first, so the end points alone already give the true extent. +static XYBounds perimeter_bounds(const std::string &gcode, double z_max = std::numeric_limits::max()) +{ + XYBounds bounds; + GCodeReader reader; + reader.parse_buffer(gcode, [&bounds, z_max](GCodeReader &self, const GCodeReader::GCodeLine &line) { + if (!line.extruding(self)) + return; + if (line.comment().find("perimeter") == std::string_view::npos) + return; + if (double(line.new_Z(self)) > z_max) + return; + const double x = line.new_X(self); + const double y = line.new_Y(self); + bounds.min_x = std::min(bounds.min_x, x); + bounds.max_x = std::max(bounds.max_x, x); + bounds.min_y = std::min(bounds.min_y, y); + bounds.max_y = std::max(bounds.max_y, y); + }); + return bounds; +} + +// Read "x,y" out of `gcode`. The config block the exporter appends restates +// machine_start_gcode verbatim, so the tag also occurs there with the placeholder still +// unexpanded; occurrences are tried in turn and the first one that parses as two numbers wins. +static bool read_marker(const std::string &gcode, const std::string &tag, Vec2d &out) +{ + for (size_t at = gcode.find(tag); at != std::string::npos; at = gcode.find(tag, at + 1)) { + const size_t from = at + tag.size(); + const size_t to = gcode.find('\n', from); + const std::string payload = gcode.substr(from, to == std::string::npos ? std::string::npos : to - from); + const size_t comma = payload.find(','); + if (comma == std::string::npos) + continue; + try { + out = Vec2d(std::stod(payload.substr(0, comma)), std::stod(payload.substr(comma + 1))); + } catch (const std::exception &) { + continue; + } + return true; + } + return false; +} + +// An IMEX machine: 7 logical extruders across 4 physical heads, mirroring the geometry the IMEX +// cases in test_multifilament.cpp use, on a 2-tool single-gantry grid. +// +// physical_extruder_map is only honoured when its length matches the nozzle count (PrintApply +// hands effective_physical_extruder_map the nozzle_diameter size), so the nozzle keys have to be +// sized to 7 as well -- otherwise the map is silently replaced with the identity and the plate +// stops being an IMEX plate at all. +// +// The grid, the tool layout and the bed are all set explicitly: they are exactly the inputs the +// zone layout divides to reach kPrimaryZoneCentre, so none of them may come from a default. +// `imex_firmware_managed_zones` is deliberately NOT set here -- it is the switch under test, and +// every case states it for itself. +// +// Everything that would put extrusions outside the object footprint is off (skirt, brim, prime +// tower, infill, top/bottom shells): that leaves perimeters as the only extrusions, so the +// emitted extent and the first-layer convex hull both reduce to the cube's own outline. +static void imex_printer(DynamicPrintConfig &config) +{ + config.set_deserialize_strict({ + { "nozzle_diameter", "0.4,0.4,0.4,0.4,0.4,0.4,0.4" }, + { "printer_extruder_id", "1,2,3,4,5,6,7" }, + { "printer_extruder_variant", "Direct Drive Standard,Direct Drive Standard,Direct Drive Standard," + "Direct Drive Standard,Direct Drive Standard,Direct Drive Standard," + "Direct Drive Standard" }, + { "extruder_printable_height", "0,0,0,0,0,0,0" }, + { "physical_extruder_map", "0,0,0,0,1,2,3" }, + { "printable_area", "0x0,300x0,300x200,0x200" }, + { "is_imex", "1" }, + { "imex_gantry_count", "1" }, + { "imex_tools_per_gantry", "2" }, + { "imex_tool_layout", "front-left" }, + { "imex_mode_names", "primary;copy" }, + { "imex_mode_active_tools", "0:P;0:P,1:C" }, + // `copy` declares head 0 Primary, and filament 1 (logical slot 0) routes there, so the + // plate is well-formed and Print::validate() lets it through. + { "imex_parallel_mode", "copy" }, + { "skirt_loops", "0" }, + { "brim_type", "no_brim" }, + { "enable_prime_tower", "0" }, + { "sparse_infill_density", "0%" }, + { "top_shell_layers", "0" }, + { "bottom_shell_layers", "0" }, + { "wall_loops", "2" }, + { "layer_height", "0.2" }, + { "initial_layer_print_height","0.2" }, + { "gcode_flavor", "klipper" }, + }); +} + +// Route every region to one filament. An unset *_filament_id is not "inherit": +// clamp_feature_filament_to_valid rewrites <=0 to 1, so leaving them unset would drag extra +// tools into tool_ordering. PrintObject.cpp's call to that function is the source of truth for +// this key list. +static void all_regions_on_filament(DynamicPrintConfig &config, int filament_1based) +{ + for (const char *key : { "outer_wall_filament_id", "inner_wall_filament_id", + "sparse_infill_filament_id", "internal_solid_filament_id", + "top_surface_filament_id", "bottom_surface_filament_id" }) + config.set_deserialize_strict({ { key, std::to_string(filament_1based) } }); +} + +// A ready-to-slice firmware-managed IMEX plate. `firmware_managed` is the one thing that +// varies between a baseline slice and a shifted one. +static DynamicPrintConfig imex_config(bool firmware_managed) +{ + DynamicPrintConfig config = multifilament_config(7); + imex_printer(config); + all_regions_on_filament(config, 1); // filament 1 => logical slot 0 => physical head 0 + config.set_deserialize_strict({ { "imex_firmware_managed_zones", firmware_managed ? "1" : "0" } }); + return config; +} + +// The offset the Print works out for itself, with nothing pushed in. Applying the config is +// enough -- the derivation reads only the config and the objects, so this needs no slice. +static Vec2d derived_offset(const DynamicPrintConfig &config, const Vec3d &plate_origin = Vec3d::Zero()) +{ + Print print; + Model model; + init_print({ cube(20) }, print, model, config); + print.set_plate_origin(plate_origin); + print.update_imex_slice_offset(); + return print.get_imex_slice_offset(); +} + +// Slice one 20mm cube. Note what is NOT here: no offset is handed to the Print. Whatever shift +// the G-code comes out with, the Print derived on its own from `config`. +static std::string slice_cube(const DynamicPrintConfig &config, const Vec3d &plate_origin = Vec3d::Zero()) +{ + Print print; + Model model; + init_print({ cube(20) }, print, model, config); + print.set_plate_origin(plate_origin); + return gcode(print); +} + +} // namespace + +// --------------------------------------------------------------------------------------------- +// Derivation +// --------------------------------------------------------------------------------------------- + +// The bug this file exists for: the offset used to arrive only from PartPlate, which a headless +// slice never runs, so `--slice` on a firmware-managed plate emitted slicer-managed coordinates +// while the mode G-code told the firmware to fan copies out from them. Nothing pushes anything +// here; the Print is expected to reach the primary zone's centre from the config alone. +TEST_CASE("A firmware-managed IMEX plate derives its slice offset with nothing pushed in", + "[IMEXSliceOffset][IMEX]") +{ + const Vec2d offset = derived_offset(imex_config(true)); + + CHECK_THAT(offset.x(), WithinAbs(kPrimaryZoneCentre.x(), 1e-9)); + CHECK_THAT(offset.y(), WithinAbs(kPrimaryZoneCentre.y(), 1e-9)); +} + +// The offset is a PLATE-LOCAL quantity. Both consumers already subtract the plate origin +// separately, so an offset that moved with the plate would subtract it twice and put every +// plate after the first a full plate stride out. This is precisely what the old plater-side +// computation got wrong: it divided the plate's world-frame outline, not the bed. +TEST_CASE("The derived IMEX slice offset is plate-local and does not move with the plate origin", + "[IMEXSliceOffset][IMEX]") +{ + const DynamicPrintConfig config = imex_config(true); + + const Vec2d at_origin = derived_offset(config); + const Vec2d on_plate3 = derived_offset(config, Vec3d(kBedWidth * 2.0, -kBedDepth * 2.0, 0.0)); + + CHECK_THAT(on_plate3.x(), WithinAbs(at_origin.x(), 1e-9)); + CHECK_THAT(on_plate3.y(), WithinAbs(at_origin.y(), 1e-9)); +} + +// The offset tracks the bed it divides, so it is not a constant that happens to match one +// printer. Halving the bed halves the primary zone and its centre with it. +TEST_CASE("The derived IMEX slice offset follows the printable area", "[IMEXSliceOffset][IMEX]") +{ + DynamicPrintConfig config = imex_config(true); + config.set_deserialize_strict({ { "printable_area", "0x0,150x0,150x100,0x100" } }); + + const Vec2d offset = derived_offset(config); + + CHECK_THAT(offset.x(), WithinAbs(kPrimaryZoneCentre.x() / 2.0, 1e-9)); + CHECK_THAT(offset.y(), WithinAbs(kPrimaryZoneCentre.y() / 2.0, 1e-9)); +} + +// Every case that must leave the offset at exactly Vec2d::Zero(). This is what protects +// existing users: an exactly-zero offset is what makes the firmware-managed path reduce to the +// old set_gcode_offset() behaviour, to the last digit. +TEST_CASE("The derived IMEX slice offset is zero unless firmware-managed zones are in play", + "[IMEXSliceOffset][IMEX]") +{ + // A Print nobody has applied anything to starts at zero. This is the value every + // non-IMEX printer keeps, and the reason nothing else in the exporter had to change. + Print fresh; + REQUIRE_THAT(fresh.get_imex_slice_offset().x(), WithinAbs(0.0, 1e-12)); + REQUIRE_THAT(fresh.get_imex_slice_offset().y(), WithinAbs(0.0, 1e-12)); + + Vec2d offset = Vec2d::Zero(); + + SECTION("an ordinary printer with no IMEX configuration at all") { + DynamicPrintConfig config = DynamicPrintConfig::full_print_config(); + config.set_deserialize_strict({ + { "sparse_infill_density", "0%" }, + { "layer_height", "0.2" }, + { "initial_layer_print_height", "0.2" }, + }); + offset = derived_offset(config); + } + SECTION("an IMEX printer in a parallel mode, but firmware-managed zones off") { + offset = derived_offset(imex_config(false)); + } + SECTION("firmware-managed zones on, but the plate is in Primary mode") { + DynamicPrintConfig config = imex_config(true); + config.set_deserialize_strict({ { "imex_parallel_mode", "primary" } }); + offset = derived_offset(config); + } + SECTION("firmware-managed zones on, but the plate has no mode at all") { + DynamicPrintConfig config = imex_config(true); + config.set_deserialize_strict({ { "imex_parallel_mode", "" } }); + offset = derived_offset(config); + } + // A plate can name a mode this printer does not define -- a mode renamed or deleted after + // the plate was set to it, or a project opened against a different printer preset. The + // exporter falls back to Primary and warns; the offset has to make the same choice, or the + // file would be shifted for a mode nothing ever activates. + SECTION("firmware-managed zones on, but the plate's mode is not one this printer defines") { + DynamicPrintConfig config = imex_config(true); + config.set_deserialize_strict({ { "imex_parallel_mode", "renamed-since" } }); + offset = derived_offset(config); + } + // The layout hangs off the primary tool's cell. `imex_tools_per_gantry` 1 on a single + // gantry is a one-cell grid: there is nothing to divide, so there is nothing to shift by. + SECTION("firmware-managed zones on, but the tool grid holds a single tool") { + DynamicPrintConfig config = imex_config(true); + config.set_deserialize_strict({ { "imex_tools_per_gantry", "1" }, + { "imex_mode_active_tools", "0:P;0:P" } }); + offset = derived_offset(config); + } + + CHECK_THAT(offset.x(), WithinAbs(0.0, 1e-12)); + CHECK_THAT(offset.y(), WithinAbs(0.0, 1e-12)); +} + +// --------------------------------------------------------------------------------------------- +// Consumption +// --------------------------------------------------------------------------------------------- + +// The whole point of the firmware-managed path: the emitted toolpaths move, and they move by the +// derived offset. The writer subtracts plate origin + IMEX shift from every point it formats, so +// the firmware-managed slice comes out at (baseline - offset). Asserting the shift rather than +// absolute coordinates keeps this independent of wherever the arranger drops the cube. +TEST_CASE("A firmware-managed IMEX plate emits coordinates shifted by the offset it derived", + "[IMEXSliceOffset][IMEX]") +{ + const XYBounds baseline = perimeter_bounds(slice_cube(imex_config(false))); + const XYBounds shifted = perimeter_bounds(slice_cube(imex_config(true))); + REQUIRE_FALSE(baseline.empty()); + REQUIRE_FALSE(shifted.empty()); + + CHECK_THAT(shifted.min_x, WithinAbs(baseline.min_x - kPrimaryZoneCentre.x(), 1e-3)); + CHECK_THAT(shifted.max_x, WithinAbs(baseline.max_x - kPrimaryZoneCentre.x(), 1e-3)); + CHECK_THAT(shifted.min_y, WithinAbs(baseline.min_y - kPrimaryZoneCentre.y(), 1e-3)); + CHECK_THAT(shifted.max_y, WithinAbs(baseline.max_y - kPrimaryZoneCentre.y(), 1e-3)); + + // A translation, not a re-slice: the plate keeps its size. + CHECK_THAT(shifted.size_x(), WithinAbs(baseline.size_x(), 1e-3)); + CHECK_THAT(shifted.size_y(), WithinAbs(baseline.size_y(), 1e-3)); +} + +// The IMEX shift is added to the plate origin, not substituted for it. A multi-plate project +// already carries a non-zero plate origin, so dropping either term from +// set_gcode_offset_with_imex_shift() would put every emitted coordinate on the wrong plate -- +// invisibly to a test that only ever slices plate 1 at the origin. The expected total is +// origin + offset precisely because the derived offset does NOT itself contain the origin. +TEST_CASE("An IMEX slice offset composes with the plate origin rather than replacing it", + "[IMEXSliceOffset][IMEX]") +{ + const Vec3d plate_origin(kBedWidth * 1.1, -kBedDepth * 1.1, 0.0); + + const XYBounds baseline = perimeter_bounds(slice_cube(imex_config(false))); + const XYBounds shifted = perimeter_bounds(slice_cube(imex_config(true), plate_origin)); + REQUIRE_FALSE(baseline.empty()); + REQUIRE_FALSE(shifted.empty()); + + const double expected_x = plate_origin.x() + kPrimaryZoneCentre.x(); + const double expected_y = plate_origin.y() + kPrimaryZoneCentre.y(); + CHECK_THAT(shifted.min_x, WithinAbs(baseline.min_x - expected_x, 1e-3)); + CHECK_THAT(shifted.max_x, WithinAbs(baseline.max_x - expected_x, 1e-3)); + CHECK_THAT(shifted.min_y, WithinAbs(baseline.min_y - expected_y, 1e-3)); + CHECK_THAT(shifted.max_y, WithinAbs(baseline.max_y - expected_y, 1e-3)); +} + +// The guard for everyone who is not using this feature. `imex_firmware_managed_zones` is read +// by nothing else in the engine, so setting it on a printer the derivation refuses to shift must +// leave the slice where it was -- it must short-circuit on `is_imex` before it ever looks at a +// bed or a mode. The zero-offset sections above cover an ordinary printer that never mentions +// the option; this covers the option turned ON where nothing may act on it. +// +// Asserted as the derived offset plus the extent of the emitted perimeters, NOT as a line-by-line +// comparison of the two exports. This slicer's output is not stable run to run -- parallel infill +// generation is the documented source -- and the M73 time estimates, the seam placement and the +// travel ordering all ride on that variation, so comparing every emitted command would flake in +// CI rather than catch a shift. The offset is applied at emission as a pure translation, so a +// non-zero one moves the extent and this catches it; that it is exactly zero is what the +// derivation check states. +TEST_CASE("Turning on firmware-managed zones changes nothing on a non-IMEX printer", + "[IMEXSliceOffset][IMEX]") +{ + auto ordinary_printer = [](bool firmware_managed) { + DynamicPrintConfig config = DynamicPrintConfig::full_print_config(); + config.set_deserialize_strict({ + { "sparse_infill_density", "0%" }, + { "top_shell_layers", "0" }, + { "bottom_shell_layers", "0" }, + { "wall_loops", "2" }, + { "layer_height", "0.2" }, + { "initial_layer_print_height", "0.2" }, + { "skirt_loops", "0" }, + { "brim_type", "no_brim" }, + { "enable_prime_tower", "0" }, + { "imex_firmware_managed_zones", firmware_managed ? "1" : "0" }, + }); + return config; + }; + + // Nothing to apply in the first place. + const Vec2d offset = derived_offset(ordinary_printer(true)); + CHECK_THAT(offset.x(), WithinAbs(0.0, 1e-12)); + CHECK_THAT(offset.y(), WithinAbs(0.0, 1e-12)); + + // ...and the toolpaths bear that out: same plate, same place. Infill, skirt, brim and the + // prime tower are all off in this config, so the perimeters carry the whole extent. + const XYBounds baseline = perimeter_bounds(slice_cube(ordinary_printer(false))); + const XYBounds with_flag = perimeter_bounds(slice_cube(ordinary_printer(true))); + REQUIRE_FALSE(baseline.empty()); + REQUIRE_FALSE(with_flag.empty()); + + CHECK_THAT(with_flag.min_x, WithinAbs(baseline.min_x, 1e-3)); + CHECK_THAT(with_flag.max_x, WithinAbs(baseline.max_x, 1e-3)); + CHECK_THAT(with_flag.min_y, WithinAbs(baseline.min_y, 1e-3)); + CHECK_THAT(with_flag.max_y, WithinAbs(baseline.max_y, 1e-3)); +} + +// first_layer_print_min/max are what a start-G-code template hands the firmware to declare the +// print area (bed mesh bounds, PRINT_MIN/PRINT_MAX). They come from the first-layer convex hull +// pushed through Print::translate_to_print_space(), which subtracts the same IMEX shift the +// writer does, so they have to travel with the toolpaths. If they did not, a firmware-managed +// plate would probe one area and print in another. +TEST_CASE("first_layer_print_min/max track the derived IMEX slice offset", + "[IMEXSliceOffset][IMEX]") +{ + auto with_markers = [](bool firmware_managed) { + DynamicPrintConfig config = imex_config(firmware_managed); + config.set_deserialize_strict({ + { "machine_start_gcode", + ";FLMIN:{first_layer_print_min[0]},{first_layer_print_min[1]}\n" + ";FLMAX:{first_layer_print_max[0]},{first_layer_print_max[1]}\n" }, + }); + return config; + }; + + const std::string baseline_gcode = slice_cube(with_markers(false)); + const std::string shifted_gcode = slice_cube(with_markers(true)); + + Vec2d baseline_min, baseline_max, shifted_min, shifted_max; + REQUIRE(read_marker(baseline_gcode, ";FLMIN:", baseline_min)); + REQUIRE(read_marker(baseline_gcode, ";FLMAX:", baseline_max)); + REQUIRE(read_marker(shifted_gcode, ";FLMIN:", shifted_min)); + REQUIRE(read_marker(shifted_gcode, ";FLMAX:", shifted_max)); + + CHECK_THAT(shifted_min.x(), WithinAbs(baseline_min.x() - kPrimaryZoneCentre.x(), 1e-3)); + CHECK_THAT(shifted_min.y(), WithinAbs(baseline_min.y() - kPrimaryZoneCentre.y(), 1e-3)); + CHECK_THAT(shifted_max.x(), WithinAbs(baseline_max.x() - kPrimaryZoneCentre.x(), 1e-3)); + CHECK_THAT(shifted_max.y(), WithinAbs(baseline_max.y() - kPrimaryZoneCentre.y(), 1e-3)); + + // Declared area and toolpaths must be in ONE frame. The declared bounds come from the + // first-layer convex hull, which Print::first_layer_islands() builds from the object's SLICE + // CONTOUR (lslices) -- not from any extrusion path -- so it sits outside the emitted + // centrelines by whatever wall geometry lies between the two. That distance is a property of + // the wall generator, not of this feature, so it is not asserted as a constant here. + // + // What IS asserted: the declared bounds enclose the toolpaths, and the gap between the two is + // the SAME in both frames. A frame divergence -- one of the two consumers shifted, the other + // not -- moves the declared box relative to the toolpaths and breaks this by the offset. + const XYBounds baseline_layer = perimeter_bounds(baseline_gcode, 0.3); // initial_layer_print_height 0.2 + const XYBounds shifted_layer = perimeter_bounds(shifted_gcode, 0.3); + REQUIRE_FALSE(baseline_layer.empty()); + REQUIRE_FALSE(shifted_layer.empty()); + + CHECK(shifted_min.x() <= shifted_layer.min_x + 1e-3); + CHECK(shifted_min.y() <= shifted_layer.min_y + 1e-3); + CHECK(shifted_max.x() >= shifted_layer.max_x - 1e-3); + CHECK(shifted_max.y() >= shifted_layer.max_y - 1e-3); + + CHECK_THAT(shifted_layer.min_x - shifted_min.x(), + WithinAbs(baseline_layer.min_x - baseline_min.x(), 1e-3)); + CHECK_THAT(shifted_layer.min_y - shifted_min.y(), + WithinAbs(baseline_layer.min_y - baseline_min.y(), 1e-3)); + CHECK_THAT(shifted_max.x() - shifted_layer.max_x, + WithinAbs(baseline_max.x() - baseline_layer.max_x, 1e-3)); + CHECK_THAT(shifted_max.y() - shifted_layer.max_y, + WithinAbs(baseline_max.y() - baseline_layer.max_y, 1e-3)); +} diff --git a/tests/fff_print/test_multifilament.cpp b/tests/fff_print/test_multifilament.cpp index 24f5185d8f..05d5993c07 100644 --- a/tests/fff_print/test_multifilament.cpp +++ b/tests/fff_print/test_multifilament.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -856,6 +857,63 @@ TEST_CASE("Primary-mode IMEX prints still transition to the second-layer tempera CHECK(gcode.find("M104 S240") != std::string::npos); } +// A plate carries its IMEX mode as a name, matched against the printer's imex_mode_names at +// slice time, so a mode renamed or deleted underneath the plate -- or a project opened against a +// preset that names its modes differently -- leaves the plate pointing at nothing. That used to +// take every "not Primary" branch in the exporter while every name-keyed lookup came back empty, +// which is worse than either interpretation on its own: +// * the 1st->2nd layer temperature branch is mutually exclusive with the standard one, so an +// empty active-tool roster meant NO head was transitioned and every one of them held +// nozzle_temperature_initial_layer for the whole print; and +// * the initial T was suppressed on the assumption that the mode's setup script would +// select the tool, while that script -- resolved by the same name -- did not exist. +// Print::validate() does not catch it either: the unresolved name yields an empty tools string, +// so there is no declared primary and its IMEX routing guard is skipped. +// +// Falling back to Primary is what makes the file coherent again. Asserted on the two emissions +// that were actually broken rather than on the mode string, which the placeholder test in +// test_imex_mode_gcode.cpp covers. +TEST_CASE("An IMEX plate set to a mode the printer no longer defines slices as Primary", + "[MultiFilament][IMEX]") +{ + DynamicPrintConfig config = multifilament_config(7); + imex_7x4_printer(config); + all_regions_on_filament(config, 1); // filament 1 => logical slot 0 => physical head 0 + config.set_deserialize_strict({ + // imex_mode_names is "primary;copy" -- this is `copy` after a rename. + { "imex_parallel_mode", "copy-renamed" }, + { "nozzle_temperature_initial_layer", "200" }, + { "nozzle_temperature", "240" }, + }); + + const std::string gcode = slice({ cube(20) }, config); + + // The head the toolpaths run on steps 200 -> 240 at the second layer, via the standard + // per-extruder path a Primary-mode print uses. + CHECK(gcode.find("M104 S240") != std::string::npos); + // ...and only that head, addressed the way a single-head print addresses it: nothing drives + // a copy carriage here, so the fallback goes through the standard per-extruder path, where + // only filament slot 0 prints. GCodeWriter::set_temperature therefore sees + // multiple_extruders == false and emits no tool qualifier at all. Excluding the whole + // " T" suffix rather than " T1" is what makes that the assertion: a regression that routed + // the transition to T2 or T3 instead would satisfy an exclusion of T1, and the unqualified + // find above is itself prefix-satisfied by any "M104 S240 T". + CHECK(gcode.find("M104 S240 T") == std::string::npos); + + // The initial tool selection is emitted. Matched as a line-leading token rather than a whole + // line so the assertion does not depend on the trailing "; change extruder" comment, which + // is switched off by a global unrelated to IMEX. + bool selects_initial_tool = false; + std::istringstream tool_lines(gcode); + std::string line; + while (std::getline(tool_lines, line)) + if (line.rfind("T0", 0) == 0) { + selects_initial_tool = true; + break; + } + CHECK(selects_initial_tool); +} + // IQEX: when the second gantry is active the mode drives all four carriages, so every one of // them needs its own filament resolved -- for the first layer via is_extruder_used (consumed by // machine_start_gcode) and for the second via the per-tool transition. pem routes filament 1 to @@ -947,6 +1005,108 @@ TEST_CASE("IMEX heater commands name the physical head, not the logical filament CHECK(offenders.empty()); } +// The other half of that translation, and the half nothing else covers: GCodeWriter::set_temperature +// passes `this->config.is_imex.value` as the guard, NOT a constant, so a printer that is not an +// IMEX printer keeps addressing heaters by logical filament id exactly as upstream does. +// +// physical_extruder_map is not an IMEX-only key. Shipping dual-nozzle BBL profiles author it -- +// fdm_bbl_3dp_002_common ships {1, 0} -- for the inherited BBL reading of the key, and PrintApply +// deliberately leaves a non-IMEX printer's map exactly as it arrives (IMEXHelpers.hpp spells out +// the two readings). Hardcode `true` at that call site and this whole suite still passes, because +// every other case here runs either on an IMEX printer or on one with no authored map -- while +// those profiles start sending filament 0's M104/M109 to heater 1 and filament 1's to heater 0. +// +// The two filaments carry DIFFERENT idle temperatures, so the S value and the T index cross-check +// each other: a remap moves both onto the other tool and fails both halves, and no assertion can +// be satisfied by a prefix. +// +// Idle temperature rather than nozzle_temperature, for a reason specific to THIS HARNESS. Keys in +// filament_options_with_variant are rewritten at apply time by +// update_values_to_printer_extruders_for_multiple_filaments, which sets each filament's value to +// `opt->get_at(variant_index[f])` -- it RE-INDEXES per filament by that filament's extruder +// variant, it does not flatten. Per-filament nozzle temperature is a real, working feature and +// survives that pass on a printer whose filaments resolve to different variant slots. +// multifilament_config pins nozzle_diameter to a single 0.4, so every filament here resolves to +// the SAME variant slot and therefore ends up with the same value -- which is why the IMEX cases +// above tell heads apart by tool qualifier rather than by temperature. idle_temperature is not in +// that key set, so "151,173" reaches the emitter intact and the two filaments stay distinguishable. +// Ooze prevention is what puts the idle temperatures into the file at all. +TEST_CASE("Heater commands keep the logical filament id on a non-IMEX printer", + "[MultiFilament][IMEX][Regression]") +{ + DynamicPrintConfig config = multifilament_config(2, { + { "nozzle_diameter", "0.4,0.4" }, + { "printer_extruder_id", "1,2" }, + { "printer_extruder_variant", "Direct Drive Standard,Direct Drive Standard" }, + { "extruder_printable_height", "0,0" }, + // The shipping two-nozzle profile: not an IMEX printer, but it authors the swap map. + { "is_imex", "0" }, + { "physical_extruder_map", "1,0" }, + { "single_extruder_multi_material", "0" }, + // Ooze prevention drops the outgoing filament to its idle temperature on every tool + // change, through the instance set_temperature overload that carries the guard. + { "ooze_prevention", "1" }, + { "standby_temperature_delta", "-50" }, // unused while idle_temperature is set + // Per filament and distinct: these are the values the assertions pair with a heater. + { "idle_temperature", "151,173" }, + { "nozzle_temperature_initial_layer", "215,215" }, + { "nozzle_temperature", "240,240" }, + // GCodeProcessor's preheat pass rewrites the tool-change temperature commands and drops + // the ";cooldown" M104s outright, applying physical_extruder_map itself as it does. That + // pass is not the code under test, so switch it off and let the writer's emissions stand. + { "preheat_time", "0" }, + { "enable_prime_tower", "0" }, + // The assertions spell "M104 S T"; RepRapFirmware emits "G10 S P" from the + // same code, so the flavor is pinned rather than defaulted. + { "gcode_flavor", "klipper" }, + }); + + // One filament per object, so both heaters are addressed and a tool change happens in both + // directions. Assigned at the object level: a region-level filament id would not raise the + // used-filament count the tool ordering works from. + const std::vector> overrides{ + { { "extruder", "1" } }, + { { "extruder", "2" } }, + }; + const std::string gcode = slice_with_object_overrides({ cube(20), cube(20) }, config, overrides); + + // Every heater command that names a tool, collected as temperature -> the tools it was + // addressed to. Scanning beats fixed-string finds: "M104 S151" on its own is prefix-satisfied + // by "M104 S151 T1", and excluding just " T1" would let a command routed to some third tool + // through. Mirrors the offender scan in the IMEX case above. + std::map> tools_by_temp; + std::istringstream ss(gcode); + for (std::string line; std::getline(ss, line);) { + if (line.rfind("M104", 0) != 0 && line.rfind("M109", 0) != 0) + continue; + const size_t s = line.find('S'); + const size_t t = line.find(" T"); + if (s == std::string::npos || t == std::string::npos) + continue; + if (s + 1 >= line.size() || !std::isdigit((unsigned char) line[s + 1])) + continue; + if (t + 2 >= line.size() || !std::isdigit((unsigned char) line[t + 2])) + continue; + tools_by_temp[std::stoi(line.substr(s + 1))].insert(std::stoi(line.substr(t + 2))); + } + + std::string seen; + for (const auto& [temperature, tools] : tools_by_temp) { + seen += " S" + std::to_string(temperature) + "->"; + for (int tool : tools) + seen += "T" + std::to_string(tool); + } + INFO("heater commands naming a tool:" << seen); + + // Both idle temperatures have to be in the file, or the pairing below would prove nothing. + REQUIRE(tools_by_temp.count(151) == 1); + REQUIRE(tools_by_temp.count(173) == 1); + // Filament 0's idle temperature goes to heater 0 and nowhere else, filament 1's to heater 1. + // Applying physical_extruder_map {1, 0} here would swap both. + CHECK(tools_by_temp[151] == std::set{ 0 }); + CHECK(tools_by_temp[173] == std::set{ 1 }); +} + // The IMEX Primary tool prints the sliced paths directly, so it can only use a filament the // printer's physical_extruder_map routes to it. The ghost filament picker enforces that for // the secondary tools; the primary's filament comes from the ordinary object selector, which