Procházet zdrojové kódy

fix(backup): let the nozzle_id default apply, and prefer the live one (#2656)

`set_kprofiles_batch` defaults the field with
`p.get("nozzle_id", f"HS00-{diameter}")`, and a `dict.get` default only
fires when the key is *absent*. The restore always set the key —
`"nozzle_id": p.get("nozzle_id")` — so a backup that carries no nozzle_id
published `nozzle_id: null` to the printer instead. Printers that omit the
field (KProfilesView's own #1748 comments) are exactly why the default is
there, and it was unreachable from this path.

Set only when known, on the same precedence the `setting_id` line beside it
already uses: the live profile first, then the backup, then absent. Live
first is the point rather than a bonus — `nozzle_id` encodes the fitted
nozzle's type as well as its diameter (`HS00-` hardened vs `SS00-`
stainless), so a nozzle swapped since the backup makes the stored value
stale, and the write lands on the nozzle fitted now.

Read with `getattr`, matching the defensive read of `extruder_id` in
`_match_kprofile`: not every live profile carries every field, and
`test_a_live_index_that_reports_no_extruder_still_matches` is the standing
control for that.

Four tests, two of which fail without the fix. The `_live` double also
gained `nozzle_id` — it is a non-default field on the real `KProfile`
dataclass, so omitting it let the double license code the real object
would have accepted.
jmoore-skild před 1 měsícem
rodič
revize
06fa146e2f

+ 29 - 18
backend/app/services/github_restore.py

@@ -1621,24 +1621,35 @@ class GitHubRestoreService:
                         unmatched += 1
                         unmatched += 1
                     else:
                     else:
                         claimed.add(match.slot_id)
                         claimed.add(match.slot_id)
-                    profile_dicts.append(
-                        {
-                            "filament_id": p.get("filament_id", ""),
-                            "name": p.get("name", ""),
-                            "k_value": p.get("k_value", "0.020000"),
-                            "nozzle_id": p.get("nozzle_id"),
-                            "extruder_id": p.get("extruder_id", 0),
-                            # Prefer the live setting_id when we matched: it is
-                            # what the printer currently associates with the slot.
-                            "setting_id": (match.setting_id if match else None) or p.get("setting_id"),
-                            # cali_idx -1 tells the printer to add a new profile
-                            # rather than address a slot that isn't there.
-                            "cali_idx": match.slot_id if match else -1,
-                            # Only consulted for the generated-setting_id
-                            # fallback; cali_idx above takes precedence.
-                            "slot_id": 0,
-                        }
-                    )
+                    entry = {
+                        "filament_id": p.get("filament_id", ""),
+                        "name": p.get("name", ""),
+                        "k_value": p.get("k_value", "0.020000"),
+                        "extruder_id": p.get("extruder_id", 0),
+                        # Prefer the live setting_id when we matched: it is
+                        # what the printer currently associates with the slot.
+                        "setting_id": (match.setting_id if match else None) or p.get("setting_id"),
+                        # cali_idx -1 tells the printer to add a new profile
+                        # rather than address a slot that isn't there.
+                        "cali_idx": match.slot_id if match else -1,
+                        # Only consulted for the generated-setting_id
+                        # fallback; cali_idx above takes precedence.
+                        "slot_id": 0,
+                    }
+
+                    # Same precedence as setting_id, and set only when known.
+                    # nozzle_id encodes the fitted nozzle's type and diameter
+                    # ("HS00-0.4"), so the live value beats the backup's: the
+                    # user may have swapped the nozzle since. When neither knows,
+                    # the key has to be *absent* — set_kprofiles_batch supplies
+                    # HS00-{diameter} via p.get(..., default), which a key
+                    # present-and-None defeats, publishing a null nozzle_id.
+                    # Printers that omit it are the reason the default is there
+                    # (#1748), so it has to be reachable.
+                    nozzle_id = (getattr(match, "nozzle_id", None) if match else None) or p.get("nozzle_id")
+                    if nozzle_id:
+                        entry["nozzle_id"] = nozzle_id
+                    profile_dicts.append(entry)
                 if not profile_dicts:
                 if not profile_dicts:
                     continue
                     continue
                 if unmatched:
                 if unmatched:

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

@@ -1375,11 +1375,21 @@ class TestRestoreArchives:
 
 
 class TestRestoreKprofiles:
 class TestRestoreKprofiles:
     @staticmethod
     @staticmethod
-    def _live(slot_id, filament_id="GFA00", name="Bambu PLA", setting_id="PFUS123", extruder_id=0):
+    def _live(
+        slot_id,
+        filament_id="GFA00",
+        name="Bambu PLA",
+        setting_id="PFUS123",
+        extruder_id=0,
+        nozzle_id="HS00-0.4",
+    ):
         """One profile as the printer currently reports it.
         """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.
+        ``extruder_id`` and ``nozzle_id`` mirror ``KProfile`` (bambu_mqtt.py),
+        which has carried both all along; single-nozzle printers report
+        extruder 0. Both are non-default fields there, so a live profile always
+        has them — the double must too, or it licenses code that would break on
+        the real object.
         """
         """
         return SimpleNamespace(
         return SimpleNamespace(
             slot_id=slot_id,
             slot_id=slot_id,
@@ -1387,6 +1397,7 @@ class TestRestoreKprofiles:
             name=name,
             name=name,
             setting_id=setting_id,
             setting_id=setting_id,
             extruder_id=extruder_id,
             extruder_id=extruder_id,
+            nozzle_id=nozzle_id,
         )
         )
 
 
     def _client(self, live=None, sent="7", ack=(True, "")):
     def _client(self, live=None, sent="7", ack=(True, "")):
@@ -1773,6 +1784,73 @@ class TestRestoreKprofiles:
         assert profiles[0]["cali_idx"] == 4606
         assert profiles[0]["cali_idx"] == 4606
         assert tally.restored == 1
         assert tally.restored == 1
 
 
+    @pytest.mark.asyncio
+    async def test_a_backup_without_a_nozzle_id_omits_the_key(self, db_session, printer_factory):
+        """``set_kprofiles_batch`` defaults it, and only an absent key lets it.
+
+        The default is ``p.get("nozzle_id", f"HS00-{diameter}")``, which a key
+        present-and-None defeats — the batch would publish a null nozzle_id to
+        the printer. Printers that omit the field (#1748) are the reason the
+        default exists, so it has to be reachable.
+        """
+        await printer_factory(serial_number="00M09A123456789")
+        payload = self._payload()
+        payload["kprofiles/00M09A123456789/0.4.json"]["profiles"][0].pop("nozzle_id")
+        # No live match either, so neither source can supply one.
+        client = self._client(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, payload, tally)
+
+        profiles, _ = client.set_kprofiles_batch.call_args.args
+        assert "nozzle_id" not in profiles[0]
+
+    @pytest.mark.asyncio
+    async def test_the_backups_nozzle_id_is_used_when_nothing_is_live(self, db_session, printer_factory):
+        await printer_factory(serial_number="00M09A123456789")
+        client = self._client(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]["nozzle_id"] == "HS00-0.4"
+
+    @pytest.mark.asyncio
+    async def test_the_live_nozzle_id_beats_the_backups(self, db_session, printer_factory):
+        """The nozzle may have been swapped since the backup; we write to the
+        one that is fitted now, exactly as with setting_id."""
+        await printer_factory(serial_number="00M09A123456789")
+        client = self._client(live=[self._live(slot_id=4606, nozzle_id="SS00-0.4")])
+        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]["nozzle_id"] == "SS00-0.4"
+
+    @pytest.mark.asyncio
+    async def test_a_live_profile_without_a_nozzle_id_falls_back_to_the_backup(self, db_session, printer_factory):
+        """Same defensive read as extruder_id: not every live profile carries
+        every field."""
+        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]["nozzle_id"] == "HS00-0.4"
+
     @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()