From 9c4252911bf94e995d7177044f22a19cd391a64b Mon Sep 17 00:00:00 2001 From: maziggy Date: Fri, 12 Jun 2026 14:04:15 +0200 Subject: [PATCH] fix(vp): overlay incoming dict-shaped push_status fields onto cache instead of replacing (#1622 round 5) Right after the slicer picks a filament for the external spool (vt_tray, ams_id=255), Bambu firmware pushes a partial vt_tray carrying just {tray_info_idx, tray_color} - ~18 fields shorter than the pushall shape the slicer expects. The #1622 round-4 per-field accumulate (da799447) only carried over prev keys NOT in new, so the cached vt_tray was replaced wholesale with the 2-field partial. The next 1 Hz cached-as-base push delivered the stripped dict and BambuStudio rendered the external slot as invalid (color only, no tray_type / state / k / n / cali_idx / nozzle_temp_*). Reload restored it because the reconnect-triggered pushall re-seeded vt_tray, then the cycle repeated. AMS slots didn't suffer because _merge_ams_dict deep-merged them. Fix: for every top-level push_status key whose prev AND new are both dicts, overlay incoming keys onto prev rather than replace. ams is excluded (already deep-merged). The same shape protects device / online / upgrade_state / ipcam / upload / net against future firmware partials. net.info IP rewrite is unaffected - _rewrite_net_info_ips runs before caching and overlay lets the freshly-rewritten list win over prev when present. --- CHANGELOG.md | 1 + .../services/virtual_printer/mqtt_bridge.py | 23 +++++ backend/tests/unit/test_vp_mqtt_bridge.py | 94 +++++++++++++++++++ 3 files changed, 118 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98884745e..f44a939db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ All notable changes to Bambuddy will be documented in this file. ## [0.2.5b1] - Unreleased ### Fixed +- **Virtual printer external spool (`vt_tray`) went "invalid" right after a slicer filament pick (#1622 round 5, reported by @shaddowlink)** — On a P1S in non-proxy VP mode, the reporter picked a filament for the external spool slot in BambuStudio's Device tab and the slot immediately rendered as invalid (color only, no profile, no K-profile, no nozzle temps), but recovered after a virtual-printer reload. AMS slot picks worked correctly. Wire dumps (BAMBUDDY_VP_DUMP_WIRE=1) captured the asymmetry: the bridge's outgoing 1 Hz cached-as-base push delivered `vt_tray = {tray_info_idx, tray_color}` — 2 fields — where a real P1S sends ~20 (`tray_type`, `state`, `remain`, `k`, `n`, `cali_idx`, `nozzle_temp_min/max`, `tray_uuid`, `xcam_info`, ...). The same `_out.json` showed AMS slots with the full 24-field dict because `_merge_ams_dict` deep-merged them. **Root cause:** Bambu firmware sends a partial `vt_tray` incremental right after acknowledging an `ams_filament_setting` for `ams_id=255` (external spool) — carrying just the fields the slicer's pick changed. The round-4 per-field accumulate (#1622 / da799447) carried over prev keys NOT present in new, but `vt_tray` IS present in new, so the cached dict was REPLACED wholesale with the 2-field partial. The next 1 Hz cached-as-base push handed the slicer the stripped vt_tray; BambuStudio rendered the slot as invalid. Reloading the VP forced a reconnect → pushall → full vt_tray restored, and the cycle repeated on the next pick. **Fix:** `mqtt_bridge.py::_on_printer_raw` now applies the same per-field accumulate one level deeper: for every top-level key whose prev AND new are both dicts, overlay new onto prev rather than replace. `ams` is explicitly excluded (already deep-merged by `_merge_ams_dict`). The same overlay protects `device`, `online`, `upgrade_state`, `ipcam`, `upload`, `net` against future firmware partials with the same shape; the `net.info` IP rewrite path is unaffected because `_rewrite_net_info_ips` runs against `new_state["net"]` before caching and the rewritten list overrides the cached one on overlay (only `net.conf` and friends, when sent without `info`, draw from prev now). **Tests:** new `test_partial_vt_tray_update_overlays_onto_cached_full_dict` regression case in `test_vp_mqtt_bridge.py::TestPushStatusCache` constructs the exact P1S wire shape — pushall with the full ~20-field vt_tray, followed by the `{tray_info_idx, tray_color}` partial that shaddowlink's dump captured — and asserts `tray_type`, `state`, `remain`, `k`, `n`, `cali_idx`, `nozzle_temp_min/max`, `tray_uuid`, `id` all survive while the two incoming fields take their new values. All 53 bridge tests stay green; 287/287 across the broader VP test surface (mqtt_bridge / mqtt_server / vp_wire / virtual_printer); ruff clean. Bridge code path only; no migration, no new i18n keys, no frontend touch. - **Library G-code preview returned raw ZIP bytes as `text/plain` for sidecar-sliced rows (#1709, root cause + fix from @yanglei1980)** — `slice_and_persist` writes its output as a `.gcode.3mf` (a ZIP container with embedded G-code) but persisted the LibraryFile row with `file_type="gcode"`. The G-code preview endpoint at `library.py::get_gcode` short-circuits on `file_type == "gcode"` and streams the on-disk bytes with `media_type="text/plain"`, so every preview of a sidecar-sliced row handed the embedded viewer the raw ZIP body (`PK\x03\x04…`) instead of toolpath text — the viewer rendered nothing. External-folder scans (#1600) already typed `.gcode.3mf` rows correctly and hit the unzip branch, so the bug was specific to the sidecar slice path. Plain `.gcode` uploads were unaffected (their on-disk bytes really are text). **Fix:** (1) forward — `slice_and_persist` now persists `file_type="gcode.3mf"`, matching what `_classify_file_type` returns for the `.gcode.3mf` extension and what external-scan rows already use; (2) back-compat — `get_gcode` also routes to the unzip branch when the filename ends with `.gcode.3mf`, so rows already written under the bug self-heal on first preview without a DB migration. **UI gates:** three frontend call sites that gated badge colour or the preview-eye icon on `file_type == "gcode"` were extended to also accept `"gcode.3mf"` — `FileManagerPage.tsx` badge + viewer-affordance gate, `ProjectDetailPage.tsx` badge — so the new typing doesn't regress visuals. The print / queue / slice action buttons use filename-based helpers (`isSlicedFilename`, `isSliceableFilename`) that already accept `.gcode.3mf`, so they need no change. **Tests:** new `test_library_get_gcode_recovers_legacy_gcode_type_for_3mf` regression case in `test_library_api.py` constructs a row with `file_type="gcode"` + `.gcode.3mf` filename pointing at a real ZIP, asserts the response is `text/plain`, contains `G28`, and does NOT start with `PK` — pins the legacy-row recovery path. Existing `test_library_get_gcode_endpoint_accepts_compound_file_type` continues to cover the forward path. Full backend suite 5920/5920 green; ruff clean; frontend ESLint + `npm run build` clean; FileManagerPage / ProjectDetailPage / FileManagerExternalFolder vitests 69/69 green; i18n parity unchanged (no new keys). PR #1709 closed for CONTRIBUTING.md non-compliance (branched from main, no issue, template incomplete); root cause + fix shape preserved here on `dev`. - **Cloud + Orca Cloud preset resolver: pin `type` and `from` to CLI-accepted values (#1712 follow-up, reported by maziggy on the Mecha Mewtwo slice)** — Removing bundle mode (entry above) routed every slot through the cross-tier preset resolver. Cloud-tier presets surfaced two latent shape mismatches that bundle dispatch had been masking by materialising preset JSONs from `.bbscfg`-on-disk. (1) **`type` field**: Bambu Cloud labels presets with `type: "printer"` / `"print"` / `"filament"`, but the BambuStudio CLI's `--load-settings` parser only accepts `"machine"` / `"process"` / `"filament"`. The user's first failing slice produced `operator(): unknown config type print of file preset.json in load-settings` with exit code -5; the sidecar surfaces this as a generic "The input preset file is invalid and can not be parsed." (2) **`from` field**: Bambu Cloud's filament detail endpoint routinely ships presets with empty `from` (or no `from` at all). The CLI's compatibility check rejects either with `operator(): file ... 's from unsupported` (the double space in stderr = empty value). Same -5 exit, same generic "input preset invalid" surface. The sidecar's `normalizeFromField` already maps `"User"` / `"System"` → `"system"`, but it doesn't touch empty / missing values. **Fix:** `_resolve_cloud` and `_resolve_orca_cloud` now force `type = _SLOT_TO_PROFILE_TYPE[slot]` and `from = "system"` on the payload before `json.dumps`, mirroring what `_resolve_standard` already does for the standard-tier stub. Both fields are unconditionally rewritten — idempotent on already-correct payloads, and pinning to "system" is consistent with how Bambuddy presents these post-flatten presets to the CLI (no parent walk needed, the cloud detail comes back fully expanded). **Tests:** new `test_cloud_rewrites_type_field_for_cli` (7 parametric cases covering all six type-name variants Bambu Cloud emits plus the missing-type case), `test_cloud_pins_from_field_to_system` (4 cases: empty, already-system, GUI User, GUI System), and `test_cloud_synthesises_from_field_when_missing` (the actual Mecha Mewtwo failure shape) pin the resolver-level contract. Existing happy-path assertions for `_resolve_cloud` / `_resolve_orca_cloud` updated to include the new fields. 28/28 preset-resolver tests green, full backend suite 5919/5919 green, ruff clean. **What this can NOT recover:** if Bambu Cloud later starts emitting a `from` value other than empty / "User" / "System" that genuinely means something (e.g. "project"), Bambuddy will silently flatten it to "system" too. We accept that trade-off because the alternative is leaving "input preset invalid" failures on every cloud slice, and "system" matches how the sidecar's own resolver normalises the post-flatten state. diff --git a/backend/app/services/virtual_printer/mqtt_bridge.py b/backend/app/services/virtual_printer/mqtt_bridge.py index 29efc5fe8..328b72e83 100644 --- a/backend/app/services/virtual_printer/mqtt_bridge.py +++ b/backend/app/services/virtual_printer/mqtt_bridge.py @@ -608,6 +608,29 @@ class MQTTBridge: # internally in `_handle_ams_data`. if isinstance(new_state.get("ams"), dict) and isinstance(prev.get("ams"), dict): new_state["ams"] = _merge_ams_dict(prev["ams"], new_state["ams"]) + # Same per-field accumulate rule applied one level deeper for + # other top-level dict-shaped fields. Firmware sends partial + # `vt_tray` (external spool) updates right after a slicer + # `ams_filament_setting` pick — typically just `{tray_info_idx, + # tray_color}`, dropping the ~18 other fields (`tray_type`, + # `state`, `remain`, `k`, `n`, `cali_idx`, `nozzle_temp_min/max`, + # `tray_uuid`, `xcam_info`, ...) the slicer needs to render the + # slot. Without overlay the next 1 Hz cached-as-base push + # delivered the stripped dict and the slicer rendered the + # external slot as "invalid" until a reload triggered a fresh + # pushall (#1622 round 5, reported by @shaddowlink). AMS slots + # didn't suffer because `_merge_ams_dict` deep-merges per tray. + # Same shape covers `device`, `online`, `upgrade_state`, `ipcam`, + # `upload`, `net`, ... against future firmware partials too. + # `ams` is excluded — already deep-merged above. + for key, new_value in list(new_state.items()): + if key == "ams": + continue + prev_value = prev.get(key) + if isinstance(prev_value, dict) and isinstance(new_value, dict): + merged = dict(prev_value) + merged.update(new_value) + new_state[key] = merged self._latest_print_state = new_state dump_wire(self.vp_name, "in", new_state) return diff --git a/backend/tests/unit/test_vp_mqtt_bridge.py b/backend/tests/unit/test_vp_mqtt_bridge.py index 1cef1a80a..7a2fbc839 100644 --- a/backend/tests/unit/test_vp_mqtt_bridge.py +++ b/backend/tests/unit/test_vp_mqtt_bridge.py @@ -469,6 +469,100 @@ class TestPushStatusCache: await bridge.stop() + @pytest.mark.asyncio + async def test_partial_vt_tray_update_overlays_onto_cached_full_dict(self): + """Regression for #1622 round 5 (reported by @shaddowlink): right after + the slicer picks a filament for the external spool (vt_tray, ams_id=255), + Bambu firmware pushes a partial vt_tray carrying just the changed + fields — typically ``{tray_info_idx, tray_color}`` — and omits the + ~18 other keys (tray_type, state, k, n, cali_idx, nozzle_temp_min/max, + tray_uuid, xcam_info, ...) the slicer needs to render the slot. + Before this fix the per-field accumulate replaced the cached vt_tray + wholesale (it only carried over prev keys NOT present in new), so the + next 1 Hz cached-as-base push handed the slicer a stripped vt_tray and + BambuStudio rendered the external slot as "invalid" until a reload + triggered a fresh pushall. AMS slots didn't suffer because + `_merge_ams_dict` already deep-merged them. The fix overlays incoming + keys onto the previous dict for every top-level dict-shaped field + (excluding ams, which keeps its own deep merge). + """ + server = _make_server() + bridge = _make_bridge(server) + await bridge.start() + + # 1. Pushall response with the full ~20-field vt_tray dict a real + # P1S sends to bootstrap the slot. + full_push = json.dumps( + { + "print": { + "command": "push_status", + "vt_tray": { + "id": "254", + "tray_info_idx": "Pea5f68f", + "tray_type": "PLA", + "tray_sub_brands": "", + "tray_color": "F72323FF", + "tray_weight": "0", + "tray_diameter": "0.00", + "tray_temp": "0", + "tray_time": "0", + "bed_temp_type": "0", + "bed_temp": "0", + "nozzle_temp_max": "240", + "nozzle_temp_min": "190", + "xcam_info": "000000000000000000000000", + "tray_uuid": "00000000000000000000000000000000", + "ctype": 0, + "remain": -1, + "k": 0.01999999955296, + "n": 1, + "cali_idx": -1, + "state": 3, + }, + } + } + ).encode() + bridge._on_printer_raw(f"device/{H2D_SERIAL}/report", full_push) + await asyncio.sleep(0.01) + + # 2. Incremental push carrying just the two fields the slicer's pick + # changed — exactly the shape the P1S firmware sends after an + # ams_filament_setting ack. This is what shaddowlink's wire dump + # captured for the failing case. + incremental_push = json.dumps( + { + "print": { + "command": "push_status", + "vt_tray": { + "tray_info_idx": "Pea5f68f", + "tray_color": "76D9F4FF", + }, + } + } + ).encode() + bridge._on_printer_raw(f"device/{H2D_SERIAL}/report", incremental_push) + await asyncio.sleep(0.01) + + cached = bridge.get_latest_print_state() + vt = cached["vt_tray"] + # Incoming fields applied. + assert vt["tray_info_idx"] == "Pea5f68f" + assert vt["tray_color"] == "76D9F4FF" + # All other fields preserved from the prior pushall — without these + # the slicer rendered the slot as invalid. + assert vt["tray_type"] == "PLA" + assert vt["state"] == 3 + assert vt["remain"] == -1 + assert vt["k"] == 0.01999999955296 + assert vt["n"] == 1 + assert vt["cali_idx"] == -1 + assert vt["nozzle_temp_min"] == "190" + assert vt["nozzle_temp_max"] == "240" + assert vt["tray_uuid"] == "00000000000000000000000000000000" + assert vt["id"] == "254" + + await bridge.stop() + @pytest.mark.asyncio async def test_partial_ams_status_update_preserves_unit_list(self): """#1387: Bambu firmware also sends `ams` updates where the key is