Bound the second-layer temperature lookup in filament-slot space

nozzle_temperature is variant-expanded, so its length is columns rather than
filament slots. On the dynamic-nozzle path that makes it longer than the
slot count, and an out-of-slot index reached get_filament_config_index() and
came back as filament 0 - the clamp the bounds-checking was meant to remove.
Bound by the slot count instead; on the ordinary path the two are equal and
nothing changes. The is_extruder_used write gains the matching lower-bound
guard.

IMEXHelpers.hpp now states both halves of the rule its call sites follow.
Bound anything derived from the extruder map against the filament slot count
before using it as a filament id, not against the option about to be read.
And a miss is -1, which is a correct tool qualifier but matches no physical
head, so it cannot serve as a skip-the-primary sentinel: which head prints a
filament is answered by the filament and the map, never by a mode role,
since a primary-mode print may use any or all tools, one at a time.

The consequence is recorded there rather than left implicit. The two skip
sites skip nothing for a slot past the end of the map, so a plate with more
slots than nozzles double-writes the primary's pressure advance. It is
narrow and unreported, and a guard there would be a smaller change than
naming a head.

The header also records that RepRapFirmware sends an unqualified pressure
advance as M572 D0, naming drive 0 absolutely rather than the active tool.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Clifford GarwoodandClaude Opus 5 committed 2026-09-18 01:32:27 -04:00
1 parent c62385214f
commit 3c31856b85
3 files changed
+50 -8

No files matched your search

+17 -6
View File
@@ -2883,8 +2883,8 @@ static std::vector<int> get_imex_active_tools(const Print& print)
if (active_mode == kImexPrimaryMode)
return active_tools;
// An unresolved mode, and a mode the tools array is too short to cover, both hand back
// an empty tools string, which parses to no tools.
// An unresolved mode, and a mode the tools array is too short to cover, both hand back an
// empty tools string, which parses to no tools.
for (const auto& [phys, role] : parse_imex_active_tools(find_imex_mode(print.config(), active_mode).active_tools))
active_tools.push_back(phys);
return active_tools;
@@ -3418,16 +3418,23 @@ void GCode::_do_export(Print& print, GCodeOutputStream &file, ThumbnailsGenerato
const auto plate_head_map = parse_imex_head_filament_map(
print.objects().front()->config().imex_head_filament_map.value);
const ConfigOptionInts& pem = print.config().physical_extruder_map;
// Bounds-checked, not get_at() -- see IMEXHelpers.hpp. The value is a skip-primary
// sentinel below, so a clamp would suppress whichever head sits at pem[0].
// Bounds-checked, not get_at(): a clamp would suppress whichever head sits at pem[0].
// A miss stays -1 and matches no head, so nothing is skipped -- see IMEXHelpers.hpp.
const int primary_physical =
((int) initial_extruder_id >= 0 &&
(int) initial_extruder_id < (int) pem.values.size())
? pem.values[(int) initial_extruder_id]
: -1;
// Bound by the array, not by the filament count. The array is padded to
// MAXIMUM_EXTRUDER_NUMBER on purpose: start G-code addresses HEADS through it
// (fdm_toolchanger_common.json gates M104 T0..T5 on it), and a parallel copy print has
// more heads than filaments by definition. PlaceholderParser clamps an out-of-range
// first_layer_temperature read to filament 0, which is the right temperature when every
// head is printing the same filament. Narrowing this to the filament count leaves the
// secondary carriages unheated.
for (int logical : imex_secondary_logical_slots(
get_imex_active_tools(print), primary_physical, plate_head_map, pem))
if (logical < (int)is_extruder_used.size())
if (logical >= 0 && logical < (int) is_extruder_used.size())
is_extruder_used[logical] = true;
}
@@ -5955,7 +5962,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.
const int num_filament_columns = (int)print.config().nozzle_temperature.values.size();
// 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());
// 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.
+15 -2
View File
@@ -122,8 +122,21 @@ std::vector<ImexMode> imex_mode_table(const ConfigBase& cfg);
//
// The same nozzle-vs-slot divergence bites in the other direction: anything derived from pem
// -- `resolve_filament_for_head`, `first_filament_for_physical_head` -- answers in NOZZLE index
// space, so bound it against the filament array you are about to index before using it as a
// filament id.
// space, so bound it against the FILAMENT SLOT COUNT before using it as a filament id. Not
// against the option you are about to read: a variant-expanded option such as
// nozzle_temperature is as long as its columns, and is_extruder_used is padded to
// MAXIMUM_EXTRUDER_NUMBER, so both admit slots the project does not have.
//
// A miss is -1, and -1 is a correct tool QUALIFIER but not a skip-the-primary sentinel: no
// physical head equals it, so a test written as `head == primary` stops skipping anything. Do
// not substitute the mode's declared primary there. Which head prints a filament is answered by
// the filament and pem, never by a mode role: a primary-mode print may use any or all tools, one
// at a time, and pem is what says which. When pem cannot answer, the head is genuinely unknown -
// an omitted qualifier then applies to the active tool, which is the head printing that filament,
// so the honest options are to leave it unqualified or to emit nothing, not to name a carriage.
// Known gap, unreported: the two skip sites in GCode.cpp therefore skip nothing for a filament
// slot past the end of pem, so a plate with more slots than nozzles double-writes the primary's
// pressure advance and hands it another filament's transition temperature.
//
// -1 as a tool qualifier reaches GCodeWriter::set_pressure_advance, which omits the qualifier
// on Klipper, Marlin and BBL but substitutes the historical `D0` on RepRapFirmware
+18
View File
@@ -400,6 +400,24 @@ TEST_CASE("imex_secondary_logical_slots - drops unrouted physicals, deduplicates
REQUIRE(out2 == std::vector<int>{0}); // both secondaries point at slot 0; only emitted once
}
TEST_CASE("imex_secondary_logical_slots - a -1 primary skips nothing", "[IMEX]") {
// Why callers must not pass the pem-miss sentinel as the primary: no physical head equals
// -1, so the primary is enumerated like a secondary and its first-routed slot is returned.
// Downstream that marks the primary's slot as loaded, emits a second pressure advance over
// the one set_extruder() already wrote, and hands it another filament's transition
// temperature. Take the primary from the mode instead (imex_primary_tool_for_mode).
ConfigOptionInts pem; pem.values = {0, 1};
CHECK(imex_secondary_logical_slots({0, 1}, /*primary*/0, {}, pem) == std::vector<int>{1});
CHECK(imex_secondary_logical_slots({0, 1}, /*primary*/-1, {}, pem) == std::vector<int>{0, 1});
}
TEST_CASE("imex_primary_tool_for_mode - names the declared primary, -1 when there is none", "[IMEX]") {
CHECK(imex_primary_tool_for_mode("0:P,1:C") == 0);
CHECK(imex_primary_tool_for_mode("2:P,0:C,1:C") == 2);
CHECK(imex_primary_tool_for_mode("0:C,1:C") == -1);
CHECK(imex_primary_tool_for_mode("") == -1);
}
TEST_CASE("imex_secondary_logical_slots - only-primary-active returns empty", "[IMEX]") {
// Primary mode (just T0 active) → no secondaries.
auto pem = make_pem({0, 0, 0, 0, 1, 2, 3});