mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-09-30 03:01:21 +02:00
fix(vp): emit FINISH after FTP upload so Print-flow slicers unwedge (#1280)
Bambuddy's VP supports two slicer flows: Send (file upload only — what queue/immediate/review modes are designed for) and Print (file upload + start-print, intended for proxy mode). When a user clicks Print against a non-proxy mode the VP must still respond gracefully — the file is fine to receive, just the start-print never happens. Instead the slicer wedged at "Downloading...(0%)" and blocked the next dispatch with "The printer is busy with another print job". Cause: on_file_received transitioned gcode_state PREPARE -> IDLE directly. Print-flow slicers watch the state cycle and only release their in-flight-job lock on PREPARE -> ... -> FINISH (or FAILED). PREPARE -> IDLE looks like "printer abandoned my job" and keeps the prior job pinned in the slicer's memory. Fix: transition PREPARE -> FINISH with prepare_percent=100. The 1-Hz periodic status push broadcasts the new state to every connected slicer within a second. Send-flow slicers don't watch this state so the change is a no-op for them; Print-flow slicers see the FINISH they were waiting for and unwedge.
This commit is contained in:
@@ -28,6 +28,8 @@ All notable changes to Bambuddy will be documented in this file.
|
||||
- **Copy spool — duplicate any spool's settings into a fresh inventory row in two clicks** ([#1234](https://github.com/maziggy/bambuddy/issues/1234), [PR #1246](https://github.com/maziggy/bambuddy/pull/1246) by @MiguelAngelLV) — Adds a copy button (`Copy` icon) next to the existing edit button on every spool in the inventory page across all three views (table row, card, grouped table inner row). Clicking it opens the existing `SpoolFormModal` pre-filled with every field from the source spool — material, brand, color, slicer preset, label/core/cost, K-profiles, all of it — except `weight_used` which is reset to 0 (since the new spool starts full) and the RFID identity fields (`tag_uid`, `tray_uuid`, `tag_type`, `data_origin`) which aren't part of the form payload anyway, so the new spool is its own physical roll. Save calls `api.createSpool` (or `api.createSpoolmanInventorySpool` in Spoolman mode — both inherit the dispatch routing for free). Closes the long-running gap where users with many near-identical spools (e.g. five 1 kg PETG-CF rolls bought in a single order) had to re-enter every field from scratch on each one. **Implementation shape:** `SpoolFormModalProps.mode: 'create' | 'edit' | 'copy'` (exported as `SpoolFormMode`) replaces the previous `isEditing = !!spool` heuristic — every existing call site in `InventoryPage.tsx` was updated to pass the explicit mode, and the modal's title / submit-button label / weight-reset gate / submit-route branching all key on `mode` directly. The `onCopy` callback is optional on `SpoolCard`, `SpoolTableRow`, and `SpoolTableGroup` (matches the existing `onPrintLabel?` pattern), so the button is conditionally rendered and other consumers of those subcomponents don't get a copy affordance forced on them. Card-view and table-row buttons stop click propagation so clicking copy doesn't also fire the parent row's edit handler. **Quick Add interaction:** the Quick Add toggle is gated `mode === 'create'` (was `!isEditing`), so it stays out of copy mode — otherwise a user could enable Quick Add and bump quantity to N under the singular "Copy Spool" title and silently bulk-create N copies via `bulkCreateMutation`. **i18n:** new `inventory.copySpool` key across all 8 locales (en + de translated, fr/it/ja/pt-BR/zh-CN/zh-TW seeded with English fallback per project flow). **Tests:** 3 new in `SpoolFormModal.test.tsx` (`SpoolFormModal copy mode` describe block — title shows "Copy Spool", save calls `createSpool` not `updateSpool`, `weight_used` reset to 0 in the create payload when copying a spool with non-zero usage), 2 new in `InventoryPageCopyButton.test.tsx` (table-row copy button click → "Copy Spool" heading, cards-view copy button click → same heading after switching view modes) — guards against the three call sites drifting apart. Existing `SpoolFormBulk.test.tsx` and `SpoolFormModal.test.tsx` renders that omitted the `mode` prop were updated with the explicit `mode="create"` so the tightened Quick Add gate doesn't hide the toggle from them. Both `InventoryPageCopyButton.test.tsx` and `InventoryPageDeepLink.test.tsx` gained MSW handlers for the modal's open-time fetches (`/api/v1/cloud/status`, `/api/v1/cloud/local-presets`, `/api/v1/cloud/builtin-filaments`, `/api/v1/inventory/color-catalog`, `/api/v1/inventory/spool-catalog`, `/api/v1/printers/`) — without them MSW passes through to the real network, ECONNREFUSEs, and the rejected fetch resolves after the test environment is torn down, surfacing as a flaky "window is not defined" unhandled rejection in the modal's `setLoadingCloudPresets(false)` finally block (pre-existing flake hit ~1 in 3 full-suite runs at PR head).
|
||||
|
||||
### Fixed
|
||||
- **Virtual Printer wedged the slicer at "Downloading...(0%)" when a user clicked Print (instead of Send) against a non-proxy-mode VP, and blocked the next dispatch with "The printer is busy with another print job"** ([#1280](https://github.com/maziggy/bambuddy/issues/1280), reported by @kleinwareio) — Bambuddy's VP supports two distinct dispatch flows from the slicer: **Send** (file upload only — the path queue / immediate / review modes are designed for) and **Print** (file upload + start-print, intended for proxy mode where there's a real printer behind the VP). The reporter's setup was queue mode but they clicked Print, which is unsupported there. The user-facing symptom was wedging instead of a clean error: the FTP upload completed, the file landed in Bambuddy's queue, but Orca's UI froze at `Downloading...(0%)` and the next attempt was blocked. **Cause:** the VP's simulated state machine, in `backend/app/services/virtual_printer/manager.py::on_file_received`, jumped `PREPARE → IDLE` directly after the FTP upload completed. The Send flow doesn't watch the post-upload state, so Send users never noticed. The Print flow watches the gcode_state cycle expecting `PREPARE → RUNNING → FINISH` and only releases its in-flight-job lock when it sees `FINISH` (or `FAILED`). Going `PREPARE → IDLE` looks to the Print-flow slicer like "printer abandoned my job without confirming completion" → UI keeps the prior job pinned → next dispatch is blocked. `gcode_file_prepare_percent` also stayed at `"0"` for the whole upload window, which is why Orca's "Downloading X%" progress bar never advanced. **Fix:** `on_file_received` now transitions `PREPARE → FINISH` with `prepare_percent="100"` and the just-completed filename. The VP's 1-Hz periodic status push (`mqtt_server.py:363`) broadcasts the new state to every connected slicer within a second, so Orca clears its lock and the next dispatch goes through. The transition is gated to `.3mf` uploads only — auxiliary uploads (printer-side `.gcode` blobs etc.) leave the visible state alone. Treats Print and Send identically in non-proxy modes — Print is now silently handled as "file received, treat as completed" instead of wedging the slicer. Send remains a no-op behavior change because Send doesn't watch the post-upload state. **Tests:** 2 new tests in `backend/tests/unit/services/test_virtual_printer.py` pin (1) the FINISH transition with the correct filename + prepare_percent="100", and (2) the non-3MF guard. Affects every VP mode that isn't proxy (`immediate`, `print_queue`, `review`) on every slicer using the Print flow (BambuStudio + OrcaSlicer in LAN-mode).
|
||||
|
||||
- **External-spool filament selection silently rolled back: every "Generic PLA" / preset change for the external slot looked applied in the UI but failed on the printer, and the next print threw "no mapping"** ([#1279](https://github.com/maziggy/bambuddy/issues/1279), reported by @kleinwareio) — Repro: P1S, no AMS, vt_tray active. User picks any filament for the external slot via Bambuddy. The UI looked normal, but the printer's MQTT response was `{"command":"ams_filament_setting", "result":"fail", "reason":"error string"}`. The companion `extrusion_cali_sel` command succeeded, so the K-profile stuck but the filament *identity* didn't — and the next print therefore had nothing to map to. **Cause:** `backend/app/services/bambu_mqtt.py::ams_set_filament_setting` encoded the single-external-spool case as `{ams_id: 255, tray_id: 0, slot_id: 0}`. The "LOCAL `tray_id = 0`" comment in the code was a misread of the printer's *response* shape (the printer echoes `tray_id: 0` as the slot-within-virtual-unit, not the slot index used in the *request*). **Verification:** captured BambuStudio → X1C `ams_filament_setting` publish via `mosquitto`-compatible paho-mqtt subscriber on the same broker, BambuStudio set the external slot to a PLA preset, the published REQ was `{ams_id: 255, tray_id: 254, slot_id: 0, tray_info_idx: "P4d64437", tray_color: "F72323FF", tray_type: "PLA", ...}` and the printer's REP returned `result: "success"`. The on-wire convention for `ams_filament_setting` on the external spool is therefore the *global* tray index (`tray_id: 254`), not a local slot number (`tray_id: 0`). **Fix:** `mqtt_tray_id = 254` for the single-external branch in both `ams_set_filament_setting` and `reset_ams_slot` (which shares the convention). The dual-external branch (H2D, `len(vt_tray) > 1`) was **not** in the captured exchange and is left at `mqtt_tray_id = 0` until a Studio → H2D capture confirms the correct value — a regression test pins the current dual-external encoding so any future change to that branch surfaces immediately. **Affected printers:** every printer whose MQTT push reports `vt_tray` as a single-element list — i.e. one external slot. That covers all single-nozzle Bambu printers (P1P, P1S, A1, A1 mini, X1C, X1E) plus dual-nozzle models that use a single external feed (X2D). **Not affected** by this change: H2D / H2C / H2S, which expose two external slots and go through a separate `len(vt_tray) > 1` branch. That branch is preserved at its existing `mqtt_tray_id = 0` encoding because the captured exchange did not cover it; if the same misencoding turns out to affect dual-external too, a Studio → H2D capture will surface the right values and a follow-up patch will land. **Known asymmetry not touched in this PR:** the inline `ams_filament_setting` built by `_probe_developer_mode` (`bambu_mqtt.py:2971-2985`) still hardcodes `tray_id=0`. The probe is robust to this — its detection logic only matches `reason: "verify failed"` so it correctly identifies dev-mode regardless of whether the command itself succeeds — but the two builders should be unified in a follow-up. **Tests:** 5 new tests in `backend/tests/unit/services/test_bambu_mqtt.py::TestAmsFilamentSettingExternalSpoolEncoding` pin the X1C/P1S/A1 single-external fix, `reset_ams_slot` symmetry, regular AMS slot encoding unchanged, AMS-HT slot encoding unchanged, and the explicitly-unverified dual-external encoding (so any future change to the dual branch surfaces in diff review).
|
||||
|
||||
- **Scan For Timelapse matched the wrong video when an older print's filename happened to land near a later archive's completion** ([#1278](https://github.com/maziggy/bambuddy/issues/1278), reported by @1000Delta) — Repro: P2S in LAN-Only mode (no NTP, so printer clock is drifted +8h from UTC), two prints on the same day. Archive 1 correctly attached `video_2026-05-08_09-41-29.mp4`. Archive 2 (started at 16:39:09 UTC, expected `video_2026-05-09_00-42-42.mp4`) reused Archive 1's video with a misleading `diff: 0:02:19`. **Cause:** `scan_timelapse`'s Strategy 2 matcher in `backend/app/api/routes/archives.py` had two compounding flaws. (1) It compared the filename timestamp against both `archive.started_at` **and** `archive.completed_at` with a 48 h tolerance — but the filename always represents the print's START time, never its end, so the end-time branch was a semantic mistake whose only effect was creating false positives. For Archive 2, the stale filename `09:41:29` shifted by hypothesis offset `-8h` → `17:41:29`, which happened to fall ~2 minutes before Archive 2's completion → "diff" 2m19s won. (2) The matcher tried seven hypothesised offsets `[0, ±1, ±7, ±8]`, which densely covers a wide span of the day. Even with the end-time branch removed, the wrong video at offset `-7` lands at `16:41:29` → 2m20s from Archive 2's start, beating the correct video's 3m33s at offset `+8`. **Fix:** extracted Strategy 2 into a pure `_match_timelapse_by_timestamp(video_files, archive_start)` helper that (a) only compares against print **start** time (end-time evidence is handled separately by Strategy 3 via file mtime, which actually does reflect when writing finished), and (b) requires the best (video, offset) pair to beat the next-best pair from a *different* video by at least 15 minutes. When the top two candidates from different videos are too close to call, the helper returns `None` so the route surfaces the existing `available_files` list and the frontend's manual-selection dialog kicks in — which is the fallback the reporter explicitly asked for ("at a minimum, we should support that can fall back to letting the user manually select"). Wide offset support is preserved so EU / JST / AEST users (offsets +1, +7, +9, +10, etc.) still get auto-match when there's no ambiguity. **Tests:** 17 new tests in `backend/tests/unit/test_timelapse_match.py` pin the bug case (`test_issue_1278_archive2_refuses_to_auto_pick_ambiguous`, `test_issue_1278_archive1_still_matches_unambiguously`), the resolution path once the stale video is cleaned up (`test_archive2_resolves_when_stale_video_removed`), each of the 7 supported offsets via parametrize, and the supporting invariants (no `started_at` → `None`, non-timestamp filenames are skipped, same-video different-offset is not ambiguous, well-separated different videos still auto-pick). **Known UX gap not in this PR:** if the matcher auto-picks a wrong match, the user must delete the attached timelapse first before re-scanning — `scan_timelapse` short-circuits with `status: "exists"` when `timelapse_path` is already set. Adding a force-rescan or "wrong match, pick from candidates" affordance is a separate change.
|
||||
|
||||
@@ -217,9 +217,18 @@ class VirtualPrinterInstance:
|
||||
else:
|
||||
await self._queue_file(file_path, source_ip)
|
||||
|
||||
# Reset MQTT status back to IDLE
|
||||
# Signal job completion to the slicer. Send-flow slicers don't watch the
|
||||
# post-upload state and would be happy with anything; the Print flow
|
||||
# (intended for proxy-mode VPs, but users sometimes click it against
|
||||
# queue/immediate/review modes too — #1280) watches the gcode_state
|
||||
# cycle and only releases its in-flight-job lock when it sees FINISH.
|
||||
# Going PREPARE → IDLE wedges the slicer's UI at "Downloading...(0%)"
|
||||
# and blocks the next dispatch with "busy with another print job".
|
||||
# PREPARE → FINISH satisfies both flows. prepare_percent=100 also
|
||||
# unfreezes the slicer's "Downloading X%" progress bar which it ticks
|
||||
# against the same field during the upload window.
|
||||
if self._mqtt and file_path.suffix.lower() == ".3mf":
|
||||
self._mqtt.set_gcode_state("IDLE")
|
||||
self._mqtt.set_gcode_state("FINISH", filename=file_path.name, prepare_percent="100")
|
||||
|
||||
async def on_print_command(self, filename: str, data: dict) -> None:
|
||||
"""Handle print command from MQTT."""
|
||||
|
||||
@@ -171,6 +171,41 @@ class TestVirtualPrinterInstance:
|
||||
|
||||
mock_archive.assert_called_once_with(file_path, "192.168.1.100")
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_on_file_received_signals_FINISH_to_slicer(self, instance):
|
||||
"""Regression #1280: when a slicer's Print flow uploads to a non-proxy VP,
|
||||
the VP must transition gcode_state PREPARE → FINISH so the slicer's
|
||||
in-flight-job lock releases. Going PREPARE → IDLE wedges Orca at
|
||||
"Downloading...(0%)" and blocks the next dispatch with "busy with
|
||||
another print job".
|
||||
|
||||
Send-flow slicers don't watch the post-upload state, so this is a
|
||||
no-op behavior change for them.
|
||||
"""
|
||||
instance.mode = "immediate"
|
||||
instance._mqtt = MagicMock()
|
||||
instance._mqtt.set_gcode_state = MagicMock()
|
||||
file_path = Path("/tmp/test.3mf") # nosec B108
|
||||
|
||||
with patch.object(instance, "_archive_file", new_callable=AsyncMock):
|
||||
await instance.on_file_received(file_path, "192.168.1.100")
|
||||
|
||||
instance._mqtt.set_gcode_state.assert_called_once_with("FINISH", filename="test.3mf", prepare_percent="100")
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_on_file_received_non_3mf_does_not_touch_state(self, instance):
|
||||
"""Non-3MF uploads (e.g., a job's auxiliary files) must not transition
|
||||
the visible state — the slicer is only tracking the .3mf upload."""
|
||||
instance.mode = "immediate"
|
||||
instance._mqtt = MagicMock()
|
||||
instance._mqtt.set_gcode_state = MagicMock()
|
||||
file_path = Path("/tmp/test.gcode") # nosec B108
|
||||
|
||||
with patch.object(instance, "_archive_file", new_callable=AsyncMock):
|
||||
await instance.on_file_received(file_path, "192.168.1.100")
|
||||
|
||||
instance._mqtt.set_gcode_state.assert_not_called()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_archive_file_skips_non_3mf(self, instance):
|
||||
"""Verify non-3MF files are skipped and cleaned up."""
|
||||
|
||||
Reference in New Issue
Block a user