Преглед изворни кода

fix(backup): match a K-profile on its own extruder, not just its filament (#2656)

`_match_kprofile` scoped candidates by `filament_id` alone, and
`_current_kprofile_index` reads the live index per nozzle *diameter* — so on a
dual-nozzle printer both extruders' profiles come back in one list.

On an H2D with the same filament calibrated on both extruders, the `setting_id`
arm then matched whichever profile the printer happened to list first. A
backed-up extruder-0 entry took extruder 1's slot and went into the batch as
`{extruder_id: 0, cali_idx: <extruder-1 slot>}`, writing one extruder's
calibration over the other's and counting it restored. With an entry per
extruder — the ordinary case, since the same preset on both nozzles is what a
dual-nozzle printer is for — the two swapped slots and clobbered each other.

The `name` arm and the single-candidate fallback were equally unscoped, so this
was never only about the ambiguous case.

The data was already in hand and already being read: the backup entry carries
`extruder_id` (`:1546` copies it straight into the outgoing dict) and
`KProfile` has carried `extruder_id: int` on the live side all along. Scope
`candidates` by it the same way `filament_id` already scopes them, and the
single-candidate fallback narrows with them — ambiguity is judged within one
extruder now.

Conditional on both sides saying which extruder they mean. A pre-#2656 backup
has no `extruder_id`, and a live index that reports none must not turn every
entry into an add — that would be a far worse regression than the bug. Two
controls cover each direction of that.

`claimed` (G3) is untouched: it is per nozzle-loop and this only narrows the
candidate set feeding it.

Tests: +4 across the three restore files (270 -> 274). Fail-pre-fix 2 (each
extruder keeps its own slot; the other extruder's profile is not a stand-in),
controls that pass either way 2.
jmoore-skild пре 1 месец
родитељ
комит
ae36d3ac13
2 измењених фајлова са 125 додато и 4 уклоњено
  1. 19 1
      backend/app/services/github_restore.py
  2. 106 3
      backend/tests/unit/test_github_restore.py

+ 19 - 1
backend/app/services/github_restore.py

@@ -1642,7 +1642,9 @@ class GitHubRestoreService:
         is the strongest signal; a delete-then-add edit regenerates it, so fall
         is the strongest signal; a delete-then-add edit regenerates it, so fall
         back to the display name, which Bambuddy's own editor preserves.
         back to the display name, which Bambuddy's own editor preserves.
         Both are scoped by ``filament_id`` — the same preset on a different
         Both are scoped by ``filament_id`` — the same preset on a different
-        filament is a different profile.
+        filament is a different profile — and by ``extruder_id``, because on a
+        dual-nozzle printer the same preset on the other extruder is a different
+        profile too.
 
 
         ``claimed`` holds the slot ids already taken by earlier entries in this
         ``claimed`` holds the slot ids already taken by earlier entries in this
         nozzle's loop, and no live profile may be claimed twice. Without it, two
         nozzle's loop, and no live profile may be claimed twice. Without it, two
@@ -1658,6 +1660,22 @@ class GitHubRestoreService:
             return None
             return None
 
 
         candidates = [c for c in current if c.filament_id == filament_id]
         candidates = [c for c in current if c.filament_id == filament_id]
+
+        # The live index is read per nozzle *diameter*, so on an H2D both
+        # extruders' profiles come back together. With the same filament
+        # calibrated on both — the ordinary case on a dual-nozzle printer, not an
+        # exotic one — filament_id alone lets extruder 0's backed-up entry match
+        # extruder 1's live profile, and the batch then carries
+        # {extruder_id: 0, cali_idx: <extruder-1 slot>}: one extruder's
+        # calibration written over the other's, counted restored.
+        #
+        # Conditional on both sides saying which extruder they mean. A pre-#2656
+        # backup carries no extruder_id, and a live index that reports none must
+        # not turn every entry into an add.
+        extruder_id = entry.get("extruder_id")
+        if isinstance(extruder_id, int) and any(getattr(c, "extruder_id", None) is not None for c in candidates):
+            candidates = [c for c in candidates if getattr(c, "extruder_id", None) == extruder_id]
+
         available = [c for c in candidates if c.slot_id not in claimed]
         available = [c for c in candidates if c.slot_id not in claimed]
         if not available:
         if not available:
             return None
             return None

+ 106 - 3
backend/tests/unit/test_github_restore.py

@@ -1232,9 +1232,19 @@ class TestRestoreArchives:
 
 
 class TestRestoreKprofiles:
 class TestRestoreKprofiles:
     @staticmethod
     @staticmethod
-    def _live(slot_id, filament_id="GFA00", name="Bambu PLA", setting_id="PFUS123"):
-        """One profile as the printer currently reports it."""
-        return SimpleNamespace(slot_id=slot_id, filament_id=filament_id, name=name, setting_id=setting_id)
+    def _live(slot_id, filament_id="GFA00", name="Bambu PLA", setting_id="PFUS123", extruder_id=0):
+        """One profile as the printer currently reports it.
+
+        ``extruder_id`` mirrors ``KProfile`` (bambu_mqtt.py), which has carried
+        it all along; single-nozzle printers report 0.
+        """
+        return SimpleNamespace(
+            slot_id=slot_id,
+            filament_id=filament_id,
+            name=name,
+            setting_id=setting_id,
+            extruder_id=extruder_id,
+        )
 
 
     def _client(self, live=None, sent="7", ack=(True, "")):
     def _client(self, live=None, sent="7", ack=(True, "")):
         """A connected printer client.
         """A connected printer client.
@@ -1527,6 +1537,99 @@ class TestRestoreKprofiles:
         profiles, _ = client.set_kprofiles_batch.call_args.args
         profiles, _ = client.set_kprofiles_batch.call_args.args
         assert [p["cali_idx"] for p in profiles] == [1, -1]
         assert [p["cali_idx"] for p in profiles] == [1, -1]
 
 
+    # --- the match is scoped to the extruder it was calibrated on -----------
+    #
+    # get_kprofiles reads per nozzle *diameter*, so on a dual-nozzle printer
+    # both extruders come back in one list. Scoping candidates on filament_id
+    # alone let one extruder's calibration be written over the other's.
+
+    @pytest.mark.asyncio
+    async def test_each_extruders_profile_lands_on_its_own_extruder(self, db_session, printer_factory):
+        """The same preset calibrated on both extruders of an H2D.
+
+        Both live profiles share a filament_id *and* a setting_id, so the
+        setting_id arm matched whichever the printer happened to list first —
+        and with an entry per extruder the two swapped slots, each overwriting
+        the other's calibration while the tally counted both restored.
+        """
+        await printer_factory(serial_number="00M09A123456789")
+        payload = self._payload()
+        entries = payload["kprofiles/00M09A123456789/0.4.json"]["profiles"]
+        entries.append({**entries[0], "extruder_id": 1, "nozzle_id": "HS00-0.4-R"})
+        client = self._client(
+            live=[
+                # Right extruder first, which is what made the bug bite.
+                self._live(slot_id=1001, extruder_id=1),
+                self._live(slot_id=1000, extruder_id=0),
+            ]
+        )
+        tally = _CategoryTally()
+
+        with patch("backend.app.services.github_restore.printer_manager") as manager:
+            manager.get_client = MagicMock(return_value=client)
+            await _service()._restore_kprofiles(db_session, payload, tally)
+
+        profiles, _ = client.set_kprofiles_batch.call_args.args
+        assert [(p["extruder_id"], p["cali_idx"]) for p in profiles] == [(0, 1000), (1, 1001)]
+        assert tally.restored == 2
+        assert not any("added as new profiles" in note for note in _messages(tally))
+
+    @pytest.mark.asyncio
+    async def test_the_other_extruders_profile_is_not_a_candidate(self, db_session, printer_factory):
+        """One backed-up entry, and the only live profile is the other extruder's.
+
+        Adding as new is the right answer: extruder 0's calibration is not a
+        stand-in for extruder 1's, however well the filament and preset line up.
+        """
+        await printer_factory(serial_number="00M09A123456789")
+        client = self._client(live=[self._live(slot_id=1001, extruder_id=1)])
+        tally = _CategoryTally()
+
+        with patch("backend.app.services.github_restore.printer_manager") as manager:
+            manager.get_client = MagicMock(return_value=client)
+            await _service()._restore_kprofiles(db_session, self._payload(), tally)
+
+        profiles, _ = client.set_kprofiles_batch.call_args.args
+        assert profiles[0]["cali_idx"] == -1
+        assert any("added as new profiles" in note for note in _messages(tally))
+
+    @pytest.mark.asyncio
+    async def test_an_entry_without_an_extruder_id_still_matches(self, db_session, printer_factory):
+        """Control: a pre-#2656 backup carries no extruder_id.
+
+        A missing key must leave the match exactly as it was, not turn every
+        entry into an add.
+        """
+        await printer_factory(serial_number="00M09A123456789")
+        payload = self._payload()
+        payload["kprofiles/00M09A123456789/0.4.json"]["profiles"][0].pop("extruder_id")
+        client = self._client(live=[self._live(slot_id=4606)])
+        tally = _CategoryTally()
+
+        with patch("backend.app.services.github_restore.printer_manager") as manager:
+            manager.get_client = MagicMock(return_value=client)
+            await _service()._restore_kprofiles(db_session, payload, tally)
+
+        profiles, _ = client.set_kprofiles_batch.call_args.args
+        assert profiles[0]["cali_idx"] == 4606
+        assert tally.restored == 1
+
+    @pytest.mark.asyncio
+    async def test_a_live_index_that_reports_no_extruder_still_matches(self, db_session, printer_factory):
+        """Control: the same, for a printer whose profiles carry no extruder_id."""
+        await printer_factory(serial_number="00M09A123456789")
+        live = SimpleNamespace(slot_id=4606, filament_id="GFA00", name="Bambu PLA", setting_id="PFUS123")
+        client = self._client(live=[live])
+        tally = _CategoryTally()
+
+        with patch("backend.app.services.github_restore.printer_manager") as manager:
+            manager.get_client = MagicMock(return_value=client)
+            await _service()._restore_kprofiles(db_session, self._payload(), tally)
+
+        profiles, _ = client.set_kprofiles_batch.call_args.args
+        assert profiles[0]["cali_idx"] == 4606
+        assert tally.restored == 1
+
     @pytest.mark.asyncio
     @pytest.mark.asyncio
     async def test_unknown_serial_is_skipped_with_reason(self, db_session):
     async def test_unknown_serial_is_skipped_with_reason(self, db_session):
         tally = _CategoryTally()
         tally = _CategoryTally()