Просмотр исходного кода

fix(archives): resolve raw_data wrapper in fallback-archive filament extraction (#1533 follow-up)

  Reporter updated to 0.2.5b1 expecting the #1533 fix to populate filament
  fields on his P2S virtual-printer prints when the .3mf is locked. His
  support bundle showed Bambuddy still creating fallback archives with NULL
  filament fields even though the print-start log line proved AMS-0-T0 had
  PETG loaded at the moment the helper should have read it
  (`AMS 0: T0(type=PETG, color=FFFFFFFF, ...)`).

  Cause: the #1533 helper `_extract_filament_data_from_mqtt(data)` in
  backend/app/main.py only looked at `data["ams"]`, but the dict that
  on_print_start actually receives at runtime is the wrapper shape
  `{"filename", "subtask_name", "remaining_time", "raw_data": <mqtt>,
  "ams_mapping"}` that backend/app/services/bambu_mqtt.py:2971-2980
  constructs. So `data["ams"]` was undefined on every real call and the
  helper silently returned `{}`, leaving the fallback archive's
  filament_type / filament_color NULL — the exact regression the original
  fix was meant to close. The 15 unit tests that shipped with #1533 all
  passed the bare inner shape directly and never exercised the callback
  wiring, so the regression slipped through a green build. Surrounding code
  in the same fallback path (main.py:2064, 2674) already reads
  `data.get("raw_data")` — the original hunk was the outlier that forgot
  the wrapper.

  Fix: helper now resolves `data["raw_data"]["ams"]` first (the callback
  shape) and only falls back to `data["ams"]` when the wrapper isn't
  present, preserving the inner-shape callers from the existing tests.
  Defensive against a non-dict `raw_data` (e.g. partial MQTT decode
  failure) falling through to the inner lookup instead of crashing.

  Tests: 5 new in `TestOnPrintStartCallbackShape` in
  test_fallback_archive_mqtt_filament.py — wrapper payload with ams_mapping
  resolves to the inner AMS state; wrapper without ams_mapping lists all
  loaded slots; the existing inner-shape callers still work after the
  additive wrapper lookup; missing raw_data returns `{}` instead of
  raising; junk raw_data (string) doesn't shadow a present inner `ams`.
  Existing 15 inner-shape tests untouched and green. Full 5378-test
  backend suite green; backend ruff clean.

  What this does NOT fix: per-filament gram usage still needs the actual
  .3mf — the printer locks it during print (P-line firmware behaviour,
  not a Bambuddy bug), and the existing 19 FTP candidate paths +
  directory probes are expected to 550 in that window. Per-print filament
  type and colour are the data point the reporter explicitly called out
  as load-bearing for AMS-expansion planning at his maker space, so this
  is what moves the needle.
maziggy 1 день назад
Родитель
Сommit
7a7dfed81a
3 измененных файлов с 103 добавлено и 1 удалено
  1. 0 0
      CHANGELOG.md
  2. 16 1
      backend/app/main.py
  3. 87 0
      backend/tests/unit/test_fallback_archive_mqtt_filament.py

Разница между файлами не показана из-за своего большого размера
+ 0 - 0
CHANGELOG.md


+ 16 - 1
backend/app/main.py

@@ -644,9 +644,24 @@ def _extract_filament_data_from_mqtt(data: dict, ams_mapping: list[int] | None =
     entries). When supplied, only the slots actually consumed by this
     print contribute. Without it the function falls back to every loaded
     AMS slot — less accurate but still useful.
+
+    Accepts both the raw inner payload (``{"ams": {"ams": [...]}, ...}``)
+    that the unit tests pass directly, AND the on_print_start callback
+    shape (``{"raw_data": {"ams": {"ams": [...]}, ...}, ...}``) the
+    bambu_mqtt service hands to main.py at runtime. The original
+    ``_extract_filament_data_from_mqtt(data)`` shipped in #1533 only
+    handled the inner shape and silently returned ``{}`` for every real
+    print start, leaving fallback archives' filament fields NULL — the
+    exact regression the fix was meant to close. Reported with a log
+    proving the AMS state was right there at
+    ``data["raw_data"]["ams"]["ams"][0]["tray"][0]`` (#1533 follow-up).
     """
     result: dict[str, str] = {}
-    ams_root = (data or {}).get("ams") or {}
+    # Look at the on_print_start wrapper first, then the inner shape.
+    raw_data = (data or {}).get("raw_data")
+    ams_root = (raw_data or {}).get("ams") if isinstance(raw_data, dict) else None
+    if not isinstance(ams_root, dict):
+        ams_root = (data or {}).get("ams") or {}
     ams_units = ams_root.get("ams") if isinstance(ams_root, dict) else None
     if not isinstance(ams_units, list) or not ams_units:
         return result

+ 87 - 0
backend/tests/unit/test_fallback_archive_mqtt_filament.py

@@ -204,3 +204,90 @@ class TestExtractFilamentDataFromMqtt:
     @pytest.mark.parametrize("data", [None, {}, {"ams": "weird-string"}])
     def test_garbage_top_level_is_empty(self, data):
         assert _extract_filament_data_from_mqtt(data or {}) == {}
+
+
+class TestOnPrintStartCallbackShape:
+    """Regression: the callback wrapper shape the bambu_mqtt service
+    actually hands to ``on_print_start`` at runtime (#1533 follow-up).
+
+    The original #1533 fix only handled the bare ``{"ams": {"ams": [...]}}``
+    inner shape, but the call site at ``backend/app/main.py::on_print_start``
+    receives the wrapper
+    ``{"filename", "subtask_name", "remaining_time", "raw_data": <mqtt>,
+       "ams_mapping"}`` from ``backend/app/services/bambu_mqtt.py:2971-2980``.
+    The lookup at ``data["ams"]`` therefore missed every real print and
+    fallback archives kept their filament fields NULL — the exact regression
+    the fix was supposed to close. Reproduced from JmanB52D's support
+    bundle whose print start log line showed
+    ``AMS 0: T0(type=PETG, color=FFFFFFFF, …)`` was sitting right there at
+    ``data["raw_data"]["ams"]["ams"][0]["tray"][0]``.
+    """
+
+    def test_callback_wrapper_payload_resolves_raw_data_path(self):
+        """The wrapper-shape payload must produce the same result the
+        inner-shape payload would."""
+        inner = {
+            "ams": {
+                "ams": [
+                    _ams_unit(0, [_tray(0, "PETG", "FFFFFFFF")]),
+                ],
+            },
+        }
+        wrapper = {
+            "filename": "/data/Metadata/plate_1.gcode",
+            "subtask_name": "xyz-10mm-calibration-cube",
+            "remaining_time": 1200,
+            "raw_data": inner,
+            "ams_mapping": [0],
+        }
+        result = _extract_filament_data_from_mqtt(wrapper, ams_mapping=[0])
+        # The 6-char rgba `FFFFFF` is what the AMS reports (with `FF` alpha
+        # tail trimmed by the catalog) — the helper preserves whatever the
+        # firmware sends.
+        assert result == {"filament_type": "PETG", "filament_color": "FFFFFFFF"}
+
+    def test_wrapper_with_no_ams_mapping_falls_back_to_all_loaded(self):
+        """Wrapper shape without an ams_mapping behaves the same as the
+        inner-shape no-mapping path: lists every loaded slot."""
+        inner = {
+            "ams": {
+                "ams": [
+                    _ams_unit(0, [_tray(0, "PLA", "FF0000"), _tray(1, "PETG", "00FF00")]),
+                ],
+            },
+        }
+        wrapper = {"raw_data": inner}
+        result = _extract_filament_data_from_mqtt(wrapper)
+        assert result == {"filament_type": "PLA,PETG", "filament_color": "FF0000,00FF00"}
+
+    def test_inner_shape_still_supported_after_wrapper_lookup(self):
+        """Existing callers that pass the inner shape directly (e.g. the
+        unit tests above) must keep working — the new lookup is additive."""
+        inner = {
+            "ams": {
+                "ams": [_ams_unit(0, [_tray(0, "ASA", "112233")])],
+            },
+        }
+        assert _extract_filament_data_from_mqtt(inner) == {
+            "filament_type": "ASA",
+            "filament_color": "112233",
+        }
+
+    def test_wrapper_with_missing_raw_data_returns_empty(self):
+        """No raw_data wrapper AND no top-level ams → empty, no raise."""
+        wrapper = {"filename": "foo.gcode", "ams_mapping": [0]}
+        assert _extract_filament_data_from_mqtt(wrapper, ams_mapping=[0]) == {}
+
+    def test_wrapper_with_non_dict_raw_data_falls_through_to_inner_lookup(self):
+        """Defensive: a junk raw_data value (string / None) shouldn't crash
+        and shouldn't shadow a present inner ``ams`` either. Lets us catch
+        the case where MQTT decoding partially fails but the rest of the
+        payload is fine."""
+        wrapper = {
+            "raw_data": "garbage",
+            "ams": {"ams": [_ams_unit(0, [_tray(0, "PLA", "FF0000")])]},
+        }
+        assert _extract_filament_data_from_mqtt(wrapper) == {
+            "filament_type": "PLA",
+            "filament_color": "FF0000",
+        }

Некоторые файлы не были показаны из-за большого количества измененных файлов