diff --git a/CHANGELOG.md b/CHANGELOG.md index c0c3efd6a..fc8ca918c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,8 @@ All notable changes to Bambuddy will be documented in this file. - **Per-printer Maintenance Mode toggle (#1476, requested by @IndividualGhost1905 / Ferdi SEVER)** — Operator-flipped "out of service" state per printer, surfaced as a wrench icon + amber pill on the card and a checkbox in the Edit Printer dialog. Requested for three real-world scenarios that all share the same shape: (1) parallel Bambuddy installs (dev + prod, primary + warm spare) where the printer rejects concurrent MQTT clients except one, leaving the others in a flicker-online state burning CPU and network; (2) printers under repair / awaiting spare parts that shouldn't accept queue jobs but should remain visible on the dashboard so they aren't forgotten; (3) temporary suspension during maintenance work. **What was already there, what was missing.** The backend field `Printer.is_active: bool` has shipped since the initial Bambuddy release — toggling it via `PATCH /printers/{id}` already disconnects MQTT (`printer_manager.disconnect_printer` at `printers.py:366`), stops the printer from being eligible for queue dispatch (`print_scheduler.py:520, 1588`, `print_queue.py:383`), excludes it from model-based filament lookups (`printers.py:197`), excludes it from metrics + diagnostic snapshots + scheduled-backup runs (`metrics.py:105`, `diagnostic_snapshot.py:126`, `github_backup.py:333`, `maintenance.py:457`), and is already honoured by PrinterSelector (filtered with a "show inactive" override, greyed + "(inactive)" label when shown). All three of Ferdi's use cases were structurally supported by `is_active` from day one. **The missing piece was UI exposure.** `grep is_active` on `PrintersPage.tsx` returned zero hits — no menu item, no edit field, no toggle. The only way to flip it was a direct API call. This change adds the surfaces that should have been there all along. **Card UI — replacement, not addition.** Per Ferdi-conversation feedback, the maintenance state replaces the print-status / cover-image container rather than stacking above it, so card heights stay identical across the grid: in expanded mode the same `` header renders an amber panel (wrench icon + "In Maintenance" + subtitle + Exit button) where the cover + progress would normally be; in compact mode a single amber pill replaces the progress bar. The header connection pill is also swapped — instead of the red "Offline" pill (which would be misleading because the disconnect is deliberate) the card shows an amber "Maintenance" pill, and the "Run Diagnostic" CTA is suppressed (that's reserved for involuntary offline triage). HMS / Queue / Firmware status pills are still gated by `status?.connected` so they fall away naturally with the MQTT disconnect. **Three entry points.** (1) Printer card three-dot overflow menu — `Enter maintenance mode` / `Exit maintenance mode` with a wrench icon, adjacent to the Edit and Reconnect actions. (2) Exit button inside the in-card amber panel, so a user noticing the card from across the room can flip back without opening the menu. (3) Checkbox in the EditPrinterModal — `Maintenance mode` with the same subtitle as the help line, so the toggle is discoverable from the edit dialog too (the checkbox is the inverse of `is_active` because the user-facing concept is "is this in maintenance" not "is it active"). **Mid-print safety prompt.** Entering maintenance mode on a printer in `RUNNING` / `PAUSE` state triggers a confirmation dialog before the toggle fires — disconnecting MQTT mid-print stops progress tracking + completion notifications for the in-flight job, which is usually NOT what the operator wants (they probably meant "after this print finishes"). Idle / FINISH / FAILED states skip the dialog and toggle directly. **What this does NOT change.** No backend change (`is_active` was already wired everywhere); no new permission (uses existing `printers:update`); no behaviour change for any other consumer (queue dispatch, scheduler, metrics, picker, backup — all already honoured `is_active`). The card stays visible on the Printers page (greyed temps/controls/fans below the amber banner) so the printer doesn't disappear from the operator's mental map — Ferdi explicitly wanted to remember it's there. Doesn't auto-pause Smart Plug logic or notification providers (would be a sensible follow-up if Ferdi asks; out of scope here to keep the diff bounded to "expose the existing gate"). The scheduled-maintenance dashboard at `/maintenance` (interval-tracked rod-cleaning / lube / belt tasks via the existing `MaintenanceHistory` and `PrinterMaintenance` models) is conceptually adjacent but operationally distinct — the dashboard tracks "this printer is due for cleaning"; Maintenance Mode tracks "this printer is currently out of service." A future "perform maintenance task → optionally enter maintenance mode while you do it" link is the natural connection but isn't wired here. **i18n.** Twelve new keys under `printers.maintenance.*` (title / subtitle / pillLabel / exitButton / menuEnter / menuExit / toastEntered / toastExited / confirmMidPrintTitle / confirmMidPrintMessage / editFieldLabel / editFieldHelp) — real translations in all 11 locales (de / en / es / fr / it / ja / ko / pt-BR / tr / zh-CN / zh-TW), parity 5228 leaves per locale, no English fallback. **Tests.** 4 new cases in `PrintersPage.test.tsx::'maintenance mode (#1476)'`: amber status panel renders with Exit button (and the regular "No active job" / "Ready to print" copy is absent — confirms the swap, not a stacked render); header pill swaps to amber Maintenance and the diagnostic CTA is suppressed; clicking Exit issues a `PATCH /printers/{id}` with `is_active: true`; active printers never show the maintenance panel. Existing test fixture (`mockPrinters`) got an explicit `is_active: true` to keep the existing 56 tests green on the new render path. **Type:** `PrinterCreate.is_active?: boolean` added to the TypeScript surface so the field flows cleanly through the existing `api.updatePrinter` helper. **Build + checks.** Full PrintersPage vitest 60/60 green; `npm run build` clean; ESLint clean; i18n parity 5228 × 11 locales green. ### Fixed +- **Print-complete notification dropped the finish photo when the FINISH-state fallback fired (#1790, reported by @needo37)** — On the FINISH-state fallback path (`bambu_mqtt.py:3258-3297`, used when stage-22 doesn't fire — cancel, external-spool-only, HMS halt, firmware variants that skip the unload phase), `on_finish_photo_moment` and `on_print_complete` were dispatched as two **independent** asyncio tasks back-to-back from the same MQTT handler. The producer (`on_finish_photo_moment`) ran the RTSP grab (15s timeout) and stored the JPEG into `_stage22_finish_frames[printer_id]` only after the grab returned; the consumer (`_background_finish_photo`, spawned by `on_print_complete`) read the cache with a single `pop()` at `main.py:4681` — no wait, no retry. On the stage-22 happy path the producer fires seconds before FINISH-state arrives so the race is invisible; on the FINISH-state fallback the gap collapses to ~0 and the consumer always wins the empty pop. After the empty pop, the fallback chain called `capture_finish_photo()` at `main.py:4739` — but the producer's RTSP grab was still in flight against the same printer, and Bambu printers allow exactly one RTSP client at a time. The consumer's grab timed out at the camera service's 30s ceiling. Reporter's log shows it exactly: `[FINISH-PHOTO-MOMENT] captured RTSP frame (394037 bytes)` at 05:31:20, then `[PHOTO-NOTIFY] Photo task returned: None` at 05:31:49 — 30s after `[PHOTO-BG] Starting`. A 394 KB frame was captured, the notification went text-only. **Why this only surfaced after #1721:** before #1721, Bambuddy force-enabled timelapse at dispatch so a video always existed and the finish photo was extracted from its last frame regardless of timing. #1721 removed the force-on (it was causing per-layer nozzle parking on Smooth-mode slicer profiles) and made the racy stage-22 cache the only good framing source. For timelapse-off prints completing via the FINISH-state fallback, there was no resilient source left. **Fix.** New per-printer `_stage22_finish_in_flight: dict[int, asyncio.Event]` synchronizes producer→consumer. The producer registers an `asyncio.Event` BEFORE its first `await` (so the consumer always sees it the moment it polls — registration is purely synchronous before any await yields control), sets the event in a `finally` block on EVERY exit path (success, no-frame, setting-disabled early return, exception), and the consumer awaits the event with `asyncio.wait_for(event.wait(), timeout=20.0)` before reading the cache. The 20s ceiling is sized against the producer's 15s RTSP timeout — bounded headroom, can't hang notifications. The consumer pops the dict entry when it starts waiting so cleanup is a single side; the producer's `set()` works on a local ref. Side-effect win: because the consumer is blocked behind the producer's completion, the consumer's own RTSP fallback can no longer collide with the producer's in-flight grab — Failure 2 (concurrent RTSP timeout) is closed alongside Failure 1 (cache race) by the same change. **What this does NOT change.** The timelapse path (`timelapse_was_active=True`) returns before registering the event — the consumer takes the `_capture_finish_photo_from_timelapse` branch and never waits; no regression. Aborted / failed prints don't dispatch `on_finish_photo_moment` at all (status="completed" gate at `bambu_mqtt.py:3258`) — no event registered, consumer behaves as today. External-camera printers (`external_camera_enabled`) and printers with a live stream open in the UI (buffered RTSP frame) are unaffected — the producer still uses those non-contended sources first. No change to `camera.py` lock semantics. **Tests.** 7 new cases in `test_finish_photo_moment_sync.py` pin every limb of the contract: event is registered before the first await (uses a slow-capture stub to observe the dict mid-run), event is set after successful capture, event is set when the producer captured no frame, event is set even when the capture function raises (the `finally` is load-bearing), event is NOT registered on the `timelapse_was_active=True` early-return, event IS set when the `capture_finish_photo` setting is disabled (the late early return — important so the consumer doesn't hang on a no-op producer), and an end-to-end producer/consumer pair finishes promptly with the cached frame visible to the consumer. Adjacent tests (`test_finish_photo_from_timelapse.py`, `test_reprint_clears_stale_timelapse.py`) still green. Ruff clean. + - **Archives drag-and-drop overlay stuck after cancel (#1510, reported by @maikolscripts)** — Cancelling a drag on the Archives page — by dragging back out of the browser window, releasing outside the page, or pressing Escape mid-drag — left the full-screen "Drop .3mf files here" overlay visible until the user refreshed. **Cause.** The old inline `handleDragLeave` only hid the overlay when `e.currentTarget === e.target` (i.e. the dragLeave event fired on the wrapper itself, not a child). That condition was structurally safe for crossing internal element boundaries but rarely held for the three cancel paths above — drag-out-of-window fires dragLeave with `target` at the nearest child to the cursor; Escape and drag-abort fire no leave event at all on the wrapper. **Fix.** Moved the page-wide drop handling into the new `usePageFileDrop` hook (also consumed by File Manager — see the linked Added entry). The hook checks `relatedTarget` containment instead of `currentTarget === target`, and adds document-level `drop` / `dragend` / `keydown(Escape)` listeners that only register while `isDraggingOver === true` so the cancel paths all reset uniformly. Three of the 13 new hook test cases pin the cancel paths explicitly so a future regression on any one of them fails its own case. Also moved the previously-hardcoded English "Drop .3mf files here" string in `ArchivesPage.tsx:3202` to the existing `archives.page.dropFilesHere` i18n key (which already had translations in all 11 locales) so the overlay localises correctly — same change of behaviour as `archives.releaseToUpload` already had. - **File Manager list-view column headers misaligned with their body cells** — Both the header row and each list row used the same `grid-cols-[auto_1fr_120px_100px_100px_100px_min-content]` template — looked correct at the CSS level — but the two `
`s were **sibling grids**, not a shared grid, so each computed `min-content` for the trailing actions column independently. The header's trailing column is an empty `
` → `min-content` resolved to 0; body rows had 4–7 action icons → `min-content` resolved to ~220px. With different trailing widths, the `1fr` Name column got a different amount of room in each grid, which pushed every fixed column to its right (`Uploaded By`, `Type`, `Size`, `Prints`) further right in the header than in the body. Visually the body cells looked **shifted left** of their column headers. **Fix.** Replaced the trailing `min-content` with a fixed `220px` in both the auth-enabled and auth-disabled grid templates (matching the comment that already documented the expected width of the 7-icon strip on sliced 3MFs). Updated the explanatory comment with the sibling-grid pitfall so the next person doesn't re-introduce it. No tests changed; the misalignment was purely visual (no DOM ordering / interaction changed), and the existing 51 FileManagerPage tests stay green. diff --git a/backend/app/main.py b/backend/app/main.py index 21418c673..5880d1cce 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -346,6 +346,15 @@ _active_prints: dict[tuple[int, str], int] = {} # nozzle parking on slicer profiles with Timelapse Type = Smooth). _stage22_finish_frames: dict[int, bytes] = {} +# #1790: per-printer producer-done event. Set by `on_finish_photo_moment` in its +# `finally` block (whether it captured a frame or not). The consumer in +# `_background_finish_photo` waits on it before reading `_stage22_finish_frames` +# so the FINISH-state fallback path — where moment and completion are dispatched +# back-to-back — doesn't race past the producer with an empty pop, and the +# consumer's RTSP fallback can't collide with the producer's still-in-flight RTSP +# grab (Bambu printers allow only one RTSP client at a time). +_stage22_finish_in_flight: dict[int, asyncio.Event] = {} + # Per-printer "connected" edge tracker. Used by `on_printer_status_change` # to fire `reconcile_stale_active_prints` exactly once per (re)connection # (#1542 follow-up — power-cycle ghost prints). The value is True after @@ -3735,6 +3744,14 @@ async def on_finish_photo_moment(printer_id: int, data: dict): ) return + # #1790: register the producer-done event BEFORE the first await so the + # consumer in `_background_finish_photo` — which is dispatched back-to-back + # with us on the FINISH-state fallback path — sees it as soon as it polls. + # The `finally` below guarantees `set()` runs on every exit, including + # early returns and exceptions, so the consumer's bounded wait can't hang. + producer_done = asyncio.Event() + _stage22_finish_in_flight[printer_id] = producer_done + try: async with async_session() as db: from backend.app.api.routes.settings import get_setting @@ -3807,6 +3824,11 @@ async def on_finish_photo_moment(printer_id: int, data: dict): printer_id, e, ) + finally: + # #1790: always unblock the consumer's bounded wait — whether we stored + # a frame, gave up, or hit an exception. Local ref means cleanup of the + # dict entry by the consumer doesn't affect signalling. + producer_done.set() async def on_print_complete(printer_id: int, data: dict): @@ -4678,6 +4700,22 @@ async def on_print_complete(printer_id: int, data: dict): # has the better framing instead of the post-bed-drop angle # the live-camera fallback below would give. if not photo_filename: + # #1790: on the FINISH-state fallback path the producer + # task is dispatched back-to-back with this consumer, so + # a bare pop would race past with an empty result and + # the RTSP fallback below would collide with the + # producer's still-in-flight grab (single-client RTSP + # on Bambu printers). Wait for the producer to finish + # or give up before touching the cache. + in_flight = _stage22_finish_in_flight.pop(printer_id, None) + if in_flight is not None: + try: + await asyncio.wait_for(in_flight.wait(), timeout=20.0) + except asyncio.TimeoutError: + logger.warning( + "[PHOTO-BG] timed out waiting for stage-22 producer for printer %s — proceeding to fallback", + printer_id, + ) cached_frame = _stage22_finish_frames.pop(printer_id, None) if cached_frame: photos_dir = archive_dir / "photos" diff --git a/backend/tests/unit/test_finish_photo_moment_sync.py b/backend/tests/unit/test_finish_photo_moment_sync.py new file mode 100644 index 000000000..28f91b4d5 --- /dev/null +++ b/backend/tests/unit/test_finish_photo_moment_sync.py @@ -0,0 +1,208 @@ +"""Regression tests for the #1790 producer-consumer synchronization. + +`on_finish_photo_moment` (producer) and `_background_finish_photo` +(consumer) are dispatched back-to-back on the FINISH-state fallback path +(`bambu_mqtt.py:3258-3297`). Before #1790, the consumer ran a single +`pop()` on `_stage22_finish_frames` with no wait — racing past the +producer with an empty result, then doing its own RTSP grab that +collided with the producer's still-in-flight grab (Bambu printers allow +one RTSP client). Net result: a captured frame was logged, the cache +was populated ~1s later, but the notification went text-only. + +The fix is an `asyncio.Event` per printer registered in +`_stage22_finish_in_flight` by the producer and awaited (with timeout) +by the consumer. These tests pin the producer side of that contract. +""" + +import asyncio +from contextlib import asynccontextmanager +from types import SimpleNamespace +from unittest.mock import AsyncMock + +import pytest + +from backend.app import main as main_module +from backend.app.main import on_finish_photo_moment + + +@asynccontextmanager +async def _fake_session(printer): + """Async-session stub that returns `printer` from scalar_one_or_none().""" + result = SimpleNamespace(scalar_one_or_none=lambda: printer) + session = SimpleNamespace(execute=AsyncMock(return_value=result)) + yield session + + +@pytest.fixture +def fake_printer(): + return SimpleNamespace( + id=7, + ip_address="192.0.2.7", + access_code="x", + model="X1C", + external_camera_enabled=False, + external_camera_url=None, + external_camera_type=None, + external_camera_snapshot_url=None, + ) + + +@pytest.fixture(autouse=True) +def _clean_state(): + """Don't leak event/cache dict entries across tests.""" + main_module._stage22_finish_in_flight.clear() + main_module._stage22_finish_frames.clear() + yield + main_module._stage22_finish_in_flight.clear() + main_module._stage22_finish_frames.clear() + + +@pytest.fixture +def patched_env(fake_printer, monkeypatch): + monkeypatch.setattr(main_module, "async_session", lambda: _fake_session(fake_printer)) + + async def _get_setting(_db, key): + if key == "capture_finish_photo": + return "true" + return None + + monkeypatch.setattr( + "backend.app.api.routes.settings.get_setting", + _get_setting, + ) + monkeypatch.setattr( + "backend.app.api.routes.camera.get_buffered_frame", + lambda _pid: None, + ) + return fake_printer + + +async def test_event_registered_before_first_await(patched_env, monkeypatch): + """The consumer needs to find the event the moment it polls — that + means registration must complete BEFORE any `await` yields control + back to the loop.""" + # Slow the first await (DB session entry) so we can observe the dict + # before the producer makes any real progress. + seen_during_capture = {} + + async def _slow_capture(**_kwargs): + seen_during_capture["registered"] = patched_env.id in main_module._stage22_finish_in_flight + await asyncio.sleep(0) + return b"\xff\xd8frame" + + monkeypatch.setattr( + "backend.app.services.camera.capture_camera_frame_bytes", + _slow_capture, + ) + + await on_finish_photo_moment(patched_env.id, {"trigger": "finish_state"}) + + assert seen_during_capture["registered"] is True + + +async def test_event_set_after_successful_capture(patched_env, monkeypatch): + async def _capture(**_kwargs): + return b"\xff\xd8frame" + + monkeypatch.setattr( + "backend.app.services.camera.capture_camera_frame_bytes", + _capture, + ) + + await on_finish_photo_moment(patched_env.id, {"trigger": "finish_state"}) + + event = main_module._stage22_finish_in_flight[patched_env.id] + assert event.is_set() + assert main_module._stage22_finish_frames[patched_env.id] == b"\xff\xd8frame" + + +async def test_event_set_when_capture_returns_no_frame(patched_env, monkeypatch): + """Producer gives up (RTSP timeout, no buffered frame, no external + camera) — consumer must NOT wait the full 20s for nothing.""" + + async def _capture(**_kwargs): + return None + + monkeypatch.setattr( + "backend.app.services.camera.capture_camera_frame_bytes", + _capture, + ) + + await on_finish_photo_moment(patched_env.id, {"trigger": "finish_state"}) + + event = main_module._stage22_finish_in_flight[patched_env.id] + assert event.is_set() + assert patched_env.id not in main_module._stage22_finish_frames + + +async def test_event_set_even_when_capture_raises(patched_env, monkeypatch): + """Producer hit a bug or network error — `finally` still has to + release the consumer.""" + + async def _capture(**_kwargs): + raise RuntimeError("camera went away") + + monkeypatch.setattr( + "backend.app.services.camera.capture_camera_frame_bytes", + _capture, + ) + + await on_finish_photo_moment(patched_env.id, {"trigger": "finish_state"}) + + event = main_module._stage22_finish_in_flight[patched_env.id] + assert event.is_set() + + +async def test_no_event_when_timelapse_was_active(patched_env): + """On the timelapse-on path the consumer takes the + `_capture_finish_photo_from_timelapse` branch and shouldn't be + blocked by a producer wait — the producer doesn't enter the + lifecycle.""" + await on_finish_photo_moment( + patched_env.id, + {"trigger": "stage_22", "timelapse_was_active": True}, + ) + + assert patched_env.id not in main_module._stage22_finish_in_flight + + +async def test_event_set_when_capture_setting_disabled(patched_env, monkeypatch): + """Even on the early-return-before-capture path, the event must be + released so the consumer doesn't hang on a no-op producer.""" + + async def _disabled_setting(_db, _key): + return "false" + + monkeypatch.setattr( + "backend.app.api.routes.settings.get_setting", + _disabled_setting, + ) + + await on_finish_photo_moment(patched_env.id, {"trigger": "finish_state"}) + + event = main_module._stage22_finish_in_flight[patched_env.id] + assert event.is_set() + + +async def test_consumer_wait_unblocked_when_producer_completes(patched_env, monkeypatch): + """End-to-end sync check: a consumer-style waiter awaiting the + event finishes promptly once the producer's finally fires.""" + + async def _capture(**_kwargs): + await asyncio.sleep(0.05) + return b"\xff\xd8frame" + + monkeypatch.setattr( + "backend.app.services.camera.capture_camera_frame_bytes", + _capture, + ) + + producer = asyncio.create_task(on_finish_photo_moment(patched_env.id, {"trigger": "finish_state"})) + + await asyncio.sleep(0) # let the producer register + + event = main_module._stage22_finish_in_flight[patched_env.id] + await asyncio.wait_for(event.wait(), timeout=1.0) + + assert main_module._stage22_finish_frames[patched_env.id] == b"\xff\xd8frame" + await producer