From af52c4f2ff3abbe4417d90d127ace7be0b092136 Mon Sep 17 00:00:00 2001 From: maziggy Date: Tue, 12 May 2026 08:48:30 +0200 Subject: [PATCH] fix(spoolman): allow AMS-HT ams_id range in slot-assignment table (#1274) H2C / H2D AMS-HT units report ams_id 128+ (one ams_id per unit, single tray), but spoolman_slot_assignments.ck_ams_id_range only admitted 0-7 and 255. Every attempt to link a Spoolman spool to an AMS-HT slot died with `CHECK constraint failed: ck_ams_id_range`. The internal spool_assignment table has no such constraint and works fine. Widen the formula to (0-7) OR (128-191) OR 255 in the model, the CREATE TABLE DDL, and an idempotent in-place migration for existing installs (Postgres: DROP/ADD CONSTRAINT; SQLite: detect stale formula in sqlite_master, rebuild via _v2 rename pattern). --- CHANGELOG.md | 2 + backend/app/core/database.py | 107 +++++++++++++++++- .../app/models/spoolman_slot_assignment.py | 9 +- .../test_spoolman_slot_assignments.py | 26 +++++ backend/tests/unit/test_spoolman_slot_ddl.py | 24 ++++ 5 files changed, 165 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9dceb4de4..9a2f056c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,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 +- **Linking a Spoolman spool to an AMS-HT slot no longer fails with a CHECK constraint error** ([#1274](https://github.com/maziggy/bambuddy/issues/1274), reported by guillaume.houba) — On H2C / H2D, AMS-HT units report `ams_id` 128+ (one ams_id per unit, single tray). The `spoolman_slot_assignments` table's `ck_ams_id_range` constraint only allowed 0-7 (standard AMS) or 255 (external), so the upsert on `POST /spoolman/inventory/slot-assignments` blew up with `IntegrityError: CHECK constraint failed: ck_ams_id_range` and the user had no way to link any spool to an AMS-HT slot. Widened the constraint formula to `(ams_id >= 0 AND ams_id <= 7) OR (ams_id >= 128 AND ams_id <= 191) OR ams_id = 255` — matches the value range the internal `spool_assignment` table already accepts and leaves room for up to 64 AMS-HT units (the existing `bambu_mqtt`/usage-tracker code uses the same 128-based addressing). Updated in the ORM model (`models/spoolman_slot_assignment.py`) and both the SQLite/Postgres `CREATE TABLE` DDL in `core/database.py`. New idempotent migration `_migrate_widen_spoolman_slot_ams_id_range`: Postgres path runs `DROP CONSTRAINT IF EXISTS` + `ADD CONSTRAINT` (no data risk — the new formula is strictly wider than the old); SQLite path detects the stale formula in `sqlite_master`, table-rebuilds via the standard `_v2` rename pattern used elsewhere in this file (`_migrate_update_auto_link_constraint` at `database.py:418`), and leaves pre-constraint legacy tables untouched. Tests: `test_ams_id_check_admits_ams_ht_range` (ORM + DDL formula) and `test_assign_accepts_ams_ht_id` (end-to-end `POST /slot-assignments` with `ams_id=128`). + - **X2D live camera stream no longer cut by Obico polling / snapshot capture** ([#1271](https://github.com/maziggy/bambuddy/issues/1271), reported by @clabeuhtegrite) — The MJPEG fan-out broadcaster from #1089 lets multiple browser viewers share one upstream RTSP socket per printer, but internal callers (Obico AI polling at the user's configured `obico_poll_interval`, and the manual `/camera/snapshot` endpoint) still opened their own fresh RTSP connections. X1C / H2D / P2S firmware tolerates brief concurrent camera sockets so the gap was invisible there. X2D firmware `01.01.00.00` (and likely future firmwares) enforces strict single-camera-connection more aggressively: every Obico poll (default every 5 s) kicked the live stream, the broadcaster paid the multi-second RTSP handshake to reconnect, and the user saw the stream cut "all the time." New helper `try_get_active_buffered_frame(printer_id)` at [`api/routes/camera.py:74`](backend/app/api/routes/camera.py) returns the broadcaster's last buffered frame (always <1 s old while any viewer is connected) and `None` when no viewer is active. Obico's `_capture_frame` and the `/camera/snapshot` endpoint check it first and only fall through to a fresh socket when no stream is running — preserving today's behavior when nobody is watching. `plate_detection` and `layer_timelapse` deliberately not converted: plate-detection needs guaranteed-fresh frames post-print (false-positive risk if the user already grabbed the print in the same second), and layer-timelapse is for external cameras only. Regression tests: `test_camera_snapshot_reuses_buffered_frame_when_stream_active` and two `TestCaptureFrameSharesBroadcasterUpstream` Obico tests. - **Usage tracker: spool swaps in UNUSED slots mid-print no longer charge the old spool** ([#1269](https://github.com/maziggy/bambuddy/issues/1269), reported by @maugsburger) — Path 2 of the usage tracker (AMS remain% delta fallback) iterated every AMS tray that had a remain% delta, even slots the print never touched. When a user swapped spools in an unrelated slot during a print, the new spool reports `remain=0` (no RFID tag yet) while the snapshot from print-start was 100%, so the fallback charged the originally-assigned spool the full 1000 g. Reporter's case: single-filament print on AMS0-T3 (`ams_mapping=[3]`), swapped a spool in T1 and another in T2 to refill while the print continued — wound up with `Spool 27 consumed 1000.0g (100%) on printer 1 AMS0-T1` and `Spool 24 consumed 170.0g (17%) on printer 1 AMS0-T2`, neither of which were ever in the print. Fix: the fallback now builds `print_used_keys` from `session.ams_mapping`, `state.tray_change_log`, and `session.tray_now_at_start` (the three runtime signals telling us which trays were actually part of the print), converts each global tray ID to `(ams_id, tray_id)` using the standard convention (254/255 → external, ≥128 → AMS-HT, otherwise `id // 4, id % 4`), and skips fallback for trays whose key is not in that set. When all three signals are empty (legacy edge case: no slicer push, no MQTT tray-change events, no `tray_now` at start) the legacy "scan every tray" behavior is preserved so we don't regress prints with no metadata. Regression test in `test_usage_tracker.py::test_skips_fallback_for_trays_outside_print_mapping` reproduces the reporter's exact scenario. diff --git a/backend/app/core/database.py b/backend/app/core/database.py index e666c0d62..a019f8172 100644 --- a/backend/app/core/database.py +++ b/backend/app/core/database.py @@ -502,6 +502,103 @@ async def _migrate_update_auto_link_constraint(conn) -> None: raise +async def _migrate_widen_spoolman_slot_ams_id_range(conn) -> None: + """Widen ck_ams_id_range on spoolman_slot_assignments to admit AMS-HT (#1274). + + Old formula: (ams_id >= 0 AND ams_id <= 7) OR ams_id = 255 + New formula: (ams_id >= 0 AND ams_id <= 7) OR (ams_id >= 128 AND ams_id <= 191) OR ams_id = 255 + + The H2C/H2D AMS-HT reports ams_id 128+. The old constraint rejected every + AMS-HT slot link with `IntegrityError: CHECK constraint failed: ck_ams_id_range`. + + PostgreSQL: DROP CONSTRAINT IF EXISTS + ADD new formula via _safe_execute. + SQLite: table recreation when the old (narrower) formula is detected in + sqlite_master. Fresh installs already have the widened constraint from + the CREATE TABLE migration above. + """ + from sqlalchemy import text + + _NEW_FORMULA = "(ams_id >= 0 AND ams_id <= 7) OR (ams_id >= 128 AND ams_id <= 191) OR ams_id = 255" + _CONSTRAINT_NAME = "ck_ams_id_range" + + if not is_sqlite(): + await _safe_execute( + conn, + f"ALTER TABLE spoolman_slot_assignments DROP CONSTRAINT IF EXISTS {_CONSTRAINT_NAME}", + ) + await _safe_execute( + conn, + f"ALTER TABLE spoolman_slot_assignments ADD CONSTRAINT {_CONSTRAINT_NAME} CHECK ({_NEW_FORMULA})", + ) + return + + row = ( + await conn.execute( + text("SELECT sql FROM sqlite_master WHERE type='table' AND name='spoolman_slot_assignments'") + ) + ).fetchone() + if not row: + return + sql = row[0] or "" + # Already widened by an earlier run or by the fresh-install CREATE TABLE above. + if "ams_id >= 128" in sql: + return + # Pre-migration table without any CHECK constraint at all → leave alone; + # the app-level validation handles correctness and we don't risk a + # destructive table rebuild for a constraint that isn't blocking anyone. + if "ck_ams_id_range" not in sql and "ams_id <= 7" not in sql: + return + + try: + async with conn.begin_nested(): + await conn.execute(text("DROP TABLE IF EXISTS spoolman_slot_assignments_v2")) + await conn.execute( + text( + "CREATE TABLE spoolman_slot_assignments_v2 (" + "id INTEGER PRIMARY KEY AUTOINCREMENT, " + "printer_id INTEGER NOT NULL REFERENCES printers(id) ON DELETE CASCADE, " + f"ams_id INTEGER NOT NULL CHECK ({_NEW_FORMULA}), " + "tray_id INTEGER NOT NULL CHECK (tray_id >= 0 AND tray_id <= 3), " + "spoolman_spool_id INTEGER NOT NULL, " + "assigned_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, " + "CONSTRAINT uq_slot_assignment UNIQUE(printer_id, ams_id, tray_id)" + ")" + ) + ) + await conn.execute( + text( + "INSERT INTO spoolman_slot_assignments_v2 " + "(id, printer_id, ams_id, tray_id, spoolman_spool_id, assigned_at) " + "SELECT id, printer_id, ams_id, tray_id, spoolman_spool_id, assigned_at " + "FROM spoolman_slot_assignments" + ) + ) + original = (await conn.execute(text("SELECT count(*) FROM spoolman_slot_assignments"))).scalar_one() + copied = (await conn.execute(text("SELECT count(*) FROM spoolman_slot_assignments_v2"))).scalar_one() + if copied != original: + raise RuntimeError( + f"spoolman_slot_assignments migration: row count mismatch after copy " + f"({original} in source, {copied} in copy)" + ) + await conn.execute(text("DROP TABLE spoolman_slot_assignments")) + await conn.execute(text("ALTER TABLE spoolman_slot_assignments_v2 RENAME TO spoolman_slot_assignments")) + # The index sits on the renamed table; recreate it idempotently + # to handle older sqlite versions that don't auto-rename indexes. + await conn.execute( + text( + "CREATE INDEX IF NOT EXISTS ix_slot_assignment_spool " + "ON spoolman_slot_assignments (spoolman_spool_id)" + ) + ) + except Exception as exc: + logger.error( + "spoolman_slot_assignments ck_ams_id_range widening (SQLite table recreation) FAILED: %s", + exc, + exc_info=True, + ) + raise + + async def run_migrations(conn): """Run all schema migrations and data backfills on startup. @@ -2039,13 +2136,14 @@ async def run_migrations(conn): # Migration: Create spoolman_slot_assignments table for local AMS-slot→Spoolman-spool mapping. # Replaces the pattern of writing spool.location in Spoolman (which polluted the # user-editable storage_location field in the UI). + # ck_ams_id_range formula was widened in #1274 to admit AMS-HT (ams_id 128-191). await _safe_execute( conn, """ CREATE TABLE IF NOT EXISTS spoolman_slot_assignments ( id INTEGER PRIMARY KEY AUTOINCREMENT, printer_id INTEGER NOT NULL REFERENCES printers(id) ON DELETE CASCADE, - ams_id INTEGER NOT NULL CHECK ((ams_id >= 0 AND ams_id <= 7) OR ams_id = 255), + ams_id INTEGER NOT NULL CHECK ((ams_id >= 0 AND ams_id <= 7) OR (ams_id >= 128 AND ams_id <= 191) OR ams_id = 255), tray_id INTEGER NOT NULL CHECK (tray_id >= 0 AND tray_id <= 3), spoolman_spool_id INTEGER NOT NULL, assigned_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, @@ -2057,7 +2155,7 @@ async def run_migrations(conn): CREATE TABLE IF NOT EXISTS spoolman_slot_assignments ( id SERIAL PRIMARY KEY, printer_id INTEGER NOT NULL REFERENCES printers(id) ON DELETE CASCADE, - ams_id INTEGER NOT NULL CHECK ((ams_id >= 0 AND ams_id <= 7) OR ams_id = 255), + ams_id INTEGER NOT NULL CHECK ((ams_id >= 0 AND ams_id <= 7) OR (ams_id >= 128 AND ams_id <= 191) OR ams_id = 255), tray_id INTEGER NOT NULL CHECK (tray_id >= 0 AND tray_id <= 3), spoolman_spool_id INTEGER NOT NULL, assigned_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, @@ -2070,6 +2168,11 @@ async def run_migrations(conn): "CREATE INDEX IF NOT EXISTS ix_slot_assignment_spool ON spoolman_slot_assignments (spoolman_spool_id)", ) + # Migration: widen ck_ams_id_range on spoolman_slot_assignments to allow + # AMS-HT ids (128-191). Existing installs created before #1274 carry the + # stale formula which rejects every AMS-HT slot link with a CHECK violation. + await _migrate_widen_spoolman_slot_ams_id_range(conn) + # Migration: Create spoolman_k_profile table for K-value calibration profiles linked to Spoolman spools. await _safe_execute( conn, diff --git a/backend/app/models/spoolman_slot_assignment.py b/backend/app/models/spoolman_slot_assignment.py index 67e498d66..73d29a48f 100644 --- a/backend/app/models/spoolman_slot_assignment.py +++ b/backend/app/models/spoolman_slot_assignment.py @@ -27,7 +27,14 @@ class SpoolmanSlotAssignment(Base): __table_args__ = ( UniqueConstraint("printer_id", "ams_id", "tray_id", name="uq_slot_assignment"), - CheckConstraint("(ams_id >= 0 AND ams_id <= 7) OR ams_id = 255", name="ck_ams_id_range"), + # 0-7: standard AMS units. 128-191: AMS-HT (each unit uses ams_id 128+, + # single tray). 255: external / VT tray. Matches the value range the + # internal `spool_assignment` table accepts. See #1274 — H2C with + # AMS-HT on the left nozzle reports ams_id=128. + CheckConstraint( + "(ams_id >= 0 AND ams_id <= 7) OR (ams_id >= 128 AND ams_id <= 191) OR ams_id = 255", + name="ck_ams_id_range", + ), CheckConstraint("tray_id >= 0 AND tray_id <= 3", name="ck_tray_id_range"), ) diff --git a/backend/tests/integration/test_spoolman_slot_assignments.py b/backend/tests/integration/test_spoolman_slot_assignments.py index 996ebc6c3..78fdd9c68 100644 --- a/backend/tests/integration/test_spoolman_slot_assignments.py +++ b/backend/tests/integration/test_spoolman_slot_assignments.py @@ -105,6 +105,32 @@ class TestAssignSpoolmanSlot: assert rows[0]["ams_id"] == 0 assert rows[0]["tray_id"] == 0 + @pytest.mark.asyncio + @pytest.mark.integration + async def test_assign_accepts_ams_ht_id(self, async_client: AsyncClient, slot_settings, test_printer, mock_client): + """#1274: AMS-HT units report ams_id 128+. The pre-fix ck_ams_id_range + only allowed 0-7 / 255, so the upsert blew up with `CHECK constraint + failed: ck_ams_id_range` and the user couldn't link any spool to the + H2C/H2D AMS-HT slot. This guards the widened range from regressing. + """ + response = await async_client.post( + "/api/v1/spoolman/inventory/slot-assignments", + json={ + "spoolman_spool_id": 51, + "printer_id": test_printer.id, + "ams_id": 128, # AMS-HT on the left nozzle (matches issue's failing INSERT) + "tray_id": 0, + }, + ) + + assert response.status_code == 200, response.text + all_resp = await async_client.get( + "/api/v1/spoolman/inventory/slot-assignments/all", + params={"printer_id": test_printer.id}, + ) + rows = all_resp.json() + assert any(r["ams_id"] == 128 and r["spoolman_spool_id"] == 51 for r in rows) + @pytest.mark.asyncio @pytest.mark.integration async def test_assign_does_not_call_update_spool( diff --git a/backend/tests/unit/test_spoolman_slot_ddl.py b/backend/tests/unit/test_spoolman_slot_ddl.py index 5fa401edd..a46aa0a6b 100644 --- a/backend/tests/unit/test_spoolman_slot_ddl.py +++ b/backend/tests/unit/test_spoolman_slot_ddl.py @@ -75,3 +75,27 @@ class TestSpoolmanSlotDdl: table = SpoolmanSlotAssignment.__table__ check_names = {c.name for c in table.constraints if isinstance(c, CheckConstraint)} assert "ck_tray_id_range" in check_names, f"ck_tray_id_range not in ORM check constraints: {check_names}" + + def test_ams_id_check_admits_ams_ht_range(self): + """#1274: ck_ams_id_range must accept AMS-HT (ams_id 128-191). + + H2C / H2D AMS-HT units report ams_id starting at 128 (one ams_id per + unit, single tray). The pre-fix constraint only allowed 0-7 and 255, + so every AMS-HT slot link failed with CHECK violation. Both the ORM + formula and the SQLite/Postgres DDL strings must include the new range. + """ + from sqlalchemy import CheckConstraint + + from backend.app.models.spoolman_slot_assignment import SpoolmanSlotAssignment + + table = SpoolmanSlotAssignment.__table__ + ams_check = next(c for c in table.constraints if isinstance(c, CheckConstraint) and c.name == "ck_ams_id_range") + formula = str(ams_check.sqltext) + assert "128" in formula and "191" in formula, ( + f"ck_ams_id_range formula does not cover AMS-HT range: {formula!r}" + ) + + # Same check at the raw-DDL level so a stale DDL definition can't + # silently ship with the loosened ORM formula. + ddl = _extract_spoolman_slot_ddl(is_sqlite=True) + assert "128" in ddl and "191" in ddl, "AMS-HT range missing from CREATE TABLE DDL"