mirror of
https://github.com/OrcaSlicer/OrcaSlicer.git
synced 2026-10-10 17:21:10 +00:00
fix(imex): name the physical head in M104/M109 tool indices
M104/M109 address a heater, but every caller of the instance GCodeWriter::set_temperature overload addresses filaments by logical id, so on a printer whose physical_extruder_map is not the identity the emitted T named the wrong head -- or, where the logical id exceeds the head count, no head at all. With a map of 0,0,0,0,1,2,3 a toolchange to filament 5 emitted "M109 S265 T4" and "M104 S190 T4 ;cooldown" while the head it meant was T1. Upstream already treats these commands as physical: the preheat it injects in GCodeProcessor maps through the same map before emitting, and BBS's own wipe tower does likewise. Emitting logical is the half that never got the memo. That mismatch also disabled the cooldown suppression beside the preheat, which compares the line's T against pem[tool_number] and so never matched a logical one -- 171 cooldowns survived in a two-head print where none should have. Worse, it could match the wrong line: a cooldown for filament 1 emitted T1, and a toolchange to filament 5 gives pem[4] == 1, so a legitimate cooldown for head 0 was deleted because the incoming head happened to be numbered 1. Translate once, in the instance overload every logical-space caller passes through. The static overload is already physical-in and is left alone. Gated on is_imex. physical_extruder_map carries two readings in this tree: the BBS paths index it by extruder id, the IMEX paths by filament id, and the two coincide only when the filament and nozzle counts match. Mapping unconditionally would impose the IMEX reading on profiles that mean the other one -- fdm_bbl_3dp_002_common ships a non-identity [1,0], spared today only because single_extruder_multi_material suppresses the T qualifier entirely. The wipe tower's interface-temperature pass has to move with it. It strips the M109 that post_toolchange emits by searching for that filament's tool index, so it now searches for the mapped one; left alone it would have stopped matching, and the surviving blocking M109 would have silently defeated the interface temperature. Its sibling pass reads WipeTower2 output, which emits no T at all, and is deliberately unchanged. The bare T<n> toolchange stays logical -- it selects an AFC lane, not a heater. Test slices two objects across a head boundary, the only case that reaches this emission: the same-physical short-circuit in set_extruder suppresses the cooldown entirely for lane swaps within one head. It scans every M104/M109 rather than matching fixed strings, so it catches any unmapped emission and not just the two sites changed here. With the mapping neutered it reports 101 offending lines; with it in place, none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
f0778ef5fa
commit
2e883df7d9
+11
-2
@@ -1617,7 +1617,16 @@ static std::vector<Vec2d> get_path_of_change_filament(const Print& print)
|
||||
: gcodegen.config().nozzle_temperature.get_at(new_fi);
|
||||
if (std::abs(tcr.print_z) < EPSILON)
|
||||
base_temp = gcodegen.config().nozzle_temperature_initial_layer.get_at(new_fi);
|
||||
const std::string t_token = " T" + std::to_string(new_extruder_id);
|
||||
// This pass strips the M109 that OozePrevention::post_toolchange just emitted into
|
||||
// toolchange_gcode_str, so it must look for the tool index that emission actually
|
||||
// wrote -- physical, translated by GCodeWriter::set_temperature -- not the logical
|
||||
// id the temperature lookups above use. Matching on the logical id would leave the
|
||||
// blocking M109 in place and silently defeat the tower interface temperature.
|
||||
// (The sibling scan further down reads WipeTower2 output, whose set_extruder_temp
|
||||
// emits no T at all, so it is deliberately left alone.)
|
||||
const int heater_id = imex_physical_heater_for(
|
||||
gcodegen.config().is_imex.value, gcodegen.config().physical_extruder_map, new_extruder_id);
|
||||
const std::string t_token = " T" + std::to_string(heater_id);
|
||||
std::string out;
|
||||
out.reserve(toolchange_gcode_str.size());
|
||||
size_t pos = 0;
|
||||
@@ -1638,7 +1647,7 @@ static std::vector<Vec2d> get_path_of_change_filament(const Print& print)
|
||||
const std::string t_val = trimmed.substr(t_pos + 1, t_end == std::string::npos ? std::string::npos : t_end - (t_pos + 1));
|
||||
if (!t_val.empty()) {
|
||||
try {
|
||||
matches_extruder = std::stoi(t_val) == new_extruder_id;
|
||||
matches_extruder = std::stoi(t_val) == heater_id;
|
||||
} catch (...) {
|
||||
matches_extruder = false;
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
#include "CustomGCode.hpp"
|
||||
#include "I18N.hpp"
|
||||
#include "PrintConfig.hpp"
|
||||
#include "IMEXHelpers.hpp"
|
||||
#include "ClipperUtils.hpp"
|
||||
#include "Geometry/ArcWelder.hpp"
|
||||
#include "Line.hpp"
|
||||
@@ -285,8 +286,16 @@ std::string GCodeWriter::set_temperature(unsigned int temperature, GCodeFlavor f
|
||||
std::string GCodeWriter::set_temperature(unsigned int temperature, bool wait, int tool) const
|
||||
{
|
||||
// set tool to -1 to make sure we won't emit T parameter for single extruder or SEMM
|
||||
if (!this->multiple_extruders || m_single_extruder_multi_material)
|
||||
if (!this->multiple_extruders || m_single_extruder_multi_material) {
|
||||
tool = -1;
|
||||
} else {
|
||||
// Every caller of this overload addresses filaments by LOGICAL id, but M104/M109
|
||||
// name a physical heater -- so translate at the one point they all pass through.
|
||||
// The static overload below is already physical-in (GCode.cpp:5931,
|
||||
// GCode/GCodeProcessor.cpp:1410) and must not be remapped, which is why the
|
||||
// translation lives here and not there.
|
||||
tool = imex_physical_heater_for(this->config.is_imex.value, this->config.physical_extruder_map, tool);
|
||||
}
|
||||
return set_temperature(temperature, this->config.gcode_flavor, wait, tool);
|
||||
}
|
||||
|
||||
|
||||
@@ -44,6 +44,15 @@ int imex_pem_tool_for(int filament_id, const std::string& parallel_mode, const C
|
||||
return pem.get_at(filament_id);
|
||||
}
|
||||
|
||||
int imex_physical_heater_for(bool is_imex, const ConfigOptionInts& pem, int logical_id)
|
||||
{
|
||||
// Bounds-check rather than get_at(): get_at() CLAMPS to values.front(), which would
|
||||
// silently retarget an out-of-range id at whatever heater sits in slot 0.
|
||||
if (!is_imex || logical_id < 0 || logical_id >= (int) pem.values.size())
|
||||
return logical_id;
|
||||
return pem.values[logical_id];
|
||||
}
|
||||
|
||||
bool imex_suppresses_bare_toolchange(const std::string& parallel_mode, unsigned int toolchange_count)
|
||||
{
|
||||
return toolchange_count <= 1
|
||||
|
||||
@@ -87,6 +87,22 @@ ConfigOptionInts effective_physical_extruder_map(const PresetBundle& pb);
|
||||
// firmware commands like any non-IMEX printer.
|
||||
int imex_pem_tool_for(int filament_id, const std::string& parallel_mode, const ConfigOptionInts& pem);
|
||||
|
||||
// Translate a logical filament id into the physical heater that an M104/M109 `T` must
|
||||
// name. Applies in every IMEX mode including Primary -- a heater command names hardware,
|
||||
// so it does not depend on a parallel mode being active the way imex_pem_tool_for does.
|
||||
//
|
||||
// Gated on is_imex because physical_extruder_map carries two readings in this tree: the
|
||||
// BBL paths index it by extruder id (GCode.cpp:3333, WipeTower.cpp:1353), the IMEX paths
|
||||
// by filament id. Those coincide only when the filament and nozzle counts match, so an
|
||||
// ungated mapping would impose the IMEX reading on profiles that mean the other one --
|
||||
// fdm_bbl_3dp_002_common ships a non-identity [1,0] and is spared today only because
|
||||
// single_extruder_multi_material suppresses the T qualifier entirely.
|
||||
//
|
||||
// Out-of-range ids pass through unchanged, the same rule GCodeProcessor's preheat rewrite
|
||||
// uses (GCode/GCodeProcessor.cpp:1407), so a printer whose map is the registered
|
||||
// single-entry default is unaffected.
|
||||
int imex_physical_heater_for(bool is_imex, const ConfigOptionInts& pem, int logical_id);
|
||||
|
||||
// True when GCode::set_extruder should suppress its bare T<n> at the print-start
|
||||
// initial-tool selection because the active IMEX parallel mode's setup macro
|
||||
// (imex_mode_gcode) and machine_start_gcode already activate the primary tool
|
||||
|
||||
@@ -894,6 +894,59 @@ TEST_CASE("IQEX modes emit first- and second-layer temperatures for every active
|
||||
CHECK(gcode.find("M104 S240 T3") != std::string::npos);
|
||||
}
|
||||
|
||||
// M104/M109 name a physical heater, but every caller of the instance set_temperature overload
|
||||
// addresses filaments by logical id. pem routes filament 5 (logical 4) to head 1, so a
|
||||
// toolchange between filaments 1 and 5 must cool and wait on T1 -- never T4, which on this
|
||||
// machine is an AFC lane index and names no heater at all.
|
||||
//
|
||||
// Regression: the same-physical short-circuit in set_extruder hides this for lane swaps within
|
||||
// one head (filaments 1-4 all map to head 0, so no cool-down is emitted), so only a toolchange
|
||||
// that CROSSES heads reaches the emission. Ooze prevention must be on for pre/post_toolchange
|
||||
// to run at all.
|
||||
TEST_CASE("IMEX heater commands name the physical head, not the logical filament",
|
||||
"[MultiFilament][IMEX][Regression]")
|
||||
{
|
||||
DynamicPrintConfig config = multifilament_config(7);
|
||||
imex_7x4_printer(config);
|
||||
config.set_deserialize_strict({
|
||||
{ "imex_parallel_mode", "primary" },
|
||||
{ "ooze_prevention", "1" },
|
||||
{ "standby_temperature_delta", "-50" },
|
||||
{ "single_extruder_multi_material", "0" },
|
||||
{ "nozzle_temperature_initial_layer", "200,200,200,200,200,200,200" },
|
||||
{ "nozzle_temperature", "240,240,240,240,240,240,240" },
|
||||
});
|
||||
|
||||
// Two objects on filaments 1 and 5: logical 0 -> head 0, logical 4 -> head 1.
|
||||
const std::vector<std::vector<ConfigBase::SetDeserializeItem>> overrides{
|
||||
{ { "extruder", "1" } },
|
||||
{ { "extruder", "5" } },
|
||||
};
|
||||
const std::string gcode = slice_with_object_overrides({ cube(20), cube(20) }, config, overrides);
|
||||
|
||||
// The bare toolchange stays LOGICAL -- it is an AFC lane selector, not a heater.
|
||||
CHECK(gcode.find("\nT4") != std::string::npos);
|
||||
|
||||
// Every heater command carrying a tool must name a configured head (0-3 here), never a
|
||||
// logical slot above the head count. Scanning beats a fixed-string check: it fails on any
|
||||
// stray unmapped emission, not just the two sites this test was written for.
|
||||
std::istringstream ss(gcode);
|
||||
std::string line;
|
||||
std::vector<std::string> offenders;
|
||||
while (std::getline(ss, line)) {
|
||||
if (line.rfind("M104", 0) != 0 && line.rfind("M109", 0) != 0)
|
||||
continue;
|
||||
const size_t t = line.find(" T");
|
||||
if (t == std::string::npos || t + 2 >= line.size() || !std::isdigit((unsigned char) line[t + 2]))
|
||||
continue;
|
||||
if (std::stoi(line.substr(t + 2)) > 3)
|
||||
offenders.push_back(line);
|
||||
}
|
||||
INFO("heater commands naming a non-existent head: " << offenders.size()
|
||||
<< (offenders.empty() ? "" : " e.g. " + offenders.front()));
|
||||
CHECK(offenders.empty());
|
||||
}
|
||||
|
||||
// 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
|
||||
|
||||
Reference in New Issue
Block a user