public_git/bambuddy
1
0
Fork 0
mirror of https://github.com/maziggy/bambuddy.git synced 2026-10-20 09:03:43 +02:00
Code Issues Packages Projects Releases Wiki Activity

Keep both modes' slot assignments across an inventory mode switch (issue #2812)

Browse Source 
Turning Spoolman mode on ran an unfiltered delete(SpoolAssignment) across every
printer. Turning it straight back off cleared the other table instead, so the
two directions were symmetric in code and one-way in effect, and the setting
auto-saves on a 500 ms debounce with no save button and no confirmation.
Opening the settings page to see what the option did was enough to destroy the
configuration: the reporter's log shows four toggles in 85 seconds, which is
someone looking and reverting, and the assignments never came back.

The deletion was not careless. Checks that read both assignment tables would
otherwise let a row in the mode you are not using answer for the mode you are,
which is how #1473 was fixed, and emptying the inactive table made that
impossible by construction. The cost was that the guarantee was bought with the
user's data. That decision belongs to the readers -- the mode is a property of
the install, not of the rows -- so spoolman_owns_assignments now answers it and
nothing is deleted on a toggle. Each mode keeps its own assignments and
switching is reversible. Existing installs need no migration: their inactive
table is already empty, because it was being emptied.

Six sites had to be told which mode they meant, and only two of them are the
reads you would guess at, the missing-assignment notification and the queue cost
estimate. The per-slot K-profile lookup consults the built-in table first and,
on a hit with no matching profile, deliberately stops rather than falling
through to Spoolman, so a leftover row would have shadowed the Spoolman binding
for that slot -- the symptom #1556 reported from the other direction.
configure_ams_slot *writes* a K-profile against whichever table answers first,
so the same leftover would have filed a calibration against a spool the printer
is not drawing on and never written the local one, leaving a calibration that
appeared to succeed and then did not apply.

The auto-unlink pass in on_ams_change is the one that would have made this
change worthless. It drops any assignment whose tray no longer matches the
fingerprint it recorded, and it ends in db.delete. Ungated, it would have
removed the preserved rows one slot at a time as the AMS contents changed under
the other mode -- the same loss, arriving slowly enough not to be connected to
the toggle that caused it.

The sixth is the built-in remaining-weight fallback inside the Spoolman AMS
sync, and it is deliberately left inert rather than woken up. It could never
fire while the table it reads was being emptied, it is keyed by slot rather than
by spool, and create_spool writes remaining_weight unconditionally where the
update path does not -- so preserving the rows would have seeded a stale figure
into a brand new Spoolman spool the first time a tray reported an unusable
remain%. The query stays, gated off, so the intent survives for whoever
revisits the cross-mode fallback.

Separately, a print that could not debit a spool said nothing about it, and that
is what turned a mis-click into lost filament. The reporter's print was already
running when they toggled. At completion it resolved its 3MF, read its
per-filament grams, resolved its tray, and then skipped the debit because the
assignment row no longer existed -- logged at INFO, invisible under the default
log level, while the completion notification fired as usual. 65.49 g was never
deducted and they only noticed because a spool's remaining weight looked wrong.
_resolve_spool_id_for_tray has no tag or fingerprint fallback, so there was
nothing else to catch it.

The skip is now a warning naming the grams, and a completed print that failed to
charge a tray it drew from raises the missing-spool-assignment notification. The
print-start check cannot cover this and was right to stay quiet: the assignments
existed when it ran. The two are different statements -- the first says the
weight may not be tracked, the second says it was not -- so a print warned at
start will notify twice, which is the right trade. Collected across the print
rather than fired per slot, and given the caller's session, because this runs
inside on_print_complete's transaction and opening a second one to read the
printer's name would deadlock against it on SQLite. This is independent of the
toggle and catches any other cause of an assignment disappearing mid-print.
This commit is contained in:
maziggy
2026-08-25 17:17:26 +02:00
parent 39835437a3
commit 08a58b1ef4
14 changed files with 760 additions and 122 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/app/api/routes/printers.py
+25 -10
View File
@@ -2935,15 +2935,27 @@ async def configure_ams_slot(
# tell, and on those machines extruder 0 is the only one there is.
kp_extruder = resolved_extruder if resolved_extruder is not None else 0
# Spoolman SlotAssignment first — has UniqueConstraint, idempotent.
sm_result = await db.execute(
select(SpoolmanSlotAssignment).where(
SpoolmanSlotAssignment.printer_id == printer_id,
SpoolmanSlotAssignment.ams_id == ams_id,
SpoolmanSlotAssignment.tray_id == tray_id,
# Only the active mode's assignment table decides where this
# K-profile is stored. Reading Spoolman first and falling through
# was safe while the inactive table was emptied on every mode
# toggle; nothing is emptied since #2812, so a leftover Spoolman
# row in built-in mode would file the calibration against a spool
# the printer is not using and never write the local profile —
# the calibration would appear to succeed and then not apply.
from backend.app.services.inventory_mode import spoolman_owns_assignments
spoolman_mode = await spoolman_owns_assignments(db)
sm_assignment = None
if spoolman_mode:
# Spoolman SlotAssignment — has UniqueConstraint, idempotent.
sm_result = await db.execute(
select(SpoolmanSlotAssignment).where(
SpoolmanSlotAssignment.printer_id == printer_id,
SpoolmanSlotAssignment.ams_id == ams_id,
SpoolmanSlotAssignment.tray_id == tray_id,
)
)
)
sm_assignment = sm_result.scalar_one_or_none()
sm_assignment = sm_result.scalar_one_or_none()
if sm_assignment:
existing = await db.execute(
select(SpoolmanKProfile).where(
@@ -2981,8 +2993,11 @@ async def configure_ams_slot(
tray_id,
cali_idx,
)
else:
# Local SpoolAssignment + SpoolKProfile (no UNIQUE — use .first())
elif not spoolman_mode:
# Local SpoolAssignment + SpoolKProfile (no UNIQUE — use .first()).
# Skipped in Spoolman mode even when a local row survives: the
# profile would be filed against a spool this printer is not
# drawing on, and the mode's own table has nothing to bind to.
local_result = await db.execute(
select(SpoolAssignment)
.options(selectinload(SpoolAssignment.spool))
backend/app/api/routes/settings.py
+19 -17
View File
@@ -8,7 +8,7 @@ from pathlib import Path
from fastapi import APIRouter, Depends, File, HTTPException, UploadFile
from fastapi.responses import FileResponse, JSONResponse
from pydantic import BaseModel, Field
from sqlalchemy import delete, func, select
from sqlalchemy import func, select
from sqlalchemy.ext.asyncio import AsyncSession
from backend.app.core.auth import RequirePermissionIfAuthEnabled, caller_is_api_key, require_energy_cost_update
@@ -552,22 +552,24 @@ async def update_spoolman_settings(
now_enabled = new_val == "true"
await set_setting(db, "spoolman_enabled", new_val)
# Switching to Spoolman: clear built-in inventory slot assignments
if not was_enabled and now_enabled:
from backend.app.models.spool_assignment import SpoolAssignment
result = await db.execute(delete(SpoolAssignment))
logger.info("Cleared %d spool assignments on switch to Spoolman mode", result.rowcount)
# Switching back to internal mode: clear Spoolman slot assignments — the
# symmetric counterpart of the clear above. Without this, stale
# spoolman_slot_assignments rows linger and would wrongly count as
# "assigned" in any mode-agnostic check (e.g. the missing-spool-
# assignment notification, which unions both tables — #1473).
elif was_enabled and not now_enabled:
from backend.app.models.spoolman_slot_assignment import SpoolmanSlotAssignment
result = await db.execute(delete(SpoolmanSlotAssignment))
logger.info("Cleared %d Spoolman slot assignments on switch to internal mode", result.rowcount)
# Nothing is deleted on a mode change (#2812). Each mode keeps its slot
# assignments in its own table, so both can hold rows at once and the
# toggle is reversible: switching to Spoolman to see what it does, then
# switching back, returns you to the assignments you had.
#
# This used to empty the other mode's table on every toggle. The reason
# was real -- checks that read both tables would let a row in the mode
# you are not using answer for the mode you are -- but the cost was that
# inspecting a mode destroyed your configuration, with no confirmation
# and no way back, and the deletion was unfiltered across every printer.
# The readers that could be confused now ask which mode is active
# (``spoolman_owns_assignments``), which is where that decision belongs:
# the mode is a property of the install, not of the rows.
if was_enabled != now_enabled:
logger.info(
"Inventory mode switched to %s; slot assignments in both tables kept",
"Spoolman" if now_enabled else "built-in",
)
if "spoolman_url" in settings:
await set_setting(db, "spoolman_url", normalize_str_setting("spoolman_url", settings["spoolman_url"]))
if "spoolman_sync_mode" in settings: