Quellcode durchsuchen

fix(skip-objects): scope the object list to the plate being printed (#2522)

extract_printable_objects_from_3mf() has accepted a plate_number since it was
written and no caller ever passed one, so it took root.find(".//plate") — the
first plate in the file. Passing one would not have helped either: the lookup
was .//plate[@plate_idx='N'], a predicate on an attribute neither Bambu Studio
nor OrcaSlicer writes. The index lives in a <metadata key="index"> child, as
threemf_tools and filament_requirements already read it, so the selector never
matched and fell back to plate 1 regardless.

On an all-plates .gcode.3mf that meant Skip Objects offered the wrong plate's
objects, with that plate's marker positions drawn over the correct plate's
thumbnail (/cover resolves the plate properly via resolve_plate_id, the object
list did not). The reporter printed a one-object plate and was shown the four
copies from another plate of the same file.

Select the plate on its index metadata, and pass resolve_plate_id(state) at all
three call sites so the list and the thumbnail share one resolver. Also stop
peek_plate_index_in_3mf() reporting plate 1 for a multi-plate file: it backs the
running one, so an all-plates upload printing plate 2+ lost its archive entirely.
maziggy vor 1 Monat
Ursprung
Commit
ce31de65c2

Datei-Diff unterdrückt, da er zu groß ist
+ 0 - 0
CHANGELOG.md


+ 8 - 1
backend/app/api/routes/printers.py

@@ -3362,7 +3362,14 @@ async def get_printable_objects(
                 if downloaded and temp_path.exists():
                     with open(temp_path, "rb") as f:
                         data = f.read()
-                    objects, bbox_all = extract_printable_objects_from_3mf(data, include_positions=True)
+                    # Scope to the running plate: an all-plates 3MF lists every
+                    # plate's objects, and offering plate 1's while the printer
+                    # runs plate 2 makes every skip a misfire (#2522).
+                    objects, bbox_all = extract_printable_objects_from_3mf(
+                        data,
+                        plate_number=resolve_plate_id(client.state),
+                        include_positions=True,
+                    )
                     if objects:
                         client.state.printable_objects = objects
                         client.state.printable_objects_bbox_all = bbox_all

+ 32 - 18
backend/app/main.py

@@ -106,6 +106,7 @@ from backend.app.services.printer_manager import (
     parse_plate_id,
     printer_manager,
     printer_state_to_dict,
+    resolve_plate_id,
 )
 from backend.app.services.smart_plug_manager import smart_plug_manager
 from backend.app.services.spool_assignment_notifications import (
@@ -2300,19 +2301,28 @@ def _load_objects_from_archive(archive, printer_id: int, logger) -> None:
     try:
         from backend.app.services.archive import extract_printable_objects_from_3mf
 
+        client = printer_manager.get_client(printer_id)
+        if not client:
+            return
+
         file_path = app_settings.base_dir / archive.file_path
         if file_path.is_file() and str(file_path).endswith(".3mf"):
             with open(file_path, "rb") as f:
                 threemf_data = f.read()
-            # Extract with positions for UI overlay
-            printable_objects, bbox_all = extract_printable_objects_from_3mf(threemf_data, include_positions=True)
+            # Extract with positions for UI overlay, scoped to the plate that
+            # is printing — resolve_plate_id is the same resolver /cover uses,
+            # so the object list can't disagree with the thumbnail it is drawn
+            # over (#2522).
+            printable_objects, bbox_all = extract_printable_objects_from_3mf(
+                threemf_data,
+                plate_number=resolve_plate_id(client.state),
+                include_positions=True,
+            )
             if printable_objects:
-                client = printer_manager.get_client(printer_id)
-                if client:
-                    client.state.printable_objects = printable_objects
-                    client.state.printable_objects_bbox_all = bbox_all
-                    client.state.skipped_objects = []
-                    logger.info("Loaded %s printable objects for printer %s", len(printable_objects), printer_id)
+                client.state.printable_objects = printable_objects
+                client.state.printable_objects_bbox_all = bbox_all
+                client.state.skipped_objects = []
+                logger.info("Loaded %s printable objects for printer %s", len(printable_objects), printer_id)
     except Exception as e:
         logger.debug("Failed to extract printable objects from archive: %s", e)
 
@@ -3351,16 +3361,20 @@ async def on_print_start(printer_id: int, data: dict):
                 try:
                     from backend.app.services.archive import extract_printable_objects_from_3mf
 
-                    with open(temp_path, "rb") as f:
-                        threemf_data = f.read()
-                    # Extract with positions for UI overlay
-                    printable_objects, bbox_all = extract_printable_objects_from_3mf(
-                        threemf_data, include_positions=True
-                    )
-                    if printable_objects:
-                        # Store objects in printer state
-                        client = printer_manager.get_client(printer_id)
-                        if client:
+                    client = printer_manager.get_client(printer_id)
+                    if client:
+                        with open(temp_path, "rb") as f:
+                            threemf_data = f.read()
+                        # Extract with positions for UI overlay, scoped to the
+                        # plate that is printing — an all-plates 3MF carries
+                        # every plate's objects (#2522).
+                        printable_objects, bbox_all = extract_printable_objects_from_3mf(
+                            threemf_data,
+                            plate_number=resolve_plate_id(client.state),
+                            include_positions=True,
+                        )
+                        if printable_objects:
+                            # Store objects in printer state
                             client.state.printable_objects = printable_objects
                             client.state.printable_objects_bbox_all = bbox_all
                             client.state.skipped_objects = []  # Reset skipped objects for new print

+ 45 - 29
backend/app/services/archive.py

@@ -69,13 +69,37 @@ def resolve_display_stem(filename: str) -> str:
     return Path(name).stem
 
 
+def _read_plate_index(plate) -> int | None:
+    """Return the 1-based index of a ``slice_info.config`` ``<plate>`` element, or None.
+
+    Bambu Studio and OrcaSlicer record it as a ``<metadata key="index"
+    value="N"/>`` child — there is no ``plate_idx`` attribute on ``<plate>``
+    itself, so an XPath predicate on one never matches (#2522).
+    """
+    for meta in plate.findall("metadata"):
+        if meta.get("key") == "index":
+            value = meta.get("value")
+            if not value:
+                return None
+            try:
+                return int(value)
+            except ValueError:
+                return None
+    return None
+
+
 def peek_plate_index_in_3mf(file_path: Path) -> int | None:
-    """Return the plate index recorded inside a Bambu 3MF, or None.
+    """Return the plate index a single-plate Bambu 3MF represents, or None.
 
     Reads only ``Metadata/slice_info.config`` to keep this cheap — used by
     the print-start callback to verify that the 3MF we just downloaded over
     FTP actually matches the plate the printer is running (#1204). The full
     ThreeMFParser does much more work and runs later inside ArchiveService.
+
+    An all-plates export carries every plate, so "which plate is this file"
+    has no answer; returning None there keeps the #1204 guard from reading
+    plate 1 out of such a file, declaring a mismatch against the plate that
+    is really running, and discarding a perfectly good 3MF (#2522).
     """
     try:
         with zipfile.ZipFile(file_path, "r") as zf:
@@ -83,20 +107,12 @@ def peek_plate_index_in_3mf(file_path: Path) -> int | None:
                 return None
             content = zf.read("Metadata/slice_info.config").decode()
             root = ET.fromstring(content)
-            plate = root.find(".//plate")
-            if plate is None:
+            plates = root.findall(".//plate")
+            if len(plates) != 1:
                 return None
-            for meta in plate.findall("metadata"):
-                if meta.get("key") == "index":
-                    value = meta.get("value")
-                    if value:
-                        try:
-                            return int(value)
-                        except ValueError:
-                            return None
+            return _read_plate_index(plates[0])
     except Exception:
         return None
-    return None
 
 
 _PLATE_SUFFIX_RE = re.compile(r"^(.*?)(\s*-\s*Plate\s+|_plate_)(\d+)$", re.IGNORECASE)
@@ -615,26 +631,26 @@ def extract_printable_objects_from_3mf(
             content = zf.read("Metadata/slice_info.config").decode()
             root = ET.fromstring(content)
 
-            # Find the correct plate
-            if plate_number:
-                plate = root.find(f".//plate[@plate_idx='{plate_number}']")
-                if plate is None:
-                    plate = root.find(".//plate")
-            else:
-                plate = root.find(".//plate")
+            plates = root.findall(".//plate")
+            if not plates:
+                return printable_objects
 
+            # Pick the plate that is actually printing. An all-plates export
+            # lists every plate, so without this we offered the objects (and
+            # the marker positions) of plate 1 whatever the printer was
+            # running (#2522). Falling back to the first plate keeps the
+            # single-plate export — the common case — working when the caller
+            # has no plate to give us.
+            plate = None
+            if plate_number is not None:
+                plate = next((p for p in plates if _read_plate_index(p) == plate_number), None)
             if plate is None:
-                return printable_objects
+                plate = plates[0]
 
-            # Get actual plate index from metadata (sliced files only have one plate)
-            plate_idx = plate_number or 1
-            for meta in plate.findall("metadata"):
-                if meta.get("key") == "index":
-                    try:
-                        plate_idx = int(meta.get("value", "1"))
-                    except ValueError:
-                        pass  # Use default plate_idx if value is non-numeric
-                    break
+            # Derive plate_idx from the plate we settled on, never from the
+            # requested one: on a fallback they differ, and plate_idx also
+            # selects the plate_N.json the positions come from.
+            plate_idx = _read_plate_index(plate) or 1
 
             # Load position data from plate_N.json if we need positions
             # Build a lookup by name - use list to handle duplicate names

+ 199 - 0
backend/tests/unit/services/test_printable_objects_plate.py

@@ -0,0 +1,199 @@
+"""Skip-object extraction must follow the plate that is actually printing (#2522).
+
+An "all plates" sliced export lists every plate in ``slice_info.config`` and
+ships a ``plate_N.json`` per plate. Bambuddy read the *first* plate whatever
+the printer was running, so a reporter printing plate 2 (one object) was
+offered plate 1's four objects, with plate 1's marker positions drawn over
+plate 2's thumbnail.
+
+Two defects fed that: no call site passed ``plate_number``, and the lookup it
+would have used (``.//plate[@plate_idx='N']``) tested an attribute Bambu and
+Orca never write — the index lives in a ``<metadata key="index">`` child, so
+the selector silently fell back to plate 1 regardless.
+
+The fixture below mirrors the reporter's real file: plate 1 holds four copies
+of ``stand_pillow_01.stl`` (identify_ids 2040/2062/2084/2106), plate 2 holds
+one (2168).
+"""
+
+import json
+import logging
+import zipfile
+from io import BytesIO
+from types import SimpleNamespace
+
+import pytest
+
+from backend.app import main as main_module
+from backend.app.services.archive import (
+    extract_printable_objects_from_3mf,
+    peek_plate_index_in_3mf,
+)
+
+PLATE_1_IDS = [2040, 2062, 2084, 2106]
+PLATE_2_ID = 2168
+
+
+def _plate_xml(index: int, ids: list[int]) -> str:
+    objects = "".join(f'<object identify_id="{i}" name="stand_pillow_01.stl" skipped="false" />' for i in ids)
+    return f'<plate><metadata key="index" value="{index}"/><metadata key="weight" value="10"/>{objects}</plate>'
+
+
+def _plate_json(boxes: list[list[float]]) -> str:
+    """A plate_N.json with one bbox_objects entry per box, plus their union."""
+    return json.dumps(
+        {
+            "bbox_all": [
+                min(b[0] for b in boxes),
+                min(b[1] for b in boxes),
+                max(b[2] for b in boxes),
+                max(b[3] for b in boxes),
+            ],
+            "bbox_objects": [
+                # The ids here are the slicer's own bbox ids, which do NOT equal
+                # identify_id in real files — matching is by name, as before.
+                {"id": 9000 + n, "name": "stand_pillow_01.stl", "bbox": box}
+                for n, box in enumerate(boxes)
+            ],
+        }
+    )
+
+
+# Four copies in a square (plate 1) vs. a single copy elsewhere (plate 2).
+PLATE_1_BOXES = [[0, 0, 10, 10], [90, 0, 100, 10], [0, 90, 10, 100], [90, 90, 100, 100]]
+PLATE_2_BOXES = [[40, 40, 60, 60]]
+
+
+def _multi_plate_3mf() -> bytes:
+    buf = BytesIO()
+    with zipfile.ZipFile(buf, "w") as zf:
+        zf.writestr(
+            "Metadata/slice_info.config",
+            f"<config><header/>{_plate_xml(1, PLATE_1_IDS)}{_plate_xml(2, [PLATE_2_ID])}</config>",
+        )
+        zf.writestr("Metadata/plate_1.json", _plate_json(PLATE_1_BOXES))
+        zf.writestr("Metadata/plate_2.json", _plate_json(PLATE_2_BOXES))
+    return buf.getvalue()
+
+
+def _single_plate_3mf(index: int) -> bytes:
+    """A per-plate export: one <plate>, but its index is the original plate number."""
+    buf = BytesIO()
+    with zipfile.ZipFile(buf, "w") as zf:
+        zf.writestr(
+            "Metadata/slice_info.config",
+            f"<config><header/>{_plate_xml(index, [PLATE_2_ID])}</config>",
+        )
+        zf.writestr(f"Metadata/plate_{index}.json", _plate_json(PLATE_2_BOXES))
+    return buf.getvalue()
+
+
+class TestExtractPrintableObjectsPlateScope:
+    def test_returns_only_the_requested_plates_objects(self):
+        objects, bbox_all = extract_printable_objects_from_3mf(
+            _multi_plate_3mf(), plate_number=2, include_positions=True
+        )
+        assert list(objects) == [PLATE_2_ID]
+        # Positions come from plate_2.json, not plate_1.json.
+        assert objects[PLATE_2_ID]["x"] == 50
+        assert objects[PLATE_2_ID]["y"] == 50
+        assert bbox_all == [40, 40, 60, 60]
+
+    def test_other_plate_of_the_same_file_resolves_independently(self):
+        objects, bbox_all = extract_printable_objects_from_3mf(
+            _multi_plate_3mf(), plate_number=1, include_positions=True
+        )
+        assert sorted(objects) == PLATE_1_IDS
+        assert bbox_all == [0, 0, 100, 100]
+
+    def test_no_plate_given_falls_back_to_the_first(self):
+        objects = extract_printable_objects_from_3mf(_multi_plate_3mf())
+        assert sorted(objects) == PLATE_1_IDS
+
+    def test_unknown_plate_falls_back_without_mixing_plates(self):
+        # Plate 9 doesn't exist. We fall back to the first plate — and must read
+        # ITS plate_1.json, not a plate_9.json that isn't there. Getting this
+        # wrong would return plate 1's objects with no positions at all.
+        objects, bbox_all = extract_printable_objects_from_3mf(
+            _multi_plate_3mf(), plate_number=9, include_positions=True
+        )
+        assert sorted(objects) == PLATE_1_IDS
+        assert bbox_all == [0, 0, 100, 100]
+        assert all(o["x"] is not None for o in objects.values())
+
+    def test_single_plate_export_keeps_its_own_index(self):
+        # Bambu Studio's "current plate" export carries one <plate> whose index
+        # is still the original plate number, and its positions live in
+        # plate_3.json. Asking for plate 3 must match it.
+        objects, bbox_all = extract_printable_objects_from_3mf(
+            _single_plate_3mf(3), plate_number=3, include_positions=True
+        )
+        assert list(objects) == [PLATE_2_ID]
+        assert bbox_all == [40, 40, 60, 60]
+
+
+class TestPeekPlateIndexMultiPlate:
+    def test_multi_plate_file_has_no_single_plate_index(self, tmp_path):
+        # The #1204 guard compares this against the plate parsed from gcode_file
+        # and throws the 3MF away on mismatch. An all-plates upload has no one
+        # answer, and reporting plate 1 made the guard discard a correct file.
+        path = tmp_path / "all-plates.3mf"
+        path.write_bytes(_multi_plate_3mf())
+        assert peek_plate_index_in_3mf(path) is None
+
+    def test_single_plate_file_still_reports_its_index(self, tmp_path):
+        path = tmp_path / "one-plate.3mf"
+        path.write_bytes(_single_plate_3mf(2))
+        assert peek_plate_index_in_3mf(path) == 2
+
+
+class TestLoadObjectsFromArchiveWiring:
+    """The extractor took a plate_number all along — no caller ever passed one."""
+
+    @pytest.fixture
+    def archive_3mf(self, tmp_path, monkeypatch):
+        path = tmp_path / "job.3mf"
+        path.write_bytes(_multi_plate_3mf())
+        monkeypatch.setattr(main_module.app_settings, "base_dir", tmp_path)
+        return SimpleNamespace(file_path="job.3mf")
+
+    def _client(self, **state):
+        fields = {
+            "printable_objects": {},
+            "printable_objects_bbox_all": None,
+            "skipped_objects": [1],
+            "gcode_file": None,
+            "subtask_name": None,
+            "dispatched_plate_id": None,
+            "dispatched_subtask": None,
+        }
+        fields.update(state)
+        return SimpleNamespace(state=SimpleNamespace(**fields))
+
+    def _load(self, monkeypatch, client, archive):
+        monkeypatch.setattr(main_module.printer_manager, "get_client", lambda pid: client)
+        main_module._load_objects_from_archive(archive, 1, logging.getLogger(__name__))
+
+    def test_uses_the_plate_bambuddy_dispatched(self, monkeypatch, archive_3mf):
+        # Bambuddy-dispatched print: the plate is known from the dispatch itself,
+        # which is what the reporter's P1S does (its gcode_file echo carries no
+        # plate path — #1166).
+        client = self._client(dispatched_plate_id=2, dispatched_subtask="job", subtask_name="job")
+        self._load(monkeypatch, client, archive_3mf)
+
+        assert list(client.state.printable_objects) == [PLATE_2_ID]
+        assert client.state.printable_objects_bbox_all == [40, 40, 60, 60]
+        assert client.state.skipped_objects == []
+
+    def test_uses_the_plate_parsed_from_gcode_file(self, monkeypatch, archive_3mf):
+        # Print started outside Bambuddy: the plate comes from the gcode path.
+        client = self._client(gcode_file="/data/Metadata/plate_2.gcode")
+        self._load(monkeypatch, client, archive_3mf)
+
+        assert list(client.state.printable_objects) == [PLATE_2_ID]
+
+    def test_unknown_plate_still_loads_the_first(self, monkeypatch, archive_3mf):
+        client = self._client()
+        self._load(monkeypatch, client, archive_3mf)
+
+        assert sorted(client.state.printable_objects) == PLATE_1_IDS

Einige Dateien werden nicht angezeigt, da zu viele Dateien in diesem Diff geändert wurden.