From 82c90c63874a30be530605852912376487e9a8ef Mon Sep 17 00:00:00 2001 From: maziggy Date: Wed, 13 May 2026 09:57:20 +0200 Subject: [PATCH] fix(mqtt): skip print-start fire on first RUNNING after Bambuddy startup (#1304) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restarting Bambuddy mid-print misfired the plate-check + archive flow. The is_new_print guard treated _previous_gcode_state=None → RUNNING as a transition, but None just means we haven't seen any prior state yet — catch-up from a printer that was already running, not a fresh start. Add `_previous_gcode_state is not None` to the guard. _was_running still flips on unconditionally, so completion detection is unchanged. 3 tests that asserted the buggy behavior now seed an explicit prior state; new regression test pins the contract for the reporter's exact scenario. --- CHANGELOG.md | 2 + backend/app/services/bambu_mqtt.py | 1 + .../tests/unit/services/test_bambu_mqtt.py | 51 +++++++++++++++++++ 3 files changed, 54 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 259072b1d..a5d2287a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ All notable changes to Bambuddy will be documented in this file. - **Spoolman weight tracking now uses per-print grams for all spools, matching the internal Filament Inventory** ([#1119](https://github.com/maziggy/bambuddy/issues/1119), reported by @Moskito99) — Spoolman previously had two mutually-exclusive weight paths: AMS remain%×tray_weight auto-sync (default; only worked for Bambu Lab spools with valid RFID tray_weight) and per-print 3MF-grams tracking (only enabled when "Disable AMS Weight Sync" was toggled on). Non-BL spools without RFID fell through both paths — AMS auto-sync had no tray_weight to multiply, and the inventory_remaining fallback was wiped because activating Spoolman deletes the internal `spool_assignment` table — so Spoolman never saw a weight update for them. The internal Filament Inventory has no such gap: it always uses per-print 3MF grams as the primary path with AMS-remain% delta as fallback, and it works for every spool type. Spoolman now does the same: per-print tracking runs whenever Spoolman is enabled and is the only writer of `remaining_weight`. AMS auto-sync continues to maintain spool metadata and slot assignments but no longer touches weight (eliminating the double-count that would otherwise occur for BL spools with both paths active). `store_print_data` ([`spoolman_tracking.py:159`](backend/app/services/spoolman_tracking.py)) had its `disable_weight_sync` early-return removed; the three `sync_ams_tray` callsites (`main.py:1450` auto-sync, `spoolman.py:318` per-printer manual, `spoolman.py:517` sync-all) now hard-code `disable_weight_sync=True`. The `spoolman_disable_weight_sync` setting is now deprecated and a no-op — kept in the DB/UI for backwards compat. Behavioral consequence for existing users on the default flag (False): live AMS-based remaining_weight updates between prints stop happening; weight updates now arrive once per print completion with 3MF gram precision. Regression test in `test_spoolman_tracking.py::test_stores_tracking_when_disable_weight_sync_is_false` proves the early-return is gone. ### Fixed +- **Restarting Bambuddy mid-print triggered plate-check pause + duplicate archive** ([#1304](https://github.com/maziggy/bambuddy/issues/1304), reported by @kleinwareio) — When a P1S print was in progress and the user updated the Bambuddy container (`latest` → `daily` in the report, but the same path fires on any restart), Bambuddy paused the live print with an "Object detected on build plate" warning AND re-archived the in-progress file as a duplicate. Root cause: the print-start detector at `backend/app/services/bambu_mqtt.py:2780` gated on `self._previous_gcode_state != "RUNNING"`, which is true whether we just saw IDLE→RUNNING (a real print start) OR we just constructed a fresh BambuMQTTClient and `_previous_gcode_state` is still its initial `None` (catch-up push from a printer already running). The fresh-client case fired `on_print_start`, which downstream ran the plate-detection-and-pause flow at `main.py` AND the FTP-download-and-archive flow — exactly the two symptoms in the bug report. Fix: added `self._previous_gcode_state is not None` to the `is_new_print` guard, so the first push from the printer in a new process lifetime never counts as a state transition into RUNNING. `_was_running` still flips to `True` via the unconditional "Track RUNNING state" block at `bambu_mqtt.py:2795`, so print-completion detection keeps working — only the start callback is suppressed. Three existing tests that asserted on the old (buggy) behavior were updated to seed `_previous_gcode_state = "IDLE"` first, matching the realistic lifecycle of a print actually starting (Bambuddy has been observing IDLE/FINISH before RUNNING); they now exercise the correct path. New regression test `test_first_running_push_after_bambuddy_restart_does_not_fire_print_start` pins the contract for the reporter's exact scenario — and asserts that `_was_running` still becomes True so completion still fires when the print ends. The `is_file_change` branch was unaffected (it already required `_previous_gcode_file is not None`, so restart-catch-up never reached it anyway). + - **Create User form rejected weak passwords with an opaque "HTTP 422" toast** ([#1303](https://github.com/maziggy/bambuddy/issues/1303), reported by @TrickShotMLG02) — Three independent UX gaps stacked on top of each other. **(1) Discoverability**: the Create User and Edit User modals showed no hint about the backend's password complexity requirements (`min 8 chars` + uppercase + lowercase + digit + special character; enforced in `backend/app/schemas/auth.py:_validate_password_complexity`). Reporter typed an 8-character all-digits password and had no way to know why it failed. **(2) Validation mismatch**: the frontend's pre-submit check at `SettingsPage.tsx` was only `password.length < 6`, accepting passwords the backend would reject — every weak password got bounced after the round-trip instead of getting blocked locally. **(3) Error display fragility**: when the backend returned a 422 with a Pydantic detail array, the API client's error parser at `frontend/src/api/client.ts:107` could fall through to the bare `HTTP ${status}` fallback if the mapped/filtered detail array ended up empty after stripping the `"Value error, "` prefix — masking the real reason as just "HTTP 422". Fixes: (1) added a `passwordRequirements` helper line under both password inputs in Create User / Edit User; (2) extracted `checkPasswordComplexity` into `frontend/src/utils/password.ts`, called from `handleCreateUser` and `handleUpdateUser` before the API request — it returns the same FIRST failing rule the backend's validator would have flagged (uppercase before lowercase before digit before special, matching `_validate_password_complexity`'s order — fixing one rule shouldn't immediately trip a different message), and the submit button is disabled until all rules pass; (3) the API client now falls back to `JSON.stringify(detail)` when the mapped array is empty, so a malformed but non-empty 422 detail surfaces SOMETHING informative instead of a bare status code. New translation keys `settings.passwordRequirements`, `settings.toast.passwordNeeds{Uppercase, Lowercase, Digit, Special}`, plus the existing `passwordTooShort` text updated from "6 characters" to "8 characters". English + German fully translated (German reporter's locale); FR/IT/PT-BR translated using straightforward equivalents; JA/ZH-CN/ZH-TW seeded with English for the new complexity messages (existing project flow for new strings). 7 new unit tests in `frontend/src/__tests__/utils/password.test.ts` pin the validator's contract, including the reporter's exact `"12345678"` input which now produces a local "Password must contain at least one uppercase letter" toast instead of a 422 round-trip. - **External NAS scan hung forever and never committed subdirectories** ([#1299](https://github.com/maziggy/bambuddy/issues/1299), reported by @joeferrante) — Linking an external mount with ~1200 subdirectories caused the "Link External Folder" modal to spin until the FE gave up, after which the mount appeared in the sidebar but with no subdirectories, and subsequent scans had no effect either. The reporter's support bundle pinpointed two compounding problems. **(1) `TypeError: unsupported operand type(s) for /: 'str' and 'str'` on every STL** — 1,606 instances in the log. `generate_stl_thumbnail` at `stl_thumbnail.py:119` does `thumbnails_dir / thumb_filename`, which requires a `Path`, but the external-scan call site at `library.py:1256` passed both arguments as `str` (`generate_stl_thumbnail(str(filepath), str(thumb_dir))`). Every STL crashed inside the `try/except` and got logged at WARNING level — visible spam but more importantly wasted work (`trimesh.load()` and matplotlib setup ran before the failing division). Fix: defensive `Path()` coerce at the top of `generate_stl_thumbnail` so the function works regardless of how callers pass args. Regression test `test_string_arguments_accepted_without_typeerror` pins the contract. **(2) Scan ran STL thumbnail generation synchronously inside the HTTP request** — even after fix (1), `trimesh.load()` + matplotlib render is 1–5 seconds per STL; on a NAS with thousands of STLs that's hours of work blocking the modal. Frontend would time out, user would refresh, the HTTP request would be cancelled, `db.commit()` at `library.py:1331` would never run, and no folder/file rows would be committed — which is exactly why "subsequent scans have no effect" (each retry started from scratch and hit the same wall). Fix: scan now defers STL thumbnails to a background task. After `db.commit()`, the route spawns `asyncio.create_task(_backfill_external_stl_thumbnails(folder_ids))` with the full set of folder IDs from `folder_cache.values()` (covers both pre-existing subfolders AND the ones created during this scan — `all_folder_ids` is snapshotted before the walk and would have missed the new ones), then returns immediately. The background task opens its own `async_session`, walks every STL file with `thumbnail_path IS NULL` in the linked folder tree, generates each thumbnail, and commits per-file so a server restart mid-run only loses the in-flight thumbnail. Survives FE refresh because the task lives in the FastAPI event loop, not the request scope. The reporter's smaller mount (`/mnt/NAS_3d_files/3mf_Files`, 4 subdirectories) used to work because it completed inside the FE timeout window — with this fix, the 1200-subdir parent mount completes equally fast and thumbnails fill in over the following minutes. **Auto-scan after create unchanged**: `FileManagerPage.tsx:1147-1151` still calls `scanExternalFolder` immediately after `createExternalFolder`, which is correct UX — what changed is that the scan response now arrives in seconds instead of timing out. diff --git a/backend/app/services/bambu_mqtt.py b/backend/app/services/bambu_mqtt.py index 5de984ac3..7f8fb3296 100644 --- a/backend/app/services/bambu_mqtt.py +++ b/backend/app/services/bambu_mqtt.py @@ -2779,6 +2779,7 @@ class BambuMQTTClient: current_file = self.state.gcode_file or self.state.current_print is_new_print = ( self.state.state == "RUNNING" + and self._previous_gcode_state is not None # #1304: skip on first push after Bambuddy startup and self._previous_gcode_state != "RUNNING" and current_file and not self._was_running # Prevent duplicates when resuming from PAUSE diff --git a/backend/tests/unit/services/test_bambu_mqtt.py b/backend/tests/unit/services/test_bambu_mqtt.py index 1bcb69917..952e03aeb 100644 --- a/backend/tests/unit/services/test_bambu_mqtt.py +++ b/backend/tests/unit/services/test_bambu_mqtt.py @@ -338,6 +338,9 @@ class TestRealisticMessageFlow: mqtt_client.on_print_start = on_start mqtt_client.on_print_complete = on_complete + # Seed a prior state so the first RUNNING push is treated as a real + # state transition rather than a Bambuddy-restart catch-up (#1304). + mqtt_client._previous_gcode_state = "IDLE" # 1. Print starts with timelapse mqtt_client._process_message( @@ -1603,6 +1606,9 @@ class TestRequestTopicAmsMapping: mqtt_client.on_print_start = on_start mqtt_client._captured_ams_mapping = [0, 4, -1, -1] + # Seed a prior state so the first RUNNING push is treated as a real + # state transition rather than a Bambuddy-restart catch-up (#1304). + mqtt_client._previous_gcode_state = "IDLE" # Trigger print start mqtt_client._process_message( @@ -1625,6 +1631,9 @@ class TestRequestTopicAmsMapping: start_data.update(data) mqtt_client.on_print_start = on_start + # Seed a prior state so the first RUNNING push is treated as a real + # state transition rather than a Bambuddy-restart catch-up (#1304). + mqtt_client._previous_gcode_state = "IDLE" mqtt_client._process_message( { @@ -1639,6 +1648,48 @@ class TestRequestTopicAmsMapping: assert "ams_mapping" in start_data assert start_data["ams_mapping"] is None + def test_first_running_push_after_bambuddy_restart_does_not_fire_print_start(self, mqtt_client): + """Regression for #1304: Bambuddy restart mid-print misfired plate check + archive. + + When Bambuddy restarts while a print is already in progress, the freshly + constructed BambuMQTTClient has `_previous_gcode_state = None`. The first + push_status the printer sends reports `gcode_state: RUNNING`. Before the + fix, the (None → RUNNING) transition satisfied is_new_print's guard and + fired on_print_start, which then ran plate detection (objects on plate → + paused the live print) AND re-archived the file (duplicate archive). + + With the fix in place the on_print_start callback must NOT be called for + this catch-up push, but `_was_running` still tracks the print so + completion detection works the same way as before. + """ + start_data = {} + + def on_start(data): + start_data.update(data) + + mqtt_client.on_print_start = on_start + # Explicit: this simulates a fresh Bambuddy process attaching to a + # printer that's already in the middle of a print. + mqtt_client._previous_gcode_state = None + mqtt_client._was_running = False + + mqtt_client._process_message( + { + "print": { + "gcode_state": "RUNNING", + "gcode_file": "/data/Metadata/big_print.gcode", + "subtask_name": "big_print", + } + } + ) + + assert start_data == {}, "on_print_start must not fire on Bambuddy-restart catch-up" + # Completion detection still needs to know we're tracking a running job. + assert mqtt_client._was_running is True + # And the state-update bookkeeping ran so the NEXT push won't keep + # treating the first RUNNING as fresh. + assert mqtt_client._previous_gcode_state == "RUNNING" + def test_print_complete_callback_includes_ams_mapping(self, mqtt_client): """on_print_complete callback data includes captured ams_mapping.""" complete_data = {}