Преглед на файлове

fix(spoolman): concurrent copies of an archive are charged, unfed slots charge nothing

maziggy преди 6 дни
родител
ревизия
fb69a665d8

+ 3 - 0
CHANGELOG.md

@@ -164,6 +164,9 @@ 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
+- **Spoolman usage** —
+  - Two copies of the same archive printing at once on two printers are both charged to Spoolman. The second print's start failed on the shared archive and that print was never deducted.
+  - A filament slot the print does not feed is no longer charged to the external spool, or to whichever AMS tray sits at that position, as with the built-in inventory.
 - **Spoolman slots** —
   - Assigning or linking a Spoolman spool to an AMS slot is now checked against what the printer reports back, as with the built-in inventory. A printer that ignored the new filament showed no "not confirmed" notice, and later usage went to the wrong spool.
   - Configure AMS Slot opens with the slicer preset set on the Spoolman spool when no per-printer-model preset applies, and the slot's K profile is matched under it. Before, the dialog opened with no preset and the K profile was matched against the tray's own filament.

+ 22 - 19
backend/app/services/spoolman_tracking.py

@@ -152,15 +152,12 @@ async def _get_printer_serial(printer_id: int) -> str:
 def _resolve_global_tray_id(slot_id: int, slot_to_tray: list | None, ams_trays: dict | None = None) -> int:
     """Map a 1-based slot_id to a global_tray_id using optional custom mapping.
 
-    Custom mapping: slot_to_tray[slot_id - 1] is used when >= 0.
-    A value of -1 in the custom mapping means the slicer routed this slot to
-    the external spool. BambuStudio converts virtual tray IDs (254/255) to -1
-    in the flat ams_mapping array before sending to the printer — see
-    start_print() in bambu_mqtt.py which documents this convention. We mirror
-    it here: when -1 is seen, look up the external spool's actual
-    global_tray_id (254/255) in ams_trays rather than falling through to the
-    position-based default (which would map slot_id=1 to the first AMS tray
-    and credit an unrelated spool — see #1276, regression of #853).
+    Custom mapping: slot_to_tray[slot_id - 1] is used when >= 0. A negative
+    value means the slot feeds nothing and answers -1, which no tray has: the
+    flat ams_mapping writes an external spool as -1 on the wire, but the
+    captured mapping puts its 254/255 back from ams_mapping2 (#3166), so a -1
+    left in it is unmapped. Never the position-based default for such a slot,
+    which would credit an unrelated spool (#1276, regression of #853).
     Position-based default: uses sorted ams_trays keys so external spools (ID 254/255)
     naturally follow standard AMS trays, matching the slicer's slot numbering.
     Final fallback: slot_id - 1 (legacy, works for pure AMS without external spools).
@@ -169,14 +166,11 @@ def _resolve_global_tray_id(slot_id: int, slot_to_tray: list | None, ams_trays:
         mapped_tray = slot_to_tray[slot_id - 1]
         if mapped_tray >= 0:
             return mapped_tray
-        if mapped_tray == -1 and ams_trays:
-            # -1 means external spool. 254 = VIRTUAL_TRAY_DEPUTY_ID (main on
-            # single-nozzle, left/deputy on H2D dual-nozzle); 255 =
-            # VIRTUAL_TRAY_MAIN_ID. Prefer 254 when both exist since that's
-            # what single-nozzle printers report via tray_now.
-            for ext_id in (254, 255):
-                if ext_id in ams_trays:
-                    return ext_id
+        # A negative value is a slot the print does not feed: the captured
+        # mapping already names an external spool 254/255 (#3166), and the
+        # built-in tracker charges nothing for it either. No tray has this id,
+        # so the slot is skipped rather than guessed by position.
+        return -1
     # Position-based default: sort available tray IDs so external spools (254/255)
     # come after standard AMS trays, matching the slicer's slot assignment order.
     if ams_trays:
@@ -496,12 +490,16 @@ async def store_print_data(
         # Resolve the queue item once — used both for the plate-scoped 3MF parsing
         # fallback (#1697: multi-plate file dispatched for one plate must only count
         # that plate's filament) and for the ams_mapping fallback below.
+        # This printer's: a batch prints one archive as several queue items,
+        # on several printers at once.
         queue_result = await db.execute(
             select(PrintQueueItem)
             .where(PrintQueueItem.archive_id == archive_id)
+            .where(PrintQueueItem.printer_id == printer_id)
             .where(PrintQueueItem.status == "printing")
+            .order_by(PrintQueueItem.id.desc())
         )
-        queue_item = queue_result.scalar_one_or_none()
+        queue_item = queue_result.scalars().first()
         # Caller-supplied plate_id wins (direct-Print path); fall back to the queue
         # item's plate_id (queue dispatch path).
         effective_plate_id = (
@@ -1387,7 +1385,12 @@ async def report_usage(printer_id: int, archive_id: int):
 
         client = await _get_spoolman_client_with_fallback()
         if not client:
-            logger.warning("[SPOOLMAN] Not reachable for usage reporting")
+            logger.warning(
+                "[SPOOLMAN] Not reachable for usage reporting (printer=%s, archive=%s): "
+                "this print's usage is not sent, correct the spools in Spoolman by hand",
+                printer_id,
+                archive_id,
+            )
             return
 
         # Consult the live printer state for the tray-change log written by

+ 62 - 0
backend/tests/integration/test_spoolman_tracking_shared_archive.py

@@ -0,0 +1,62 @@
+"""Spoolman tracking for an archive printing on several printers at once.
+
+A batch dispatches one archive as several queue items. The print-start
+capture looked the queue item up by archive alone and insisted on one row, so
+with two copies printing it raised and the print was never charged.
+"""
+
+from types import SimpleNamespace
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+
+from backend.app.models.active_print_spoolman import ActivePrintSpoolman  # registers its table
+from backend.app.models.print_queue import PrintQueueItem
+from backend.app.services.spoolman_tracking import store_print_data
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_each_printer_reads_its_own_queue_item(db_session, printer_factory, archive_factory):
+    from sqlalchemy import select
+
+    printer_a = await printer_factory()
+    printer_b = await printer_factory()
+    archive = await archive_factory(printer_a.id, status="printing", with_run=False)
+    db_session.add(PrintQueueItem(printer_id=printer_a.id, archive_id=archive.id, plate_id=2, status="printing"))
+    db_session.add(PrintQueueItem(printer_id=printer_b.id, archive_id=archive.id, plate_id=3, status="printing"))
+    await db_session.commit()
+    plates: list = []
+
+    printer_manager = MagicMock()
+    printer_manager.get_status.return_value = SimpleNamespace(
+        raw_data={"ams": [{"id": 0, "tray": [{"id": 0, "tray_type": "PLA"}]}]}
+    )
+    mock_settings = MagicMock()
+    mock_path = MagicMock()
+    mock_path.exists.return_value = True
+    mock_settings.base_dir.__truediv__.return_value = mock_path
+
+    def _usage(path, plate_id=None):
+        plates.append(plate_id)
+        return [{"slot_id": 1, "used_g": 5.0, "type": "PLA", "color": "#FF0000"}]
+
+    with (
+        patch("backend.app.services.spoolman_tracking.app_settings", mock_settings),
+        patch("backend.app.api.routes.settings.get_setting", AsyncMock(return_value="true")),
+        patch("backend.app.utils.threemf_tools.extract_filament_usage_from_3mf", side_effect=_usage),
+        patch("backend.app.utils.threemf_tools.extract_layer_filament_usage_from_3mf", return_value=None),
+        patch("backend.app.utils.threemf_tools.extract_filament_properties_from_3mf", return_value={}),
+    ):
+        await store_print_data(
+            printer_id=printer_a.id,
+            archive_id=archive.id,
+            file_path="archives/test.3mf",
+            db=db_session,
+            printer_manager=printer_manager,
+            ams_mapping=[0],
+        )
+
+    assert plates == [2]
+    rows = (await db_session.execute(select(ActivePrintSpoolman))).scalars().all()
+    assert [r.printer_id for r in rows] == [printer_a.id]

+ 19 - 31
backend/tests/unit/services/test_spoolman_tracking.py

@@ -80,9 +80,9 @@ class TestResolveGlobalTrayId:
         assert _resolve_global_tray_id(4, mapping) == 0
 
     def test_unmapped_slot(self):
-        """Slot with -1 in mapping uses default."""
+        """Slot with -1 in mapping feeds nothing: no tray, never the default."""
         mapping = [5, -1, 2, 0]
-        assert _resolve_global_tray_id(2, mapping) == 1  # default: slot 2 -> tray 1
+        assert _resolve_global_tray_id(2, mapping, ams_trays={0: {}, 1: {}, 2: {}, 5: {}}) == -1
 
     def test_slot_beyond_mapping(self):
         """Slot beyond mapping length uses default."""
@@ -93,34 +93,22 @@ class TestResolveGlobalTrayId:
         mapping = []
         assert _resolve_global_tray_id(1, mapping) == 0
 
-    def test_minus_one_resolves_to_external_spool_when_present(self):
-        """#1276 (regression of #853): -1 in slot_to_tray is BambuStudio's
-        encoding for "external spool used" — look up the external spool in
-        ams_trays rather than falling through to the position-based default
-        (which would credit an unrelated AMS tray). Reporter ojimpo's H2S
-        had AMS slot 0 occupied with PLA and ran a TPU external-spool print;
-        the bug credited the TPU usage to the PLA spool.
-        """
-        # Single external spool (most common: H2S/X1C/P1S + external)
-        assert _resolve_global_tray_id(1, [-1], ams_trays={254: {}}) == 254
-        # AMS occupied with material AND external in use — fix prevents
-        # crediting AMS slot 0 (the actual bug from #1276)
-        assert _resolve_global_tray_id(1, [-1], ams_trays={0: {}, 1: {}, 2: {}, 3: {}, 254: {}}) == 254
-        # H2D-style deputy nozzle at 255
-        assert _resolve_global_tray_id(1, [-1], ams_trays={0: {}, 255: {}}) == 255
-        # Both external slots present (multi-nozzle) — prefer 254 (main on
-        # single-nozzle, deputy on H2D — matches tray_now reporting)
-        assert _resolve_global_tray_id(1, [-1], ams_trays={254: {}, 255: {}}) == 254
-
-    def test_minus_one_falls_through_when_no_external_in_ams_trays(self):
-        """If -1 is seen but ams_trays has no external spool (254/255),
-        fall through to position-based default (legacy behavior preserved
-        for callers that don't pass ams_trays or pre-fix call sites).
-        """
-        # ams_trays without external — fall through to legacy behavior
-        assert _resolve_global_tray_id(1, [-1], ams_trays={0: {}, 1: {}}) == 0
-        # No ams_trays passed at all — legacy fallback
-        assert _resolve_global_tray_id(1, [-1]) == 0
+    def test_the_external_spool_comes_resolved_in_the_captured_mapping(self):
+        """#1276 (regression of #853): an external-spool print must not be
+        credited to an AMS tray. The wire writes the external spool as -1, but
+        the captured mapping puts its 254/255 back from ams_mapping2 (#3166),
+        so it arrives as a tray id like any other."""
+        assert _resolve_global_tray_id(1, [254], ams_trays={0: {}, 1: {}, 2: {}, 3: {}, 254: {}}) == 254
+        assert _resolve_global_tray_id(1, [255], ams_trays={0: {}, 255: {}}) == 255
+
+    def test_minus_one_is_no_tray_even_with_an_external_spool_loaded(self):
+        """A -1 left in the captured mapping is a slot the print does not feed
+        (ams_mapping2 named no external spool for it). The built-in tracker
+        charges nothing for it; neither the external spool nor the slot's
+        position is guessed."""
+        assert _resolve_global_tray_id(1, [-1], ams_trays={0: {}, 1: {}, 254: {}}) == -1
+        assert _resolve_global_tray_id(1, [-1], ams_trays={0: {}, 1: {}}) == -1
+        assert _resolve_global_tray_id(1, [-1]) == -1
 
 
 class TestFallbackTagHelpers:
@@ -219,7 +207,7 @@ class TestStorePrintData:
         # row before inserting the new one. Two execute calls in that order.
         queue_item = SimpleNamespace(ams_mapping=json.dumps([2, -1, -1, -1]), plate_id=None)
         queue_result = MagicMock()
-        queue_result.scalar_one_or_none.return_value = queue_item
+        queue_result.scalars.return_value.first.return_value = queue_item
         delete_result = MagicMock()
         db.execute = AsyncMock(side_effect=[queue_result, delete_result])
         db.add = MagicMock()

+ 1 - 1
backend/tests/unit/test_spoolman_no3mf_remain_fallback.py

@@ -238,7 +238,7 @@ class TestStorePrintDataNo3mf:
         db = AsyncMock()
         queue_item = SimpleNamespace(ams_mapping=None, plate_id=None)
         queue_result = MagicMock()
-        queue_result.scalar_one_or_none.return_value = queue_item
+        queue_result.scalars.return_value.first.return_value = queue_item
         delete_result = MagicMock()
         db.execute = AsyncMock(side_effect=[queue_result, delete_result])
         db.add = MagicMock()

+ 4 - 4
backend/tests/unit/test_spoolman_tracking.py

@@ -80,9 +80,9 @@ class TestResolveGlobalTrayId:
         assert _resolve_global_tray_id(4, mapping) == 0
 
     def test_unmapped_slot(self):
-        """Slot with -1 in mapping uses default."""
+        """Slot with -1 in mapping feeds nothing: no tray, never the default."""
         mapping = [5, -1, 2, 0]
-        assert _resolve_global_tray_id(2, mapping) == 1  # default: slot 2 -> tray 1
+        assert _resolve_global_tray_id(2, mapping, ams_trays={0: {}, 1: {}, 2: {}, 5: {}}) == -1
 
     def test_slot_beyond_mapping(self):
         """Slot beyond mapping length uses default."""
@@ -190,7 +190,7 @@ class TestStorePrintData:
         # row before inserting the new one. Two execute calls in that order.
         queue_item = SimpleNamespace(ams_mapping=json.dumps([2, -1, -1, -1]), plate_id=None)
         queue_result = MagicMock()
-        queue_result.scalar_one_or_none.return_value = queue_item
+        queue_result.scalars.return_value.first.return_value = queue_item
         delete_result = MagicMock()
         db.execute = AsyncMock(side_effect=[queue_result, delete_result])
         db.add = MagicMock()
@@ -236,7 +236,7 @@ class TestStorePrintData:
         db = AsyncMock()
         queue_item = SimpleNamespace(ams_mapping=None, plate_id=2)
         queue_result = MagicMock()
-        queue_result.scalar_one_or_none.return_value = queue_item
+        queue_result.scalars.return_value.first.return_value = queue_item
         delete_result = MagicMock()
         db.execute = AsyncMock(side_effect=[queue_result, delete_result])
         db.add = MagicMock()