From 8f14580e30eebdc7e79cb1734da68c2176ba9fad Mon Sep 17 00:00:00 2001 From: viewit Date: Fri, 24 Jul 2026 13:57:17 +0200 Subject: [PATCH] fix(ams): don't crash on rejected multiColorBox setInfo (Issue #100) The printer replies with state="failed" and data as a 2-element list (["multi_color_box", [...]]) instead of the usual dict when it rejects a manual ACE slot filament assignment (e.g. custom-RFID/third-party material). _on_multicolor_box called data.get(...) unconditionally, crashing with AttributeError and silently dropping the report - including the regular slot-state update that would otherwise follow. Add a state="failed" guard plus a defensive isinstance check, and surface the rejection via _state["last_ams_set_error"] so the caller of handle_api_ams_set_slot's optimistic cache update isn't left showing a false success. Note: this fixes the crash, not necessarily the underlying rejection itself - the printer/ACE may still refuse assignments for material types it doesn't recognize. --- NIGHTLY_CHANGELOG.md | 1 + kobrax_moonraker_bridge.py | 9 +++ tests/test_multicolor_box_failed_state.py | 69 +++++++++++++++++++++++ 3 files changed, 79 insertions(+) create mode 100644 tests/test_multicolor_box_failed_state.py diff --git a/NIGHTLY_CHANGELOG.md b/NIGHTLY_CHANGELOG.md index 43259d9..7ae28db 100644 --- a/NIGHTLY_CHANGELOG.md +++ b/NIGHTLY_CHANGELOG.md @@ -3,3 +3,4 @@ - Fix: on printers with multiple daisy-chained ACE units and no toolhead buffer (e.g. Kobra S1 with 2 ACE Pro), only the first unit's 4 slots were ever shown on the dashboard or synced to OrcaSlicer — the bridge silently discarded every ACE unit after the first. All units now show up correctly (Issue #95, thanks @hoovercl for the detailed report and logs) - Feat: the GCode browser now supports multi-select and bulk delete — click any file's checkbox to enter select mode, use "Select All" to grab everything currently visible (respecting your active search/filter), then delete the selection with one confirmation instead of one-by-one (Issue #94, thanks @Blaim) - Fix: **printing via OrcaSlicer "Upload and print" failed (or fed the wrong spool) when a slot below the used filament was empty** — e.g. printing with Filament 4 while slot 3 was empty. The generated AMS slot mapping inserted a placeholder pointing at the physically empty tray, which the printer rejects. Placeholders now point at a loaded tray instead. Printing with all slots full already worked and is unchanged. +- Fix: manually assigning a filament profile to an ACE slot could crash the MQTT callback (`'list' object has no attribute 'get'`) when the printer rejected the assignment, e.g. for custom-RFID/third-party filament. The bridge now handles the printer's failure response cleanly instead of crashing (Issue #100, thanks @Blaim) diff --git a/kobrax_moonraker_bridge.py b/kobrax_moonraker_bridge.py index 6215f03..81a5a1b 100644 --- a/kobrax_moonraker_bridge.py +++ b/kobrax_moonraker_bridge.py @@ -1879,10 +1879,18 @@ class KobraXBridge: return activity def _on_multicolor_box(self, payload: dict): + if payload.get("state") == "failed": + log.warning(f"multiColorBox/report failed: {payload.get('data')}") + self._state["last_ams_set_error"] = True + return data = payload.get("data") or {} + if not isinstance(data, dict): + log.warning(f"multiColorBox/report: unexpected data shape: {data!r}") + return boxes = data.get("multi_color_box") or [] if not boxes: return + self._state["last_ams_set_error"] = False self._head_tools_model = int(data.get("head_tools_model", self._head_tools_model)) self._filament_mode = self._detect_filament_mode(boxes, self._head_tools_model) self._state["filament_mode"] = self._filament_mode @@ -4037,6 +4045,7 @@ class KobraXBridge: return web.json_response({"error": "color must be [r,g,b]"}, status=400) box_id, local_slot = self._global_to_box_slot(index) loop = asyncio.get_event_loop() + self._state["last_ams_set_error"] = False # setInfo goes via the web/printer topic (like tempature/set). Verified via # Workbench-Vue mqtt_setInfo verifiziert — via slicer/printer/ wurden # slot changes are ignored by the printer and overwritten with the old diff --git a/tests/test_multicolor_box_failed_state.py b/tests/test_multicolor_box_failed_state.py new file mode 100644 index 0000000..16fd71b --- /dev/null +++ b/tests/test_multicolor_box_failed_state.py @@ -0,0 +1,69 @@ +"""AttributeError crash on multiColorBox/report failure (Issue #100). + +Real KX2-Pro bug: manually assigning a filament profile to an ACE slot +(custom-RFID / third-party filament) gets rejected by the printer. Instead of +echoing the normal success shape, the printer replies with `state: "failed"` +and `data` as a 2-element LIST (`["multi_color_box", [...]]`) instead of the +usual dict (`{"multi_color_box": [...]}`). `_on_multicolor_box` called +`data.get(...)` unconditionally, crashing with +`AttributeError: 'list' object has no attribute 'get'` and silently dropping +the report (including the slot-state update it would otherwise have done). +""" +import argparse +import tempfile +from unittest.mock import MagicMock + +from kobrax_moonraker_bridge import KobraXBridge + +# Exact failure payload from the Issue #100 log. +FAILED_PAYLOAD = { + "state": "failed", + "data": ["multi_color_box", [{"filaments": {"id": 2}, "id": 0}]], +} + +SUCCESS_PAYLOAD = { + "state": "success", + "data": { + "head_tools_model": 1, + "multi_color_box": [ + {"id": -1, "slots": []}, + { + "id": 0, + "slots": [ + {"index": 0, "type": "PLA", "color": [0, 156, 189], "status": 5}, + ], + }, + ], + }, +} + + +def _bridge(): + c = MagicMock(); c.callbacks = {}; c.connected = False + args = argparse.Namespace( + printer_ip="", mqtt_port=9883, username="", password="", + mode_id="20030", device_id="", host="127.0.0.1", port=7125, + data_dir=tempfile.mkdtemp(prefix="kxmcb-"), + ) + return KobraXBridge(c, args=args) + + +def test_failed_state_does_not_crash(): + b = _bridge() + b._on_multicolor_box(FAILED_PAYLOAD) # must not raise + assert b._state["last_ams_set_error"] is True + + +def test_success_after_failure_clears_error_flag(): + b = _bridge() + b._on_multicolor_box(FAILED_PAYLOAD) + assert b._state["last_ams_set_error"] is True + b._on_multicolor_box(SUCCESS_PAYLOAD) + assert b._state["last_ams_set_error"] is False + + +def test_non_dict_data_without_failed_state_does_not_crash(): + """Defensive guard: any future non-dict `data` shape must not crash, + even if the printer doesn't set state="failed" for it.""" + b = _bridge() + b._on_multicolor_box({"state": "success", "data": ["multi_color_box", []]})