mirror of
https://github.com/OrcaSlicer/OrcaSlicer.git
synced 2026-10-11 09:51:06 +00:00
refactor(imex): extract imex_pem_tool_for helper + unit tests
The physical_extruder_map translation used by IMEX per-tool PA emission was
inlined identically at two sites in GCode.cpp (tool-change and second-layer
transition). Extract to IMEXHelpers so the routing rule ("parallel mode AND
populated pem → physical index, else -1") is testable in isolation and the
call sites read as intent rather than re-deriving the conditional.
Production change is behavior-preserving:
- Same predicate (`!mode.empty() && mode != "primary"`)
- Same empty-pem short-circuit returning -1
- Same get_at() dispatch on hit
- Both call sites replaced with a single call
Four unit tests in [IMEX] cover the routing matrix:
- non-IMEX ("") and primary mode short-circuit
- parallel mode + empty pem short-circuits (defense-in-depth; get_at would
throw on empty values otherwise)
- identity pem (non-MMU IDEX) routes filament to itself
- MMU collapse routes multiple logical slots to one physical (7-slot profile
with 4-lane MMU on physical 0 and direct drives on 1/2/3)
All IMEX + Variant regression suites pass post-refactor (133 assertions / 48
cases under libslic3r, 25 assertions / 6 cases under fff_print [PressureAdvance]).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
cb15f35444
commit
c2492ccc47
+2
-12
@@ -7589,13 +7589,7 @@ std::string GCode::set_extruder(unsigned int new_filament_id, double print_z, bo
|
|||||||
// In IMEX parallel modes each carriage needs an explicit tool address.
|
// In IMEX parallel modes each carriage needs an explicit tool address.
|
||||||
// In primary mode (single active tool) regular tool changes handle PA
|
// In primary mode (single active tool) regular tool changes handle PA
|
||||||
// so no qualifier is needed — same as a non-IMEX printer.
|
// so no qualifier is needed — same as a non-IMEX printer.
|
||||||
// Guard the pem lookup: PrintApply populates pem when printer_extruder_id
|
const int pa_tool = imex_pem_tool_for((int)new_filament_id, m_imex_parallel_mode, m_config.physical_extruder_map);
|
||||||
// is set, but defense-in-depth prevents a throw from get_at on any
|
|
||||||
// empty-pem path that might slip through in exotic profiles.
|
|
||||||
const bool imex_parallel = !m_imex_parallel_mode.empty() && m_imex_parallel_mode != "primary";
|
|
||||||
const int pa_tool = (imex_parallel && !m_config.physical_extruder_map.values.empty())
|
|
||||||
? m_config.physical_extruder_map.get_at((int)new_filament_id)
|
|
||||||
: -1;
|
|
||||||
gcode += m_writer.set_pressure_advance(m_config.pressure_advance.get_at(new_filament_id), pa_tool);
|
gcode += m_writer.set_pressure_advance(m_config.pressure_advance.get_at(new_filament_id), pa_tool);
|
||||||
// Orca: Adaptive PA
|
// Orca: Adaptive PA
|
||||||
// Reset Adaptive PA processor last PA value
|
// Reset Adaptive PA processor last PA value
|
||||||
@@ -7895,11 +7889,7 @@ std::string GCode::set_extruder(unsigned int new_filament_id, double print_z, bo
|
|||||||
gcode += m_ooze_prevention.post_toolchange(*this);
|
gcode += m_ooze_prevention.post_toolchange(*this);
|
||||||
|
|
||||||
if (m_config.enable_pressure_advance.get_at(new_filament_id)) {
|
if (m_config.enable_pressure_advance.get_at(new_filament_id)) {
|
||||||
// Empty-pem guard mirrors the earlier PA site; get_at throws on empty values.
|
const int pa_tool = imex_pem_tool_for((int)new_filament_id, m_imex_parallel_mode, m_config.physical_extruder_map);
|
||||||
const bool imex_parallel = !m_imex_parallel_mode.empty() && m_imex_parallel_mode != "primary";
|
|
||||||
const int pa_tool = (imex_parallel && !m_config.physical_extruder_map.values.empty())
|
|
||||||
? m_config.physical_extruder_map.get_at((int)new_filament_id)
|
|
||||||
: -1;
|
|
||||||
gcode += m_writer.set_pressure_advance(m_config.pressure_advance.get_at(new_filament_id), pa_tool);
|
gcode += m_writer.set_pressure_advance(m_config.pressure_advance.get_at(new_filament_id), pa_tool);
|
||||||
// Orca: Adaptive PA
|
// Orca: Adaptive PA
|
||||||
// Reset Adaptive PA processor last PA value
|
// Reset Adaptive PA processor last PA value
|
||||||
|
|||||||
@@ -33,6 +33,14 @@ ConfigOptionInts effective_physical_extruder_map(const PresetBundle& pb)
|
|||||||
return effective_physical_extruder_map(explicit_pem, pei);
|
return effective_physical_extruder_map(explicit_pem, pei);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
int imex_pem_tool_for(int filament_id, const std::string& parallel_mode, const ConfigOptionInts& pem)
|
||||||
|
{
|
||||||
|
const bool imex_parallel = !parallel_mode.empty() && parallel_mode != "primary";
|
||||||
|
if (!imex_parallel || pem.values.empty())
|
||||||
|
return -1;
|
||||||
|
return pem.get_at(filament_id);
|
||||||
|
}
|
||||||
|
|
||||||
int first_filament_for_physical_head(const ConfigOptionInts& pem, int physical)
|
int first_filament_for_physical_head(const ConfigOptionInts& pem, int physical)
|
||||||
{
|
{
|
||||||
const auto& v = pem.values;
|
const auto& v = pem.values;
|
||||||
|
|||||||
@@ -30,6 +30,14 @@ ConfigOptionInts effective_physical_extruder_map(const ConfigOptionInts* explici
|
|||||||
// all agree on what the slicer will see.
|
// all agree on what the slicer will see.
|
||||||
ConfigOptionInts effective_physical_extruder_map(const PresetBundle& pb);
|
ConfigOptionInts effective_physical_extruder_map(const PresetBundle& pb);
|
||||||
|
|
||||||
|
// Returns the physical extruder index to emit as a per-tool qualifier (PA / temperature)
|
||||||
|
// for the given filament slot, or -1 when the current IMEX state does not warrant a
|
||||||
|
// per-tool qualification: non-IMEX / primary mode, or the pem is empty / unpopulated.
|
||||||
|
// Used at tool-change and second-layer transition call sites where IMEX parallel modes
|
||||||
|
// route emission through the physical extruder, while ordinary tool changes emit bare
|
||||||
|
// firmware commands like any non-IMEX printer.
|
||||||
|
int imex_pem_tool_for(int filament_id, const std::string& parallel_mode, const ConfigOptionInts& pem);
|
||||||
|
|
||||||
// Returns the lowest 0-based logical filament index L such that pem[L] == physical.
|
// Returns the lowest 0-based logical filament index L such that pem[L] == physical.
|
||||||
// Returns -1 if no filament routes to `physical`.
|
// Returns -1 if no filament routes to `physical`.
|
||||||
// Degenerate case: an empty pem returns 0 when `physical == 0` (identity-on-head-0
|
// Degenerate case: an empty pem returns 0 when `physical == 0` (identity-on-head-0
|
||||||
|
|||||||
@@ -49,6 +49,40 @@ TEST_CASE("effective_physical_extruder_map — IDEX ghost-color regression guard
|
|||||||
REQUIRE(first_filament_for_physical_head(pem, 1) == 1); // no longer -1
|
REQUIRE(first_filament_for_physical_head(pem, 1) == 1); // no longer -1
|
||||||
}
|
}
|
||||||
|
|
||||||
|
TEST_CASE("imex_pem_tool_for — non-parallel mode returns -1", "[IMEX]") {
|
||||||
|
// Empty mode string (not in IMEX) and "primary" (IMEX present but not parallel) both
|
||||||
|
// short-circuit to bare-firmware PA/temp emission.
|
||||||
|
auto pem = make_pem({0, 1, 2});
|
||||||
|
REQUIRE(imex_pem_tool_for(0, "", pem) == -1);
|
||||||
|
REQUIRE(imex_pem_tool_for(1, "primary", pem) == -1);
|
||||||
|
REQUIRE(imex_pem_tool_for(2, "primary", pem) == -1);
|
||||||
|
}
|
||||||
|
|
||||||
|
TEST_CASE("imex_pem_tool_for — parallel mode with empty pem returns -1", "[IMEX]") {
|
||||||
|
// Defense-in-depth for exotic profiles where pem never gets populated. get_at would
|
||||||
|
// throw on an empty pem; the helper must return -1 instead so the writer emits bare.
|
||||||
|
ConfigOptionInts empty_pem;
|
||||||
|
REQUIRE(imex_pem_tool_for(0, "copy_mode", empty_pem) == -1);
|
||||||
|
REQUIRE(imex_pem_tool_for(3, "mirror_mode", empty_pem) == -1);
|
||||||
|
}
|
||||||
|
|
||||||
|
TEST_CASE("imex_pem_tool_for — identity pem routes filament to itself", "[IMEX]") {
|
||||||
|
// IDEX with printer_extruder_id = [1, 2] auto-derives pem = [0, 1]; non-MMU is identity.
|
||||||
|
auto pem = make_pem({0, 1});
|
||||||
|
REQUIRE(imex_pem_tool_for(0, "copy_mode", pem) == 0);
|
||||||
|
REQUIRE(imex_pem_tool_for(1, "copy_mode", pem) == 1);
|
||||||
|
}
|
||||||
|
|
||||||
|
TEST_CASE("imex_pem_tool_for — MMU collapse routes multiple logical slots to one physical", "[IMEX]") {
|
||||||
|
// 7-slot printer: first four slots share physical extruder 0 (a 4-lane MMU),
|
||||||
|
// slots 4/5/6 are independent on 1/2/3. Filament index is the logical slot;
|
||||||
|
// the helper returns the physical extruder carrying it.
|
||||||
|
auto pem = make_pem({0, 0, 0, 0, 1, 2, 3});
|
||||||
|
REQUIRE(imex_pem_tool_for(0, "copy_mode", pem) == 0);
|
||||||
|
REQUIRE(imex_pem_tool_for(3, "copy_mode", pem) == 0); // MMU collapse: T3 logical → T0 physical
|
||||||
|
REQUIRE(imex_pem_tool_for(6, "copy_mode", pem) == 3);
|
||||||
|
}
|
||||||
|
|
||||||
TEST_CASE("first_filament_for_physical_head — identity pem", "[IMEX]") {
|
TEST_CASE("first_filament_for_physical_head — identity pem", "[IMEX]") {
|
||||||
auto pem = make_pem({0, 1, 2, 3});
|
auto pem = make_pem({0, 1, 2, 3});
|
||||||
REQUIRE(first_filament_for_physical_head(pem, 0) == 0);
|
REQUIRE(first_filament_for_physical_head(pem, 0) == 0);
|
||||||
|
|||||||
Reference in New Issue
Block a user