Browse Source

fix(inventory): reconcile external-spool assignment when its filament type changes (#2575)

Assigning a new filament to the external spool (e.g. generic ABS over
generic TPU) left the previous inventory spool assigned. The reconciliation
that unlinks a stale external-spool assignment lives in on_ams_change, but
that callback only fired on regular AMS-unit changes: its change-hash never
included the external spool (vt_tray/vir_slot), and the external-spool data
is stored after the AMS handler runs.

Detect external-spool identity changes (type, colour, tag, or reset to
empty) and re-fire on_ams_change so the stale assignment is unlinked. The
fill percentage (remain) is excluded from the fingerprint so a running
print doesn't trigger it on every push.
maziggy 1 month ago
parent
commit
e336eb53e7

File diff suppressed because it is too large
+ 16 - 0
CHANGELOG.md


+ 45 - 0
backend/app/services/bambu_mqtt.py

@@ -570,6 +570,10 @@ class BambuMQTTClient:
         self._raw_message_handlers: list[Callable[[str, bytes], None]] = []
         self._disconnection_event: threading.Event | None = None
         self._previous_ams_hash: str | None = None  # Track AMS changes
+        # Track external-spool (vt_tray) identity changes separately: the AMS
+        # hash above covers only AMS units, so an external-spool-only filament
+        # swap would never re-trigger inventory reconciliation (#2575).
+        self._previous_vt_tray_hash: str | None = None
 
         # Cache AMS firmware/SN from get_version in case it arrives before AMS status
         # Key: ams_id (int). Value: {'sw_ver': str, 'sn': str}
@@ -1188,6 +1192,14 @@ class BambuMQTTClient:
                         vt_tray = [vt_tray]
                     self.state.raw_data["vt_tray"] = vt_tray
 
+            # The regular AMS change-hash (in _handle_ams_data) only sees AMS
+            # units, and _handle_ams_data runs before this block — so a change
+            # to the external spool alone (e.g. swapping generic TPU for generic
+            # ABS on the printer) never re-triggers on_ams_change, leaving a
+            # stale inventory assignment on the ams_id=255 slot (#2575). Detect
+            # external-spool identity changes here and fire the same callback.
+            self._maybe_trigger_external_spool_change()
+
             # Parse ams_status directly from print data (NOT from print.ams)
             # ams_status is a combined value: lower 8 bits = sub status, bits 8-15 = main status
             # Main status: 0=idle, 1=filament_change, 2=rfid_identifying, 3=assist, 4=calibration
@@ -1669,6 +1681,39 @@ class BambuMQTTClient:
             return candidates.pop()
         return None
 
+    def _maybe_trigger_external_spool_change(self):
+        """Fire on_ams_change when the external spool (vt_tray) identity changes.
+
+        The AMS change-hash in _handle_ams_data is built only from AMS units, so
+        an external-spool-only filament swap would otherwise never re-run the
+        inventory reconciliation that unlinks a stale ams_id=255 assignment
+        (#2575). The reconciliation reads vt_tray from live status itself, so we
+        just need to re-fire the callback with the current merged AMS data.
+        """
+        import hashlib
+
+        vt_tray = self.state.raw_data.get("vt_tray")
+        if not isinstance(vt_tray, list):
+            return
+        # Identity fields only — deliberately exclude `remain` so a print's
+        # steadily-dropping fill percentage doesn't fire on every MQTT push.
+        fp_parts = [
+            f"{vt.get('id')}:{vt.get('tray_type')}:{vt.get('tray_color')}:"
+            f"{vt.get('tag_uid')}:{vt.get('tray_uuid')}:{vt.get('tray_info_idx')}"
+            for vt in vt_tray
+            if isinstance(vt, dict)
+        ]
+        vt_hash = hashlib.md5(":".join(fp_parts).encode(), usedforsecurity=False).hexdigest()
+        if vt_hash == self._previous_vt_tray_hash:
+            return
+        self._previous_vt_tray_hash = vt_hash
+        if self.on_ams_change:
+            logger.debug(
+                "[%s] External spool (vt_tray) changed, triggering sync callback",
+                self.serial_number,
+            )
+            self.on_ams_change(self.state.raw_data.get("ams") or [])
+
     def _handle_ams_data(self, ams_data):
         """Handle AMS data changes for Spoolman integration.
 

+ 92 - 0
backend/tests/unit/services/test_external_spool_change.py

@@ -0,0 +1,92 @@
+"""External-spool (vt_tray) change detection (#2575).
+
+The AMS change-hash in ``_handle_ams_data`` is built only from AMS units, so a
+filament swap on the external spool alone (e.g. generic TPU -> generic ABS on
+the printer) used to never re-trigger ``on_ams_change``. That left a stale
+inventory assignment on the ``ams_id=255`` slot: Bambuddy kept showing the old
+filament after the physical type had changed.
+
+These tests drive full MQTT messages through ``_process_message`` and assert the
+callback fires exactly when the external spool's *identity* changes — and not on
+every push (e.g. a steadily-dropping ``remain`` percentage during a print).
+"""
+
+import pytest
+
+from backend.app.services.bambu_mqtt import BambuMQTTClient
+
+
+def _ext_spool_msg(tray_type: str, remain: int = 100, color: str = "000000FF"):
+    """A realistic print message carrying only external-spool (vt_tray) data."""
+    return {
+        "print": {
+            "vt_tray": {
+                "id": "254",
+                "tray_type": tray_type,
+                "tray_color": color,
+                "tray_info_idx": "",
+                "tag_uid": "0000000000000000",
+                "tray_uuid": "00000000000000000000000000000000",
+                "remain": remain,
+            }
+        }
+    }
+
+
+class TestExternalSpoolChangeDetection:
+    @pytest.fixture
+    def mqtt_client(self):
+        return BambuMQTTClient(
+            ip_address="192.168.1.100",
+            serial_number="TEST123",
+            access_code="12345678",
+        )
+
+    def test_type_swap_fires_callback(self, mqtt_client):
+        """Swapping the external filament type re-triggers the sync callback."""
+        calls: list = []
+        mqtt_client.on_ams_change = lambda ams_data: calls.append(ams_data)
+
+        # First observation of the external spool (TPU) — fires once.
+        mqtt_client._process_message(_ext_spool_msg("TPU"))
+        assert len(calls) == 1
+
+        # Physical filament changed to ABS — must fire again so the stale
+        # ams_id=255 assignment gets reconciled.
+        mqtt_client._process_message(_ext_spool_msg("ABS"))
+        assert len(calls) == 2
+
+        # The callback receives the merged AMS list (never None).
+        assert all(isinstance(c, list) for c in calls)
+
+    def test_identical_push_does_not_refire(self, mqtt_client):
+        """Repeated identical vt_tray pushes fire the callback only once."""
+        calls: list = []
+        mqtt_client.on_ams_change = lambda ams_data: calls.append(ams_data)
+
+        mqtt_client._process_message(_ext_spool_msg("ABS"))
+        mqtt_client._process_message(_ext_spool_msg("ABS"))
+        mqtt_client._process_message(_ext_spool_msg("ABS"))
+        assert len(calls) == 1
+
+    def test_remain_only_change_does_not_refire(self, mqtt_client):
+        """A dropping fill percentage must not spam the reconciliation callback."""
+        calls: list = []
+        mqtt_client.on_ams_change = lambda ams_data: calls.append(ams_data)
+
+        mqtt_client._process_message(_ext_spool_msg("PLA", remain=100))
+        assert len(calls) == 1
+        # remain drops during a print — identity unchanged, no refire.
+        mqtt_client._process_message(_ext_spool_msg("PLA", remain=87))
+        mqtt_client._process_message(_ext_spool_msg("PLA", remain=42))
+        assert len(calls) == 1
+
+    def test_reset_to_empty_fires_callback(self, mqtt_client):
+        """Resetting the external spool (empty tray_type) is an identity change."""
+        calls: list = []
+        mqtt_client.on_ams_change = lambda ams_data: calls.append(ams_data)
+
+        mqtt_client._process_message(_ext_spool_msg("TPU"))
+        assert len(calls) == 1
+        mqtt_client._process_message(_ext_spool_msg(""))  # reset / unloaded
+        assert len(calls) == 2

Some files were not shown because too many files changed in this diff