Ver Fonte

Fix external-spool usage charged to an AMS spool (#3166)

Print commands carry the external spool as -1 in the flat ams_mapping
(the firmware rejects 254/255 there) and the real target only in
ams_mapping2. We captured only the flat list, so external-spool prints
looked unmapped and the usage tracker's position-based fallback
charged them to the first loaded AMS tray.

- Resolve external spools from ams_mapping2 when capturing a
  project_file (dual-nozzle keeps 254/255, single-nozzle -> 254)
- Tracker: an explicit -1 no longer falls back to a positional tray;
  a mapping naming no tray for any used slot defers to tray_now
- Keep the #1822 H2S tray_now override working with resolved mappings
maziggy há 1 dia atrás
pai
commit
37502719b7

+ 1 - 0
CHANGELOG.md

@@ -39,6 +39,7 @@ All notable changes to Bambuddy will be documented in this file.
 - **The frontend build no longer warns about `path` and `crypto` being externalized for the STEP previewer (#2976)** — `occt-import-js`, the Emscripten build behind STEP previews, requires both modules, but only inside its `ENVIRONMENT_IS_NODE` branches; in the browser it loads its `.wasm` from the URL the preview worker passes and draws randomness from `crypto.getRandomValues`. Vite still externalized both and printed two warnings on every build. `vite.config.ts` now drops exactly those two warnings for that one package through `build.rolldownOptions.onLog`, so an externalization anywhere else, or of any other module, still shows.
 
 ### Fixed
+- **Filament printed from the external spool was deducted from an AMS spool (#3166)** — The firmware doesn't accept the external spool in a print command's `ams_mapping` list. So BambuStudio and Bambuddy's own dispatch both write it there as -1, the same value as an unused slot, and put the real target in `ams_mapping2`. Bambuddy read only the first list, so an external-spool print looked unmapped, and usage tracking fell back to guessing by position: the first loaded AMS slot. The reporter's H2D charged every external-spool print that way, including 225 g of ABS taken from the PLA spool in AMS slot 1; the same was reported on H2S and P2S. The external spool is now read from `ams_mapping2` (left or right on dual-nozzle printers). A slot the mapping marks as unused is no longer given a tray by position. When a mapping names no tray at all, the tray the printer reports decides instead. The last change also stops an unused slot from taking over the external spool that another slot really used and dropping that slot's weight (#2880).
 - **Upgrading from Bambuddy 0.2.4.0 or older no longer crashes at startup** — Since 1.2.5.4, startup first converts old failure-reason labels to the new vocabulary (#2974), in both the archives and the print log. A database from before the print log had a failure-reason column (#1378) reached that conversion before the migration that adds the column, and Bambuddy stopped with `no such column: failure_reason` instead of starting. The conversion now skips a table that doesn't have the column yet; it can't hold old labels anyway. On SQLite it also rebuilds the archive search index before converting archives, because archives created before that index existed were never added to it, and updating one of them failed with `database disk image is malformed`. It only rebuilds when there is something to convert, and it skips both tables entirely when nothing needs converting, so a normal startup does less work than before.
 - **A connection check that overruns in the support bundle now says which step hung, and keeps what finished (#3164)** — The support bundle gives each printer's connection diagnostic 15 seconds and used to discard the whole result when it overran, recording only `timed_out`. A 14-printer farm's bundle came back with that marker on every printer and nothing else, so it could not show which check was slow or whether the printers were reachable at all. The diagnostic now records each check as it finishes and the step it is on; a timed-out entry carries `stalled_in` (the step that was still running), `elapsed_s`, and the checks that completed before it.
 - **PDF previews in the File Manager failed on any browser older than Chrome 145 or Firefox 144 (#2976)** — Every PDF showed "This file cannot be previewed." pdf.js 6 calls `Map.prototype.getOrInsertComputed` and other recent APIs directly, and the preview loaded its standard build, which assumes them. It now loads pdf.js's legacy build, which bundles polyfills for both the page and the worker. The worker is also bundled through Vite (`?worker&url`) instead of being copied as-is: the copied file kept a class static block that Safari 16.0-16.3 cannot parse, and `build.target` now lowers it like the rest of the app. The Safari 16 baseline check missed that file because it only scanned `.js` output; it now scans `.mjs` too. Checked in a real Chromium without the API, under the app's own Content-Security-Policy: a two-page text PDF renders, pages, and reopens.

+ 52 - 6
backend/app/services/bambu_mqtt.py

@@ -179,6 +179,40 @@ def a2l_lite_wire_ids(ams_id: int, tray_id: int) -> tuple[int, int, int] | None:
     )
 
 
+def resolve_external_spools_in_mapping(ams_mapping: object, ams_mapping2: object, is_dual_nozzle: bool) -> object:
+    """Put the external spool back into a captured flat ``ams_mapping`` (#3166).
+
+    The firmware rejects 254/255 in the flat list, so BambuStudio and our own
+    dispatch both write an external spool there as -1 -- the same value as a
+    slot that isn't fed at all -- and carry the real target only in
+    ``ams_mapping2`` as ``{ams_id: 254|255, slot_id: 0}``. Capturing the flat
+    list alone turns "printed from the external spool" into "unmapped", and
+    usage then lands on whatever tray the fallback guesses.
+
+    Each -1 whose ``ams_mapping2`` entry names an external spool becomes the
+    global tray the rest of Bambuddy uses: the ams_id itself on dual-nozzle
+    printers (254 = left/deputy, 255 = right/main), 254 on single-nozzle
+    printers, which have one external spool that the wire always calls 255.
+    ``{255, 255}`` is the unmapped marker and stays -1. Every other entry is
+    left exactly as captured. Without a usable ``ams_mapping2`` the input is
+    returned unchanged.
+    """
+    if not isinstance(ams_mapping, list) or not isinstance(ams_mapping2, list):
+        return ams_mapping
+    if len(ams_mapping2) != len(ams_mapping):
+        return ams_mapping
+    resolved = list(ams_mapping)
+    for i, (flat, detail) in enumerate(zip(ams_mapping, ams_mapping2, strict=True)):
+        if flat != -1 or not isinstance(detail, dict):
+            continue
+        ams_id = detail.get("ams_id")
+        slot_id = detail.get("slot_id")
+        if ams_id not in (254, 255) or slot_id == 255:
+            continue
+        resolved[i] = ams_id if is_dual_nozzle else 254
+    return resolved
+
+
 def apply_tray_exist_bits(
     units: list,
     tray_exist_bits_str: str | int | None,
@@ -1988,7 +2022,7 @@ class BambuMQTTClient:
                 self.state.current_project_url = url
                 self.state.last_project_url = url
             if "ams_mapping" in print_data:
-                self._captured_ams_mapping = print_data["ams_mapping"]
+                self._captured_ams_mapping = self._resolve_captured_mapping(print_data)
                 logger.info(
                     "[%s] Captured ams_mapping from print command: %s",
                     self.serial_number,
@@ -2068,13 +2102,24 @@ class BambuMQTTClient:
         # already captured this print's mapping that copy is the slicer's own,
         # and the echo can arrive without the field at all.
         if self._captured_ams_mapping is None and isinstance(print_data.get("ams_mapping"), list):
-            self._captured_ams_mapping = print_data["ams_mapping"]
+            self._captured_ams_mapping = self._resolve_captured_mapping(print_data)
             logger.info(
                 "[%s] Captured ams_mapping from print response: %s",
                 self.serial_number,
                 self._captured_ams_mapping,
             )
 
+    def _resolve_captured_mapping(self, print_data: dict) -> object:
+        """The ``ams_mapping`` of a project_file, with external spools resolved
+        from its ``ams_mapping2`` (#3166)."""
+        from backend.app.utils.printer_models import is_dual_nozzle_model
+
+        return resolve_external_spools_in_mapping(
+            print_data.get("ams_mapping"),
+            print_data.get("ams_mapping2"),
+            self._is_dual_nozzle or is_dual_nozzle_model(self.model),
+        )
+
     @staticmethod
     def _project_file_key(print_data: dict) -> str:
         """Identity of a project_file dispatch, for telling ours from a slicer's.
@@ -3185,15 +3230,16 @@ class BambuMQTTClient:
                     # slot (typically 0) when the active feed is actually the
                     # external spool. X1C / P1S / A1 correctly report 254 in
                     # that case; H2S does not. When the slicer-captured
-                    # ams_mapping is all-external (every entry == -1), the
-                    # print can only be feeding from the external spool, so
-                    # promote tray_now to 254. Mixed (e.g. [5, -1]) and
+                    # ams_mapping is all-external (every entry is 254, or -1
+                    # when the command carried no ams_mapping2 to resolve it
+                    # from, #3166), the print can only be feeding from the
+                    # external spool, so promote tray_now to 254. Mixed (e.g. [5, 254]) and
                     # AMS-only mappings are NOT overridden — there's no
                     # evidence the firmware misreports in those cases. Prints
                     # started without a captured mapping (printer-screen start,
                     # or before Bambuddy connected) fall through unchanged.
                     captured = self._captured_ams_mapping
-                    if captured and all(s == -1 for s in captured):
+                    if captured and all(s in (-1, 254, 255) for s in captured):
                         if self.state.tray_now != 254:
                             logger.debug(
                                 f"[{self.serial_number}] tray_now external-spool override (#1822): "

+ 30 - 0
backend/app/services/usage_tracker.py

@@ -1507,6 +1507,24 @@ async def _track_from_3mf(
         mapping_source or "none",
     )
 
+    # A mapping that names no tray for any slot the print used carries no
+    # evidence at all -- typically a flat ams_mapping whose external spool is
+    # -1 and came without an ams_mapping2 to resolve it (#3166). Drop it so the
+    # tray_now evidence below decides, rather than leaving every slot unfed.
+    if slot_to_tray:
+        used_slots = [u.get("slot_id", 0) for u in filament_usage if u.get("used_g", 0) > 0]
+        if used_slots and all(
+            not (0 < s <= len(slot_to_tray) and isinstance(slot_to_tray[s - 1], int) and slot_to_tray[s - 1] >= 0)
+            for s in used_slots
+        ):
+            logger.info(
+                "[UsageTracker] 3MF: mapping %s names no tray for used slots %s — ignoring it",
+                slot_to_tray,
+                used_slots,
+            )
+            slot_to_tray = None
+            mapping_source = None
+
     # 5. For single-filament non-queue prints, use tray_now from printer state
     #    Priority: tray_change_log (multi-tray split) > tray_now_at_start > current tray_now
     #              > last_loaded_tray > vt_tray check
@@ -1779,6 +1797,18 @@ async def _track_from_3mf(
                 mapped = slot_to_tray[slot_id - 1]
                 if isinstance(mapped, int) and mapped >= 0:
                     global_tray_id = mapped
+                elif mapped == -1:
+                    # The mapping says this slot isn't fed from any tray. The
+                    # position-based guess below would hand it the Nth loaded
+                    # tray anyway -- an AMS spool that never moved (#3166), or
+                    # the tray another slot really used, which that slot then
+                    # finds already handled and drops (#2880).
+                    logger.info(
+                        "[UsageTracker] 3MF: slot_id=%d is unmapped (-1) — nothing charged (used_g=%.1f)",
+                        slot_id,
+                        used_g,
+                    )
+                    continue
             # Position-based default: sort available tray IDs so external spools (254/255)
             # naturally follow standard AMS trays, matching slicer slot numbering.
             #

+ 12 - 3
backend/tests/unit/services/test_bambu_mqtt.py

@@ -7039,11 +7039,12 @@ class TestTrayNowH2SExternalSpoolOverride:
     instead of 254 when the active feed is the external spool.
 
     Bambuddy detects the all-external case via the slicer-captured
-    ams_mapping (every entry == -1) and promotes tray_now to 254 so the
-    UI active-tray highlight matches the real feed.
+    ams_mapping (every entry 254, or -1 when the command carried no
+    ams_mapping2 to resolve it from, #3166) and promotes tray_now to 254 so
+    the UI active-tray highlight matches the real feed.
 
     The override is intentionally narrow:
-      * only fires when ams_mapping is captured AND every entry is -1
+      * only fires when ams_mapping is captured AND every entry is external
       * does not touch mixed prints ([5, -1]) or AMS-only prints ([5])
       * does not fire when no ams_mapping is captured (printer-screen start)
     """
@@ -7072,6 +7073,14 @@ class TestTrayNowH2SExternalSpoolOverride:
         mqtt_client._process_message(_ams_payload(0))
         assert mqtt_client.state.tray_now == 254
 
+    def test_resolved_external_mapping_promotes(self, mqtt_client):
+        """Since #3166 the capture resolves the external spool from
+        ams_mapping2, so a single-nozzle all-external print arrives as [254]
+        rather than [-1]. The override must still fire."""
+        mqtt_client._captured_ams_mapping = [254]
+        mqtt_client._process_message(_ams_payload(0))
+        assert mqtt_client.state.tray_now == 254
+
     def test_ams_only_mapping_does_not_override(self, mqtt_client):
         """ams_mapping=[5] (AMS slot 5 only) -> firmware value trusted as-is.
         Without the all-external guard, this would falsely override real

+ 230 - 0
backend/tests/unit/test_external_spool_usage_3166.py

@@ -0,0 +1,230 @@
+"""External-spool usage charged to an AMS spool (#3166).
+
+The firmware rejects 254/255 in a print command's flat ``ams_mapping``, so
+BambuStudio and our own dispatch both write an external spool there as -1 and
+put the real target in ``ams_mapping2``. The MQTT client captured only the flat
+list, so a print fed from the external spool reached the usage tracker as
+``[-1]`` -- and the tracker's position-based fallback then charged it to the
+first loaded AMS tray. The reporter's H2D logged this for every external-spool
+print: 225 g of ABS from the right external spool deducted from the PLA spool
+in AMS slot 1.
+
+The command payloads below are verbatim from that reporter's support bundle
+(H2D, firmware 01.03.00.00, one AMS 2 Pro).
+"""
+
+from datetime import datetime, timezone
+from types import SimpleNamespace
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+
+from backend.app.models.archive import PrintArchive
+from backend.app.models.spool import Spool
+from backend.app.services.bambu_mqtt import BambuMQTTClient, resolve_external_spools_in_mapping
+from backend.app.services.usage_tracker import _track_from_3mf
+
+pytestmark = pytest.mark.unit
+
+
+def project_file(ams_mapping, ams_mapping2=None) -> dict:
+    print_data = {
+        "sequence_id": "20000",
+        "command": "project_file",
+        "param": "Metadata/plate_1.gcode",
+        "url": "ftp://Small_Spool_Adapter_(PLA)_A_Side.3mf",
+        "ams_mapping": ams_mapping,
+    }
+    if ams_mapping2 is not None:
+        print_data["ams_mapping2"] = ams_mapping2
+    return {"print": print_data}
+
+
+EXT_RIGHT = {"ams_id": 255, "slot_id": 0}
+EXT_LEFT = {"ams_id": 254, "slot_id": 0}
+UNMAPPED = {"ams_id": 255, "slot_id": 255}
+
+
+class TestResolvingTheExternalSpool:
+    def test_dual_nozzle_right_external(self):
+        assert resolve_external_spools_in_mapping([-1], [EXT_RIGHT], is_dual_nozzle=True) == [255]
+
+    def test_dual_nozzle_left_external(self):
+        assert resolve_external_spools_in_mapping([-1], [EXT_LEFT], is_dual_nozzle=True) == [254]
+
+    def test_single_nozzle_external_is_254_whatever_the_wire_says(self):
+        # BambuStudio always sends ams_id 255 for the one external spool of a
+        # single-nozzle printer; Bambuddy knows that spool as global tray 254.
+        assert resolve_external_spools_in_mapping([-1], [EXT_RIGHT], is_dual_nozzle=False) == [254]
+
+    def test_mixed_mapping_only_the_external_entry_changes(self):
+        mapping2 = [
+            {"ams_id": 0, "slot_id": 0},
+            EXT_RIGHT,
+            {"ams_id": 0, "slot_id": 2},
+            {"ams_id": 0, "slot_id": 3},
+        ]
+        assert resolve_external_spools_in_mapping([0, -1, 2, 3], mapping2, is_dual_nozzle=True) == [0, 255, 2, 3]
+
+    def test_an_unmapped_slot_stays_unmapped(self):
+        assert resolve_external_spools_in_mapping([0, -1], [{"ams_id": 0, "slot_id": 0}, UNMAPPED], True) == [0, -1]
+
+    @pytest.mark.parametrize("mapping2", [None, "junk", [EXT_RIGHT, EXT_RIGHT]])
+    def test_without_a_usable_mapping2_the_capture_is_untouched(self, mapping2):
+        assert resolve_external_spools_in_mapping([-1], mapping2, is_dual_nozzle=True) == [-1]
+
+    def test_non_list_mapping_is_returned_as_is(self):
+        assert resolve_external_spools_in_mapping(None, [EXT_RIGHT], is_dual_nozzle=True) is None
+
+
+class TestTheCapturedMapping:
+    def test_h2d_command_captures_the_right_external_spool(self):
+        client = BambuMQTTClient(ip_address="10.0.0.7", serial_number="H2D3166", access_code="12345678", model="H2D")
+        client._handle_request_message(project_file([-1], [EXT_RIGHT]))
+        assert client._captured_ams_mapping == [255]
+
+    def test_h2d_mixed_command(self):
+        client = BambuMQTTClient(ip_address="10.0.0.7", serial_number="H2D3166", access_code="12345678", model="H2D")
+        mapping2 = [
+            {"ams_id": 0, "slot_id": 0},
+            EXT_RIGHT,
+            {"ams_id": 0, "slot_id": 2},
+            {"ams_id": 0, "slot_id": 3},
+        ]
+        client._handle_request_message(project_file([0, -1, 2, 3], mapping2))
+        assert client._captured_ams_mapping == [0, 255, 2, 3]
+
+    def test_p2s_command_captures_tray_254(self):
+        client = BambuMQTTClient(ip_address="10.0.0.7", serial_number="P2S3166", access_code="12345678", model="P2S")
+        client._handle_request_message(project_file([-1], [EXT_RIGHT]))
+        assert client._captured_ams_mapping == [254]
+
+    def test_command_without_mapping2_is_captured_as_before(self):
+        client = BambuMQTTClient(ip_address="10.0.0.7", serial_number="X1C3166", access_code="12345678", model="X1C")
+        client._handle_request_message(project_file([-1]))
+        assert client._captured_ams_mapping == [-1]
+
+
+# --- The usage tracker ---------------------------------------------------------
+
+
+def _spool(spool_id: int):
+    spool = MagicMock()
+    spool.id = spool_id
+    spool.label_weight = 1000
+    spool.weight_used = 0
+    spool.cost_per_kg = None
+    spool.material = "ABS"
+    spool.rgba = None
+    return spool
+
+
+def _db(spools: dict[int, MagicMock]):
+    """Answer the tracker's archive and spool lookups by what they select."""
+    archive = MagicMock()
+    archive.id = 142
+    archive.file_path = "archives/142/test.3mf"
+    archive.extra_data = None
+    archive.plate_id = None
+    selected: list = []
+
+    async def execute(stmt, *args, **kwargs):
+        entity = stmt.column_descriptions[0].get("entity")
+        result = MagicMock()
+        value = None
+        if entity is PrintArchive:
+            value = archive
+        elif entity is Spool:
+            spool_id = stmt.whereclause.right.value
+            value = spools.get(spool_id)
+            selected.append(spool_id)
+        result.scalar_one_or_none.return_value = value
+        result.scalars.return_value.first.return_value = None
+        result.scalar.return_value = None
+        return result
+
+    db = AsyncMock()
+    db.execute = execute
+    db.add = MagicMock()
+    return db
+
+
+# The reporter's printer: PLA in AMS slot 1 (spool 20), ABS on the right
+# external spool (spool 4). The AMS slot is loaded, so the old position-based
+# fallback had a tray to land on.
+ASSIGNED = {(0, 0): 20, (255, 1): 4}
+RAW_DATA = {
+    "ams": [{"id": "0", "tray": [{"id": "0", "tray_type": "PLA"}]}],
+    "vt_tray": [{"id": "255", "tray_type": "ABS"}],
+}
+
+
+async def _track(ams_mapping, filament_usage, tray_now_at_start=-1, tray_now=255):
+    charged: list[tuple[int, int]] = []
+
+    async def resolve(printer_id, ams_id, tray_id, **kwargs):
+        charged.append((ams_id, tray_id))
+        return ASSIGNED.get((ams_id, tray_id))
+
+    printer_manager = MagicMock()
+    printer_manager.get_status.return_value = SimpleNamespace(
+        progress=100,
+        layer_num=50,
+        tray_now=tray_now,
+        last_loaded_tray=-1,
+        tray_change_log=[],
+        raw_data=RAW_DATA,
+    )
+    with (
+        patch("backend.app.core.config.settings") as mock_settings,
+        patch("backend.app.utils.threemf_tools.extract_filament_usage_from_3mf", return_value=filament_usage),
+        patch("backend.app.services.usage_tracker._resolve_spool_id_for_tray", side_effect=resolve),
+    ):
+        mock_path = MagicMock()
+        mock_path.exists.return_value = True
+        mock_settings.base_dir = MagicMock()
+        mock_settings.base_dir.__truediv__ = MagicMock(return_value=mock_path)
+        results = await _track_from_3mf(
+            printer_id=1,
+            archive_id=142,
+            status="completed",
+            print_name="Daft_Punk_Helmet_Back",
+            handled_trays=set(),
+            printer_manager=printer_manager,
+            db=_db({20: _spool(20), 4: _spool(4)}),
+            ams_mapping=ams_mapping,
+            tray_now_at_start=tray_now_at_start,
+            print_started_at=datetime.now(timezone.utc),
+        )
+    return results, charged
+
+
+ABS_ONLY = [{"slot_id": 1, "used_g": 225.14, "type": "ABS", "color": "#000000"}]
+
+
+class TestTheTrackerCharges:
+    @pytest.mark.asyncio
+    async def test_the_resolved_mapping_charges_the_external_spool(self):
+        results, charged = await _track([255], ABS_ONLY)
+        assert [r["spool_id"] for r in results] == [4]
+        assert charged == [(255, 1)]
+
+    @pytest.mark.asyncio
+    async def test_an_unmapped_slot_does_not_fall_back_to_an_ams_tray(self):
+        # A two-filament print where the mapping leaves slot 2 unfed. The old
+        # fallback handed slot 2 the second loaded tray instead.
+        usage = [
+            {"slot_id": 1, "used_g": 10.0, "type": "PLA", "color": "#000000"},
+            {"slot_id": 2, "used_g": 5.0, "type": "ABS", "color": "#000000"},
+        ]
+        results, charged = await _track([0, -1], usage)
+        assert [r["spool_id"] for r in results] == [20]
+        assert charged == [(0, 0)]
+
+    @pytest.mark.asyncio
+    async def test_an_all_unmapped_capture_defers_to_tray_now(self):
+        # A command without ams_mapping2 still captures [-1]. That names no
+        # tray, so the printer's own tray_now decides -- not AMS slot 1.
+        results, charged = await _track([-1], ABS_ONLY, tray_now_at_start=255)
+        assert (0, 0) not in charged
+        assert [r["spool_id"] for r in results] == [4]