mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-04 13:11:35 +02:00
fix(archives): assign printer_id when reusing VP-queue archives in print-start (#1403 follow-up)
VP-queue archives are created with printer_id=None at queue-add time because the scheduler hasn't picked a printer yet (and even for explicit-printer queue items, the archive predates dispatch). on_print_start's expected-archive branch updated status, started_at, and subtask_id but never assigned printer_id, so VP-queue-dispatched archives stayed permanently unassigned. That broke every UI/API path gated on archive.printer_id — critically the post-print "Scan for timelapse" action: the H.264 file is on the printer's SD card and reachable via the file browser, but the archive's scan endpoint refused the request and the button stayed greyed out forever. One-line fix: archive.printer_id = printer_id in the expected-archive branch. Guarded against clobbering an already-correct value so library-file queue items (which create their archive with the printer pre-assigned) are idempotent.
This commit is contained in:
maziggy
@@ -10,6 +10,8 @@ All notable changes to Bambuddy will be documented in this file.
|
||||
- **Camera: in-app diagnostic for "Connection lost" (#1395 follow-up)** — Second step of the camera architecture overhaul. When the camera viewer hits its error state, a new **Diagnose** button next to **Retry** runs a staged check against the printer and renders the result inline: which stage failed, how long it took, and a translated remediation hint. Cuts off the "user opens a 'camera broken' ticket → wait days → ask for the support bundle → finally figure out it was their reverse proxy / LAN-only toggle / wrong access code" loop at the user's screen. **Backend** ships `backend/app/services/camera_diagnose.py` (orchestrator) and a new `POST /printers/{id}/camera/diagnose` route in `camera.py`. Stages: (1) `tcp_reachable` — opens a TCP socket to the camera port (322 RTSPS / 6000 chamber image) with a 3-second timeout; distinguishes timeout (`tcp_timeout` → "printer not reachable, check IP/network/power") from refused (`tcp_refused` → "camera port closed, check LAN-only and developer mode") from host-unreachable (`tcp_unreachable` → "printer not reachable"). (2) `first_frame` — captures one JPEG end-to-end via the existing `capture_camera_frame_bytes` pipeline (15-second timeout, same code that powers `/camera/snapshot`); auth, RTSP handshake, and first keyframe collapse into one stage because the user-facing answer is the same regardless of which sub-layer failed. **Live-stream shortcut**: when a viewer is currently watching the printer's camera AND the buffered last-frame timestamp is fresher than 10 seconds, the diagnostic skips the real test and returns `live_stream_active_healthy` — opening a fresh socket would kick the live viewer off on single-camera-connection firmwares (the #1348 reconnect-storm trigger), so we trust the real-world evidence instead. Response includes structured metadata for support triage: `protocol` (rtsp / chamber_image), `port`, `profile` (`default` or the model name with an override — currently only `P2S`), per-stage duration in ms, and the machine-readable summary code. **Frontend** adds `CameraDiagnoseModal.tsx` that fires the API call on mount, renders one row per stage with green-check / red-X / grey-skipped icons, and shows the summary remediation message in a bordered banner styled by overall status. The metadata line at the bottom (protocol / port / profile) lets support triage ask "what does your modal say?" instead of "send the support bundle". A **Run again** button re-runs the diagnostic without dismissing the modal. **EmbeddedCameraViewer** error state grows the Diagnose button (kept "Retry" as the primary action; Diagnose is the escape hatch for users who can't see what's wrong). A small stethoscope icon also lives in the viewer's always-visible control bar between **Refresh** and **Fullscreen**, so pre-flight testing ("did my firmware update break the camera?", "is the camera up before I send a print?") doesn't require waiting for the stream to fail first. Also lifted the previously-hard-coded "Camera unavailable" / "Retry" strings into `camera.unavailable` / `camera.retry` so the error UI is properly translated alongside the new keys. **i18n**: 16 new keys (`unavailable`, `retry`, plus `diagnose.{button,modalTitle,running,runFailed,retry,stage.*,summary.*,meta.*}`) translated across all 8 locales (en/de/fr/it/ja/pt-BR/zh-CN/zh-TW). German "Diagnose" is a real cognate — added to `IDENTICAL_TO_EN_ALLOWED.de` rather than translated to a synthetic. Parity check holds at 4849 leaves per locale. **Tests**: 11 backend unit tests in `test_camera_diagnose.py` cover the live-stream shortcut (skip when fresh, run when stale), the three TCP failure modes (timeout / refused / OSError) → distinct summary codes, the first-frame stage (no-frame and capture-exception cases), the all-OK path, and the result metadata (P2S → P2S profile / rtsp / 322; A1 → default / chamber_image / 6000; X1C → default / rtsp / 322). 1 backend integration test pins the route's response shape end-to-end. 3 frontend tests in `CameraDiagnoseModal.test.tsx` (mounted → API call, failure → translated remediation, Run again → re-call). 5021 backend tests + 1905 frontend tests green; ruff clean; build clean; i18n parity clean.
|
||||
|
||||
### Fixed
|
||||
- **Archives: "Scan for timelapse" no longer permanently disabled on VP-queue-dispatched prints (#1403 follow-up, reported by @pwostran and @enjoylifenow)** — Reporters dispatched a print from a slicer via the VP print queue, the printer recorded the timelapse to its SD card (visible and downloadable via Bambuddy's file browser), but the archive UI's "Scan for timelapse" and "View Timelapse" actions stayed greyed out forever. Frontend gates both on `archive.printer_id` (`ArchivesPage.tsx:459`); backend `/archives/{id}/timelapse/scan` also 400s when the archive has no `printer_id`. Root cause was in `main.py::on_print_start`'s expected-archive branch: VP-queue archives are created with `printer_id=None` at queue-add time (we don't yet know which printer will run the job — the scheduler decides later for "Any P1S"-style queue items, and even for explicit-printer queue items the archive is created before dispatch). When the print actually starts and `on_print_start` looks up the expected archive via `_expected_prints`, the branch updated `status`, `started_at`, and `subtask_id` but never assigned `printer_id`. So the archive stayed `printer_id=None` for the entire print and forever after — and every downstream UI / API path gated on it (timelapse scan + view, the printer filter on Archives, per-printer stats, success-rate cohort attribution) treated the archive as "unassigned." One-line fix in the expected-archive branch sets `archive.printer_id = printer_id` when they differ, guarded against clobbering an already-correct value so library-file-based queue items (which create their archive with the printer pre-assigned at dispatch time) are unaffected. Two regressions in a new `test_print_start_assigns_printer_id_to_vp_archive.py`: `test_expected_archive_path_assigns_printer_id_when_unset` (VP-queue archive with `printer_id=None` → promoted to the running printer) and `test_expected_archive_path_preserves_existing_printer_id` (library-file archive with `printer_id=7` stays at 7 when the same printer runs it — the branch is idempotent on correct data). The 2 existing `test_layer_timelapse_expected_archive.py` tests + 19 `test_print_start_expected_promotion.py` regressions still green — the expected-archive branch's other side-effects (timelapse session, AMS mapping, status/started_at promotion) are unchanged. 5033 backend tests + ruff clean. **Note about the queue-mode slicer-inheritance path itself**: pwostran's specific complaint that the original #1403 fix "didn't work" was a misdiagnosis on his end — his support bundle proves Bambuddy correctly sent `timelapse: true` to the printer and the printer recorded the video; his actual gap was this archive-attachment bug. The slicer-inheritance branch added in #1403 is a no-op for OrcaSlicer's "Send to print queue" flow because Orca only does the FTP upload and never sends a `project_file` MQTT command at upload time — so the path falls back to `default_*` settings, which is the operative path for that workflow. Users who want a different per-print value still edit the queue item before starting (as pwostran did, which is why his dispatch carried `timelapse: true`).
|
||||
|
||||
- **SpoolBuddy: NFC reader works again on Raspberry Pi 5 (#1424, reported by @flom89)** — Reporter on a Pi 5 installed SpoolBuddy successfully but couldn't talk to the PN5180 NFC module (the gauge worked, so SPI hardware and wiring were fine). Manually commenting out `self._spi.no_cs = True` in the daemon restored communication; reporter wasn't sure whether removing it would regress Pi 4 installs. Root cause: Pi 5 uses the new RP1 southbridge and its kernel SPI driver (`spi-rp1`) doesn't accept the `SPI_NO_CS` ioctl the same way the historical Broadcom driver on Pi 4 did — setting `no_cs = True` on Pi 5 either errors out or silently leaves the bus in a state where transfers don't complete. **Safe to drop Pi-wide, not just Pi 5** — SpoolBuddy's PN5180 NSS line is wired to GPIO23 (manual chip-select handled by `_cs_low()` / `_cs_high()` around every transfer, because the kernel's default 5µs setup / 100µs hold timing doesn't meet the PN5180's spec). The hardware CE0 line (GPIO8) is not connected to the reader, so whether the kernel auto-toggles it during `xfer2()` is electrically invisible to the PN5180. The `no_cs = True` call was a "be polite to the bus" gesture that was always cosmetic on this hardware. Fix wraps the assignment in `try / except OSError` in both `spoolbuddy/daemon/pn5180.py` (logs at debug level) and `spoolbuddy/scripts/read_tag.py` (silent — it's a diagnostic script with no logger). Try/except over a hard delete because Pi 4 installs that work today shouldn't see any behaviour change. README updated at `spoolbuddy/README.md:23-28` to drop the "spidev.no_cs = True resolves this" sentence in favour of explaining that manual CS via GPIO23 carries the timing on its own and that Pi 4 + Pi 5 are both supported. Hardware-only path so no automated test — verified by the reporter's bench test that commenting the line out restores reads. Ruff clean.
|
||||
|
||||
- **Cover thumbnails: stop hammering FTP and GitHub when a print's 3MF isn't on the printer (#1420, reported by reporter)** — Reporter on a P2S running 0.2.4.1 saw two log-flooding bugs trigger together once they started a print whose 3MF wasn't on the printer's FTP storage (typical SD-card-only print). **(1) Cover endpoint had no negative cache.** `GET /printers/{id}/cover` cached successful 3MF thumbnail downloads in `_cover_cache` keyed by `(subtask_name, view_key)`, but never recorded failures. When all 8 candidate FTP paths returned `550 Failed to open file`, the endpoint raised 404 without remembering that it just tried — and since `cover_url` stays populated on every `PrinterStatus` response while state is RUNNING/PAUSE, every React-Query refetch and every component remount drove the frontend to re-fetch, replaying the same 8-path FTP fan-out roughly every few seconds. On the user's hardware the printer's single FTP socket was so busy with these doomed retries that it surfaced as camera-stream symptoms ("ffmpeg didn't terminate gracefully"). Fix adds a parallel `_cover_404_cache: dict[int, set[tuple[str, str]]]` that records the same `(subtask_name, view_key)` key on every 404 path — both the all-FTP-paths-failed branch and the 3MF-has-no-thumbnail-inside branch. On the next call for the same key, the endpoint short-circuits to 404 before even consulting FTP. The negative cache is cleared in `clear_cover_cache()` alongside the positive cache, which `main.py::on_print_start` already calls — so when the next print starts (different subtask, or same subtask after a re-upload of a new file) Bambuddy retries fresh. **(2) GitHub update-check had no backoff on 403 rate-limit.** Once `api.github.com` returned `403 rate limit exceeded` (typical when multiple Bambuddy instances or other tools share a NAT'd source IP and exhaust the unauthenticated 60-req/hr quota), the next call hit GitHub again immediately. Fix adds module-level `_github_rate_limit_until` epoch-seconds plus three helpers in `updates.py`: `_seconds_until_github_unblocked()`, `_record_github_rate_limit(response)` (reads `X-RateLimit-Reset` from the 403, falls back to a 1-hour pause when the header is absent or unparseable, and only extends the window — never shortens it via an out-of-order response), and `_is_github_rate_limit_response(response)` (status 403/429 with `X-RateLimit-Remaining: 0`, body-text fallback when proxies strip the header). Both call sites — `GET /updates/check` and the in-app updater's `_discover_target_release` — short-circuit when the window is active; the route surfaces a structured `{error: "GitHub rate limit reached...", retry_after_seconds: <int>}` response so the SettingsPage UI can show a real wait time instead of an opaque "failed to check for updates". The "ffmpeg didn't terminate gracefully" warning line the reporter quoted is the standard SIGTERM → 2s wait → SIGKILL pattern in `camera.py::_terminate_ffmpeg` — RTSP/TLS streams routinely take >2s to drain and that warning fires for many users with no FTP issues; once the cover loop is silenced the resource pressure is gone, and the warning itself is cosmetic. **Tests**: `test_cover_negative_cache_skips_repeat_ftp_fanout` in `test_printers_api.py` mocks `download_file_try_paths_async` to return False, calls the endpoint twice, and asserts the second call's FTP mock count is unchanged (the negative cache held); `test_check_backs_off_after_github_rate_limit` in `test_updates_api.py` patches `httpx.AsyncClient` to return a 403 with `X-RateLimit-Reset` set 10 minutes ahead and asserts the second `/updates/check` request never reaches httpx and surfaces `retry_after_seconds > 0`. 134 printers + updates integration tests green; ruff clean.
|
||||
|
||||
@@ -2016,6 +2016,15 @@ async def on_print_start(printer_id: int, data: dict):
|
||||
archive.started_at = datetime.now(timezone.utc)
|
||||
if subtask_id and not archive.subtask_id:
|
||||
archive.subtask_id = subtask_id
|
||||
# #1403 follow-up: VP-queue archives are created with
|
||||
# printer_id=None at queue-add time (we don't know which
|
||||
# printer will run the job yet). When the print actually
|
||||
# starts on a specific printer the expected-archive lookup
|
||||
# used to skip this assignment, leaving printer_id=None
|
||||
# forever — which then disables the "Scan for timelapse"
|
||||
# button in ArchivesPage (gated on !archive.printer_id).
|
||||
if archive.printer_id != printer_id:
|
||||
archive.printer_id = printer_id
|
||||
await db.commit()
|
||||
|
||||
# Track as active print
|
||||
|
||||
@@ -0,0 +1,207 @@
|
||||
"""Regression for #1403 follow-up: when on_print_start reuses a VP-queue archive,
|
||||
it must assign archive.printer_id so the post-print "Scan for timelapse" path
|
||||
in the archive UI isn't disabled forever.
|
||||
|
||||
Reporter @pwostran and @enjoylifenow both saw "Scan for timelapse" greyed out on
|
||||
archives that came from the VP print-queue flow even though the H.264 timelapse
|
||||
was sitting on the printer's SD card. The frontend gates that button on
|
||||
``!archive.printer_id`` (ArchivesPage.tsx:459). VP-queue archives are created
|
||||
with ``printer_id=None`` at queue-add time because we don't know which printer
|
||||
will run the job yet; the print-start handler's expected-archive branch updated
|
||||
status / started_at / subtask_id but never set printer_id, so the archive stayed
|
||||
unassigned forever.
|
||||
"""
|
||||
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from backend.app.main import (
|
||||
_active_prints,
|
||||
_expected_print_creators,
|
||||
_expected_print_registered_at,
|
||||
_expected_prints,
|
||||
_print_ams_mappings,
|
||||
register_expected_print,
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _clear_dicts():
|
||||
_expected_prints.clear()
|
||||
_expected_print_registered_at.clear()
|
||||
_expected_print_creators.clear()
|
||||
_print_ams_mappings.clear()
|
||||
_active_prints.clear()
|
||||
yield
|
||||
_expected_prints.clear()
|
||||
_expected_print_registered_at.clear()
|
||||
_expected_print_creators.clear()
|
||||
_print_ams_mappings.clear()
|
||||
_active_prints.clear()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_expected_archive_path_assigns_printer_id_when_unset():
|
||||
"""VP-queue archives land here with printer_id=None and must be promoted
|
||||
to the printer that actually started the job. Without this the
|
||||
/archives/{id}/timelapse/scan endpoint refuses the request (it requires
|
||||
archive.printer_id) and the UI button stays disabled."""
|
||||
mock_printer = MagicMock()
|
||||
mock_printer.id = 1
|
||||
mock_printer.auto_archive = True
|
||||
mock_printer.external_camera_enabled = False
|
||||
mock_printer.external_camera_url = None
|
||||
mock_printer.name = "TestP1S"
|
||||
|
||||
# VP-queue archive: printer_id is None — this is the bug surface.
|
||||
mock_archive = MagicMock()
|
||||
mock_archive.id = 42
|
||||
mock_archive.filename = "bambu_lab_a1_tool_plate_3.gcode.3mf"
|
||||
mock_archive.subtask_id = None
|
||||
mock_archive.print_time_seconds = None
|
||||
mock_archive.created_by_id = None
|
||||
mock_archive.printer_id = None
|
||||
mock_archive.print_name = "A1 Tool Plate 3"
|
||||
mock_archive.status = "archived"
|
||||
mock_archive.file_path = "/tmp/fake.3mf"
|
||||
mock_archive.energy_start_kwh = None
|
||||
|
||||
register_expected_print(1, "bambu_lab_a1_tool_plate_3.gcode.3mf", archive_id=42, ams_mapping=None)
|
||||
|
||||
def execute_router(stmt, *args, **kwargs):
|
||||
sql = str(stmt).lower()
|
||||
if "from printers" in sql or "from printer " in sql:
|
||||
return MagicMock(
|
||||
scalar_one_or_none=MagicMock(return_value=mock_printer),
|
||||
scalars=MagicMock(return_value=MagicMock(all=MagicMock(return_value=[mock_printer]))),
|
||||
)
|
||||
if "from print_archives" in sql or "from print_archive" in sql:
|
||||
return MagicMock(
|
||||
scalar_one_or_none=MagicMock(return_value=mock_archive),
|
||||
scalars=MagicMock(return_value=MagicMock(all=MagicMock(return_value=[mock_archive]))),
|
||||
)
|
||||
return MagicMock(
|
||||
scalar_one_or_none=MagicMock(return_value=None),
|
||||
scalars=MagicMock(return_value=MagicMock(all=MagicMock(return_value=[]))),
|
||||
)
|
||||
|
||||
mock_session = AsyncMock()
|
||||
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
|
||||
mock_session.__aexit__ = AsyncMock()
|
||||
mock_session.execute = AsyncMock(side_effect=execute_router)
|
||||
mock_session.commit = AsyncMock()
|
||||
|
||||
with (
|
||||
patch("backend.app.main.async_session") as mock_session_maker,
|
||||
patch("backend.app.main.notification_service") as mock_notif,
|
||||
patch("backend.app.main.smart_plug_manager") as mock_plug,
|
||||
patch("backend.app.main.ws_manager") as mock_ws,
|
||||
patch("backend.app.main.printer_manager") as mock_pm,
|
||||
patch("backend.app.main.mqtt_relay") as mock_relay,
|
||||
patch("backend.app.main._record_energy_start", new_callable=AsyncMock),
|
||||
patch("backend.app.main._load_objects_from_archive"),
|
||||
patch("backend.app.main._store_spoolman_print_data", new_callable=AsyncMock),
|
||||
patch("backend.app.main._send_print_start_notification", new_callable=AsyncMock),
|
||||
):
|
||||
mock_session_maker.return_value = mock_session
|
||||
mock_notif.on_print_start = AsyncMock()
|
||||
mock_plug.on_print_start = AsyncMock()
|
||||
mock_ws.send_print_start = AsyncMock()
|
||||
mock_ws.send_archive_updated = AsyncMock()
|
||||
mock_relay.on_print_start = AsyncMock()
|
||||
mock_pm.get_printer = MagicMock(return_value=MagicMock(name="Test", serial_number="TEST123"))
|
||||
|
||||
from backend.app.main import on_print_start
|
||||
|
||||
await on_print_start(
|
||||
1,
|
||||
{
|
||||
"filename": "bambu_lab_a1_tool_plate_3.gcode.3mf",
|
||||
"subtask_name": "bambu_lab_a1_tool_plate_3",
|
||||
},
|
||||
)
|
||||
|
||||
assert mock_archive.printer_id == 1, (
|
||||
"expected-archive branch must assign the running printer_id so the "
|
||||
"post-print timelapse-scan path (gated on archive.printer_id) works"
|
||||
)
|
||||
assert mock_archive.status == "printing"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_expected_archive_path_preserves_existing_printer_id():
|
||||
"""Defensive: if the archive already carries a printer_id (e.g. a
|
||||
library-file-based queue item created with the printer pre-assigned),
|
||||
don't clobber it with a stale value. The branch is idempotent on
|
||||
correct data."""
|
||||
mock_printer = MagicMock()
|
||||
mock_printer.id = 7
|
||||
mock_printer.auto_archive = True
|
||||
mock_printer.external_camera_enabled = False
|
||||
mock_printer.external_camera_url = None
|
||||
mock_printer.name = "TestP1S"
|
||||
|
||||
mock_archive = MagicMock()
|
||||
mock_archive.id = 99
|
||||
mock_archive.filename = "MyModel.3mf"
|
||||
mock_archive.subtask_id = None
|
||||
mock_archive.print_time_seconds = None
|
||||
mock_archive.created_by_id = None
|
||||
mock_archive.printer_id = 7 # already correct
|
||||
mock_archive.print_name = "MyModel"
|
||||
mock_archive.status = "archived"
|
||||
mock_archive.file_path = "/tmp/fake.3mf"
|
||||
mock_archive.energy_start_kwh = None
|
||||
|
||||
register_expected_print(7, "MyModel.3mf", archive_id=99, ams_mapping=None)
|
||||
|
||||
def execute_router(stmt, *args, **kwargs):
|
||||
sql = str(stmt).lower()
|
||||
if "from printers" in sql or "from printer " in sql:
|
||||
return MagicMock(
|
||||
scalar_one_or_none=MagicMock(return_value=mock_printer),
|
||||
scalars=MagicMock(return_value=MagicMock(all=MagicMock(return_value=[mock_printer]))),
|
||||
)
|
||||
if "from print_archives" in sql or "from print_archive" in sql:
|
||||
return MagicMock(
|
||||
scalar_one_or_none=MagicMock(return_value=mock_archive),
|
||||
scalars=MagicMock(return_value=MagicMock(all=MagicMock(return_value=[mock_archive]))),
|
||||
)
|
||||
return MagicMock(
|
||||
scalar_one_or_none=MagicMock(return_value=None),
|
||||
scalars=MagicMock(return_value=MagicMock(all=MagicMock(return_value=[]))),
|
||||
)
|
||||
|
||||
mock_session = AsyncMock()
|
||||
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
|
||||
mock_session.__aexit__ = AsyncMock()
|
||||
mock_session.execute = AsyncMock(side_effect=execute_router)
|
||||
mock_session.commit = AsyncMock()
|
||||
|
||||
with (
|
||||
patch("backend.app.main.async_session") as mock_session_maker,
|
||||
patch("backend.app.main.notification_service") as mock_notif,
|
||||
patch("backend.app.main.smart_plug_manager") as mock_plug,
|
||||
patch("backend.app.main.ws_manager") as mock_ws,
|
||||
patch("backend.app.main.printer_manager") as mock_pm,
|
||||
patch("backend.app.main.mqtt_relay") as mock_relay,
|
||||
patch("backend.app.main._record_energy_start", new_callable=AsyncMock),
|
||||
patch("backend.app.main._load_objects_from_archive"),
|
||||
patch("backend.app.main._store_spoolman_print_data", new_callable=AsyncMock),
|
||||
patch("backend.app.main._send_print_start_notification", new_callable=AsyncMock),
|
||||
):
|
||||
mock_session_maker.return_value = mock_session
|
||||
mock_notif.on_print_start = AsyncMock()
|
||||
mock_plug.on_print_start = AsyncMock()
|
||||
mock_ws.send_print_start = AsyncMock()
|
||||
mock_ws.send_archive_updated = AsyncMock()
|
||||
mock_relay.on_print_start = AsyncMock()
|
||||
mock_pm.get_printer = MagicMock(return_value=MagicMock(name="Test", serial_number="TEST123"))
|
||||
|
||||
from backend.app.main import on_print_start
|
||||
|
||||
await on_print_start(7, {"filename": "MyModel.3mf", "subtask_name": "MyModel"})
|
||||
|
||||
assert mock_archive.printer_id == 7
|
||||
assert mock_archive.status == "printing"
|
||||
Reference in New Issue
Block a user