From ce5f4e5f1a26ee894348f084351596b98386715e Mon Sep 17 00:00:00 2001 From: maziggy Date: Sat, 16 May 2026 12:31:11 +0200 Subject: [PATCH] fix(camera): don't open competing socket while a viewer is attached (#1348) Obico polling could freeze the live camera stream within seconds of opening the viewer. Cause: when the buffer-reuse path in obico_detection._capture_frame saw an empty _last_frames[printer_id] entry (stream startup before the first JPEG lands, or upstream mid-reconnect after a 30s read timeout), it fell through to capture_camera_frame_bytes() and opened a second RTSP socket. On firmwares that allow only one camera connection, that second socket forced the printer to drop the live fan-out connection - the viewer's ffmpeg then hit its own 30s timeout, looped through 30 reconnects at 0.2s, all racing the next Obico poll, and the broadcaster pump exited. Widen the gate from "do we have a buffered frame?" to "is any fan-out stream registered for this printer?". New is_stream_active() helper checks _active_streams / _active_chamber_streams independently of buffer state. _capture_frame consults it first: if a viewer is attached, it returns the buffered frame when available or None (skip this poll cycle) when not. Never opens a competing socket while a viewer is connected. Cost: at most one missed Obico detection cycle per viewer-attach (~10s lag). Benefit: zero competing-socket events while any viewer is connected. try_get_active_buffered_frame() refactored to delegate to is_stream_active() so the two helpers stay in lockstep. The /camera/snapshot caller is unchanged behaviorally (snapshot is a user-initiated single-shot; falling through to fresh capture on an empty buffer is the desired behavior there). --- CHANGELOG.md | 2 + backend/app/api/routes/camera.py | 29 ++++++++++++-- backend/app/services/obico_detection.py | 24 +++++++++--- backend/tests/unit/test_obico_detection.py | 45 +++++++++++++++++++++- 4 files changed, 88 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39486abf3..830197566 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ All notable changes to Bambuddy will be documented in this file. - **Slice modal: pick the build plate (#1337, reported by @digitalskies)** — Slicing a plain STL through the integrated slicer always defaulted to whatever `curr_bed_type` lived in the chosen process preset (typically `Cool Plate`), which the slicer CLI then rejected for high-temp filaments with `Plate 1: Cool Plate does not support filament 1`. The user had no way to switch plates short of cloning the process preset in BambuStudio, which defeats the point of the in-app slicer. The Slice modal now exposes a `Build plate` dropdown with the six canonical BambuStudio / OrcaSlicer plates (Cool Plate, Cool Plate SuperTack, Engineering Plate, High Temp Plate, Textured PEI Plate, Smooth PEI Plate) plus an explicit `Auto (use process preset)` option that preserves the previous behavior. The dropdown sits between Process profile and Filament rows so it stays visible regardless of how many filament slots the picked plate uses (a long filament list would otherwise push it off the modal's `max-h-[85vh]` scroll viewport) and is **always enabled** — including when the user picks a Printer Preset Bundle from the top BundlePicker. When the user picks a specific plate, the new `bed_type` field on `SliceRequest` ([`backend/app/schemas/slicer.py`](backend/app/schemas/slicer.py)) flows through the dispatcher via two paths: (1) **resolved-preset path** — the route helper `_patch_process_bed_type` in [`backend/app/api/routes/library.py`](backend/app/api/routes/library.py) overwrites `curr_bed_type` on the resolved process JSON before forwarding to the sidecar (no preset cloning required); (2) **bundle dispatch path** — `slice_with_bundle` in [`backend/app/services/slicer_api.py`](backend/app/services/slicer_api.py) adds a `bedType` form field to the sidecar multipart so the sidecar can pass `--curr_bed_type` through to the CLI, which lets the override take effect even though Bambuddy can't patch the bundle's process JSON locally (the sidecar materialises it from the stored .bbscfg). Sidecar versions that don't recognise the field silently no-op — the slice still runs, just with the bundle's default plate; the slicer-API fork at maziggy/orca-slicer-api will need the matching change for the bundle path to take full effect. **i18n parity:** 8 new keys (`slice.bedType.{label,auto,coolPlate,coolPlateSuperTack,engineering,highTemp,texturedPEI,smoothPEI}`) added to all 8 locales — full German translation, English fallbacks elsewhere per project convention. **Regression tests:** 4 in [`test_slice_request_bed_type.py`](backend/tests/unit/test_slice_request_bed_type.py) (`bed_type` defaults to None, accepts the six canonical strings, rejects overlong input via the schema's `max_length=64`; `_patch_process_bed_type` overwrites an existing value, adds the field when missing, and returns the input unchanged for malformed JSON or non-dict roots), 4 in [`test_library_slice_api.py`](backend/tests/integration/test_library_slice_api.py) (resolved-preset path: with `bed_type` set, the sidecar receives `"curr_bed_type": "Textured PEI Plate"` in the presetProfile multipart part; without it, `curr_bed_type` stays out of the body entirely. bundle dispatch path: `bedType` form field carries the override through to the sidecar; omitting `bed_type` keeps the form field out of the request so the bundle's own `curr_bed_type` is preserved), 2 in [`SliceModal.test.tsx`](frontend/src/__tests__/components/SliceModal.test.tsx) (dropdown selection puts `bed_type` on the request; leaving it on Auto omits the field). 59 backend slice tests + 34 SliceModal tests pass; build and i18n parity script clean. ### Fixed +- **Camera stream no longer freezes every ~30 s when Obico fault detection is enabled on the same printer the user is viewing** ([#1348](https://github.com/maziggy/bambuddy/issues/1348), reported by @SL666) — Symptom on an X1-class printer running firmware that allows only one concurrent camera connection: opening the live camera worked initially, then the stream hung within seconds and "cancelled" — clicking the in-UI refresh restored it for another few seconds before it hung again, ad infinitum. Disabling Obico fault detection made the stream stable. **Root cause** in `_capture_frame` at [`backend/app/services/obico_detection.py:209-220`](backend/app/services/obico_detection.py): the buffer-reuse path that was supposed to make Obico polling free (#1271) only worked when `_last_frames[printer_id]` was populated. In every race window where `_active_streams` had a registered entry but the JPEG buffer was empty — stream startup before the first frame lands (1–3 s on RTSP), or any moment the upstream ffmpeg was mid-reconnect after a 30 s read timeout — `try_get_active_buffered_frame(printer_id)` returned `None` and the caller fell through to `capture_camera_frame_bytes(...)`, which spawned its own ffmpeg + TLS proxy and opened a competing RTSP socket on the printer. On firmwares that only allow one camera connection, that second socket forced the printer to drop the live fan-out connection — viewers' ffmpeg hit its 30 s read timeout, looped through 30 reconnect attempts at 0.2 s each (all racing the next Obico poll 10 s later), exhausted retries, and the broadcaster pump exited. The user's viewer disconnected with no obvious cause in the log because the Obico capture itself looked successful (`Successfully captured camera frame bytes: 83484 bytes`). **Fix:** the buffer-reuse gate was widened from "do we have a frame in the buffer?" to "is any fan-out stream registered for this printer?" — even when the buffer is momentarily empty. New helper `is_stream_active(printer_id) -> bool` at [`backend/app/api/routes/camera.py:82`](backend/app/api/routes/camera.py) checks `_active_streams` / `_active_chamber_streams` independently of buffer state. `_capture_frame` now consults `is_stream_active` first: if True, it returns the buffered frame when available or `None` (skip this poll cycle) when not — it **never** opens a competing socket while a viewer is attached, regardless of buffer state. The poll loop retries 10 s later, by which time the buffer is virtually guaranteed to be populated. The fresh-socket path still fires unchanged when no viewer is connected (Obico's primary use case: detection on idle/headless prints). Cost of the fix: at most one missed Obico detection cycle per viewer-attach (~10 s lag); benefit: zero competing-socket events while any viewer is connected. `try_get_active_buffered_frame` was refactored to delegate to `is_stream_active` so the two helpers stay in lockstep — its `/camera/snapshot` caller at [`camera.py:859`](backend/app/api/routes/camera.py) is unchanged behaviorally (snapshot is user-initiated single-shot; falling through to a fresh capture if buffer is momentarily empty is the desired behavior there). **Regression tests** in [`backend/tests/unit/test_obico_detection.py`](backend/tests/unit/test_obico_detection.py): new `test_skips_poll_when_stream_active_but_buffer_empty` reproduces the exact race (viewer registered, buffer empty) and pins that `_capture_frame` returns `None` and `capture_camera_frame_bytes` is NOT called; existing `test_returns_buffered_frame_when_stream_active` and `test_falls_back_to_fresh_capture_when_no_stream` were updated to patch the new `is_stream_active` helper alongside `try_get_active_buffered_frame`. All 31 obico tests + 113 in the wider camera/obico sweep pass; ruff clean. + - **Reprints (including failed and cancelled ones) no longer overwrite the source archive's statistics; Quick Stats now adds per-print events** ([#1378](https://github.com/maziggy/bambuddy/issues/1378), reported by @IndividualGhost1905) — Symptom: after reprinting a model from the Archive page, the reprint contributed nothing to Quick Stats / Statistics; worse, if the reprint failed at 10g while the original print used 100g, the archive card and the totals both flipped from 100g to 10g — losing the original print's data. **Root cause** in [`backend/app/main.py:1973`](backend/app/main.py) (`_handle_print_start`): every reprint shared the source archive's row via the `register_expected_print`/`expected_archive_id` path, and statistics (`GET /archives/stats` in [`backend/app/api/routes/archives.py`](backend/app/api/routes/archives.py), Prometheus `/metrics` in [`backend/app/api/routes/metrics.py`](backend/app/api/routes/metrics.py)) summed PrintArchive columns — so a single archive row was the only contribution per file regardless of how many times the user pressed Reprint. The cost overwrite at [`usage_tracker.py:633`](backend/app/services/usage_tracker.py) and energy overwrite at [`main.py:3625`](backend/app/main.py) compounded it: each run's actuals replaced the previous run's values on the source row. **Architectural fix** — statistics are now event-based, not file-based. The existing `PrintLogEntry` table (one row per print event, written at print completion; lives in [`backend/app/models/print_log.py`](backend/app/models/print_log.py) and already powered the cross-archive /print-log page) gains six columns: `archive_id` (nullable FK, `ON DELETE SET NULL` so log entries survive archive deletion — preserving the #1343 soft-delete-vs-stats decoupling), `cost`, `energy_kwh`, `energy_cost`, `failure_reason`, `created_by_id`. Idempotent SQLite + Postgres migrations in [`backend/app/core/database.py`](backend/app/core/database.py). `/archives/stats` and `/metrics` now sum/count from `PrintLogEntry` joined to `PrintArchive` for time-accuracy comparisons and user-scope filters. The `_run_reprint_archive` flow still keys to the source archive (so the archive list stays one-card-per-file rather than ballooning into one card per print), but every print completion writes a new `PrintLogEntry` row with the run's actual filament / time / cost / energy / status / failure_reason. The cost overwrite in `usage_tracker.py` now only fires on the first run (counts existing `PrintLogEntry` rows for the archive); the energy background task at `main.py:3625` similarly preserves the source archive's energy_kwh on reprints while backfilling the run's energy on the matching `PrintLogEntry` row (it runs after `write_log_entry`, so it fetches and updates the latest log row for the archive). **New UX surface** — archive list responses gain four aggregate fields (`run_count`, `last_run_at`, `total_filament_actual_grams`, `successful_run_count`, `failed_run_count`) computed via a single batch query (`_load_run_aggregates` in [`backend/app/api/routes/archives.py`](backend/app/api/routes/archives.py)); the archive card renders an orange "N prints" badge when `run_count > 1`, with a tooltip breaking down successful vs failed runs. New endpoint `GET /archives/{archive_id}/runs` returns every PrintLogEntry for the archive (newest first), powering a new Print Log section at the top of the Edit Archive modal — date / status / duration / filament / cost columns plus failure_reason text under failed runs (new component [`frontend/src/components/PrintLogTable.tsx`](frontend/src/components/PrintLogTable.tsx)). i18n keys added under `archives.card.runsBadge*` and `archives.runLog.*` (en/de/ja translated; other 5 locales fall back to English per project convention). **Soft-delete contract preserved** — the `purge_stats=true` hard-delete path now also `DELETE`s linked PrintLogEntry rows so the archive's contribution truly disappears from totals; soft-delete (the default) leaves the log entries intact so #1343 stats-preservation still works. **Partial-print accuracy** — failed / cancelled / stopped reprints would have over-counted in the new stats path if PrintLogEntry just recorded the source archive's slicer estimate (100g for a print that stopped at 10g). The write_log_entry call site now uses a partial-aware filament value via the new `_compute_run_filament_grams` helper at [`backend/app/main.py`](backend/app/main.py): completed prints record the estimate (since the print finished), and partial prints prefer sum of tracked spool deltas from `usage_results` if inventory was set up, falling back to `estimate × progress%` from the MQTT push, and finally to None if no signal exists. The per-run cost write uses the same precedence: prefer the usage_results sum (raw — without the topup-to-estimate inflation that `usage_tracker.update_archive_usage` applies for archive.cost, which assumes the print completed), and only fall back to archive.cost for completed prints. **Test fixture update** — the `archive_factory` conftest fixture now synthesizes one `PrintLogEntry` per completed test archive (since stats moved to that table, the previous "create archive only" pattern would silently produce 0-stat tests); pass `with_run=False` to skip for the "archived but never printed" case. 3 integration tests in [`backend/tests/unit/test_archive_run_aggregation.py`](backend/tests/unit/test_archive_run_aggregation.py) pin the reporter's exact scenario (100g completed + 10g failed reprint → stats show 110g total / 2 prints / 1 successful / 1 failed), the archive-list aggregates wiring, and the `/runs` endpoint ordering. 14 unit tests in [`backend/tests/unit/test_run_filament_helper.py`](backend/tests/unit/test_run_filament_helper.py) lock the partial-print math across completed / failed / cancelled / stopped statuses, both inventory-tracked and untracked paths, multi-filament sums, the >100% progress clamp, and the None-fallback cases. The full backend test suite — 4921 tests across unit + integration — passes; ruff clean; frontend build clean. - **Matplotlib no longer logs `Permission denied: /app/.config` on every container start** — Matplotlib (imported lazily by the STL thumbnail generator in [`backend/app/services/stl_thumbnail.py`](backend/app/services/stl_thumbnail.py) when a user uploads an `.stl` file to the library) tries to create its font/style cache at `$HOME/.config/matplotlib` on first import. `HOME` is pinned to `/app` in the Dockerfile so containers with `pwd.getpwuid()` failures (PUID-mapped uids without a local passwd entry) still have a writable home — but `/app` itself is root-owned and not writable by the PUID:PGID the entrypoint drops to, so the first STL upload after a container start logged an `EPERM` warning and matplotlib fell back to a fresh `/tmp/matplotlib-*` dir. Functionally harmless (thumbnails still rendered) but it cluttered every support bundle and forced matplotlib to re-scan system fonts on every restart (~1-2 s per first-STL-upload). **Fix:** added `ENV MPLCONFIGDIR=/tmp/matplotlib` to the Dockerfile so matplotlib uses a guaranteed-writable cache dir up front. `/tmp` is writable by any uid so this works regardless of PUID, and the cache survives the container's lifetime so the font scan only pays its cost once per container. diff --git a/backend/app/api/routes/camera.py b/backend/app/api/routes/camera.py index 8abe5b409..371dfd708 100644 --- a/backend/app/api/routes/camera.py +++ b/backend/app/api/routes/camera.py @@ -79,6 +79,25 @@ def get_buffered_frame(printer_id: int) -> bytes | None: return _last_frames.get(printer_id) +def is_stream_active(printer_id: int) -> bool: + """Return True iff a fan-out camera stream is currently registered for this printer. + + Snapshot callers (Obico polling, manual /camera/snapshot) MUST NOT open a + second concurrent RTSP/chamber-image socket while a viewer is attached: + most Bambu firmwares allow only one camera connection, so the competing + socket either kicks the live viewer off or gets refused itself, and the + resulting reconnect storm tears down the fan-out broadcaster (see #1348). + + Callers should consult this BEFORE trying to open a fresh socket and skip + the capture cycle when it returns True — even if try_get_active_buffered_frame + returns None (the stream may be running but the first frame hasn't landed + in the buffer yet, or the upstream is mid-reconnect). + """ + return any(k.startswith(f"{printer_id}-") for k in _active_streams) or any( + k.startswith(f"{printer_id}-") for k in _active_chamber_streams + ) + + def try_get_active_buffered_frame(printer_id: int) -> bytes | None: """Return a buffered frame iff a stream is currently running for this printer. @@ -89,11 +108,13 @@ def try_get_active_buffered_frame(printer_id: int) -> bytes | None: Returns None when no broadcaster is active for this printer, so callers fall through to their existing fresh-socket path unchanged. + + NB: returning None does NOT mean "safe to open a fresh socket" — it also + fires when the stream is registered but no frame has been buffered yet + (startup race, mid-reconnect). Callers that must avoid competing sockets + should consult is_stream_active() first; see #1348. """ - has_stream = any(k.startswith(f"{printer_id}-") for k in _active_streams) or any( - k.startswith(f"{printer_id}-") for k in _active_chamber_streams - ) - if not has_stream: + if not is_stream_active(printer_id): return None return _last_frames.get(printer_id) diff --git a/backend/app/services/obico_detection.py b/backend/app/services/obico_detection.py index 6240c9e9c..c97a44561 100644 --- a/backend/app/services/obico_detection.py +++ b/backend/app/services/obico_detection.py @@ -204,13 +204,25 @@ class ObicoDetectionService: # already watching — avoids opening a second concurrent RTSP socket # on printers that allow only one camera connection (e.g. X2D # firmware 01.01.00.00; see #1271). Buffered frame is <1s old while - # a viewer is connected. Returns None when no stream is active, so - # we fall through to a fresh socket as before. - from backend.app.api.routes.camera import try_get_active_buffered_frame + # a viewer is connected. + # + # When a viewer is attached but no frame is buffered yet (startup + # race, mid-reconnect), we DELIBERATELY skip this poll cycle instead + # of falling through to capture_camera_frame_bytes. Opening a fresh + # RTSP/chamber socket would compete with the live viewer and kick + # the fan-out connection on most firmwares — exactly the freeze + # reported in #1348. The poll loop retries in ~10s. + from backend.app.api.routes.camera import is_stream_active, try_get_active_buffered_frame - buffered = try_get_active_buffered_frame(printer_id) - if buffered: - return buffered + if is_stream_active(printer_id): + buffered = try_get_active_buffered_frame(printer_id) + if buffered: + return buffered + logger.info( + "Obico: viewer attached for printer %s but buffer empty; skipping this poll to avoid competing camera socket (#1348)", + printer_id, + ) + return None return await capture_camera_frame_bytes( ip_address=printer.ip_address, diff --git a/backend/tests/unit/test_obico_detection.py b/backend/tests/unit/test_obico_detection.py index 381910ec5..2f7583d58 100644 --- a/backend/tests/unit/test_obico_detection.py +++ b/backend/tests/unit/test_obico_detection.py @@ -294,6 +294,10 @@ class TestCaptureFrameSharesBroadcasterUpstream: svc = ObicoDetectionService() with ( patch("backend.app.services.obico_detection.async_session", return_value=mock_ctx), + patch( + "backend.app.api.routes.camera.is_stream_active", + return_value=True, + ), patch( "backend.app.api.routes.camera.try_get_active_buffered_frame", return_value=FAKE_JPEG, @@ -308,6 +312,43 @@ class TestCaptureFrameSharesBroadcasterUpstream: assert result == FAKE_JPEG mock_fresh.assert_not_called() + @pytest.mark.asyncio + async def test_skips_poll_when_stream_active_but_buffer_empty(self): + """#1348: viewer attached + buffer empty must NOT open a competing socket.""" + printer = MagicMock( + external_camera_enabled=False, + external_camera_url=None, + ip_address="192.168.1.10", + access_code="12345678", + model="X1C", + ) + mock_session = MagicMock() + mock_session.get = AsyncMock(return_value=printer) + mock_ctx = MagicMock() + mock_ctx.__aenter__ = AsyncMock(return_value=mock_session) + mock_ctx.__aexit__ = AsyncMock(return_value=None) + + svc = ObicoDetectionService() + with ( + patch("backend.app.services.obico_detection.async_session", return_value=mock_ctx), + patch( + "backend.app.api.routes.camera.is_stream_active", + return_value=True, + ), + patch( + "backend.app.api.routes.camera.try_get_active_buffered_frame", + return_value=None, # Stream active, but first frame not buffered yet + ), + patch( + "backend.app.services.camera.capture_camera_frame_bytes", + new=AsyncMock(return_value=b"FRESH-CAPTURE-WOULD-KICK-VIEWER"), + ) as mock_fresh, + ): + result = await svc._capture_frame(printer_id=1) + + assert result is None, "must skip this poll cycle, not open a competing socket" + mock_fresh.assert_not_called() + @pytest.mark.asyncio async def test_falls_back_to_fresh_capture_when_no_stream(self): printer = MagicMock( @@ -327,8 +368,8 @@ class TestCaptureFrameSharesBroadcasterUpstream: with ( patch("backend.app.services.obico_detection.async_session", return_value=mock_ctx), patch( - "backend.app.api.routes.camera.try_get_active_buffered_frame", - return_value=None, # No active stream + "backend.app.api.routes.camera.is_stream_active", + return_value=False, ), patch( "backend.app.services.camera.capture_camera_frame_bytes",