spoolman: add per-tool spool tracking for multi-tool printers#1059
spoolman: add per-tool spool tracking for multi-tool printers#1059goeland86 wants to merge 4 commits intoArksine:masterfrom
Conversation
Replace single spool_id with a tool-indexed map to support StealthChanger, IDEX, and MMU setups. Each tool gets independent spool assignment and filament usage tracking. - Replace spool_id attr with _tool_spool_map + backward-compat property - Add toolchanger discovery on klippy_ready with extruder fallback - Per-tool _highest_epos watermarks for extrusion attribution - API endpoints accept optional tool param (default 0) - Status response includes tool_spool_map - Notifications include tool field - History tracking via tool_spool_map auxiliary field - Spool deletion iterates full tool map - DB migration from legacy single spool key Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Jon Charnas <goeland86@gmail.com>
Test tool-spool map CRUD, backward-compat spool_id property, extrusion attribution across tool changes, extruder discovery, spool deletion handling, API endpoints, DB persistence, migration, and history callbacks. 54 tests covering single and multi-tool scenarios. Signed-off-by: Jon Charnas <goeland86@gmail.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WebRequest.get_int() does not accept None as a default. Use web_request.get() with manual int conversion for the optional tool parameter on GET requests. Signed-off-by: Jon Charnas <goeland86@gmail.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Thanks. Before I get into a detailed code review there are a couple of high level issues that we need to work out. Maintaining compatibility with existing workflowsIt appears to me that this could potentially break existing flows for filament changes in multi-extruder setups. If the user issues Moonraker can't depend on APIs or printer objects that are not included in the official repoThis may be the bigger issue. We can't refer to objects or APIs on forks and/or modifications to Klipper. The potential for breakage and/or maintenance issues is too high. I can't reliably be sure that the modification will be maintained in sync with the current Klipper repo and I can't be sure that Klipper won't at some point in the future add a printer object with the same name. I also don't want to open up the can of worms that would exist when authors of other mods want to integrate directly with Moonraker. If its possible to rewrite this with only dependencies on current hardcoded Klipper objects I will be open to the changes. Otherwise this could be revisited if MMU/Toolchanger support is merged in the official repo. Edit: In addition, I just noticed that this PR includes AI generated code. I'm not sure yet what my policy will be on this, as we just started having these discussions. I may limit it to tests, analysis, and docs only. I may open it up to Moonraker's source with the caveat that only prior contributors that have demonstrated a working knowledge of the code base can depend on generative AI tools. There are potential licensing issues with AI generated code that need to be discussed, and ultimately I would like Moonraker (and Katapult) to have a policy that is consistent with Klipper. |
|
So I thought I'd double checked single tool use-case, but I hadn't looked in detail at a multiplexing system. My main goal was for tool changer/IDEX support. So I'll see what tests I can run since I don't have an AMS-like system yet. Regarding the unofficial printer objects, I'll try to refactor it as best I can. Is there a way to create moonraker add-ons that aren't independent services? If it's better to rework it in that way I'm happy to do so if refactoring proves impractical. Regarding AI-generated code, I fully understand your concerns. I write C++ for a living, and I've worked a long time on the Recore's embedded image project as well. Grappling with licensing is something I'm quite familiar with, and I won't push you one way or another on that stance, but what I will say is that it's a lot less of a legal hurdle for open source projects than closed-source. It becomes primarily a matter of attribution, which the LLM in question may be able to properly trace. If the code generated passes muster, both in real-world and unit-test cases, I see value in the output. I have been checking and re-directing Claude on a lot of points, regarding functionality as well as architecture, even if I'm not the most competent python programmer. |
1. When tool is omitted from set_active_spool() or the POST API, apply the spool to ALL currently tracked tools instead of just tool 0. This preserves backward compatibility with existing filament-change workflows that call set_active_spool(spool_id) without specifying a tool. 2. Remove toolchanger dependency (_discover_toolchanger_tools and _handle_toolchanger_update). Tool discovery now relies solely on standard Klipper extruder objects (extruder, extruder1, etc.) which are part of the official Klipper repo. Signed-off-by: Jon Charnas <goeland86@gmail.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the single
spool_idwith a tool-indexed map to support StealthChanger, IDEX, and MMU setups. Each tool gets independent spool assignment and filament usage tracking.Changes:
spool_idattribute with_tool_spool_mapdict + backward-compatiblespool_idproperty (returns tool 0)klippy_readywith extruder name fallback_highest_eposwatermarks for correct extrusion attribution across tool changesGET/POST /server/spoolman/spool_id) accept optionaltoolparameter (default 0)GET /server/spoolman/statusresponse includestool_spool_mapspoolman:active_spool_setnotification includestoolfieldtool_spool_mapauxiliary fieldspoolman.spool_idkey tospoolman.tool_spool_map; both keys maintained for rollback safetyBackward compatibility:
toolparameter; existing single-tool setups work identicallyspool_idproperty returns tool 0's spool for code that reads it directlyTests:
Fixes #1057
Signed-off-by: Jon Charnas goeland86@gmail.com