From f95c81e6cdafad5e9d87711cfef22ce5d98137ed Mon Sep 17 00:00:00 2001 From: Kouki Ojima Date: Mon, 24 Aug 2026 18:51:22 +0900 Subject: [PATCH] Take an RFID spool's core weight from the row that names it (issue #2909) (#2923) --- backend/app/core/database.py | 13 +- backend/app/services/spool_tag_matcher.py | 39 ++++- .../unit/services/test_spool_tag_matcher.py | 144 +++++++++++++++++- 3 files changed, 186 insertions(+), 10 deletions(-) diff --git a/backend/app/core/database.py b/backend/app/core/database.py index 0b4553377..420bf8284 100644 --- a/backend/app/core/database.py +++ b/backend/app/core/database.py @@ -4548,6 +4548,15 @@ async def _migrate_repair_rfid_core_weight(conn) -> None: """ from sqlalchemy import bindparam, text + # The same two values the creating path uses, imported rather than repeated: + # a repair that looked for a different row than the code writes would leave + # the tare it was built to correct in place. Imported inside the function to + # keep this module free of a service-layer dependency at import time. + from backend.app.services.spool_tag_matcher import ( + BAMBU_PLASTIC_SPOOL_CATALOG_NAME, + BAMBU_PLASTIC_SPOOL_CORE_WEIGHT, + ) + flag = "_backfill_2909_rfid_core_weight_done" async with conn.begin_nested(): @@ -4563,11 +4572,11 @@ async def _migrate_repair_rfid_core_weight(conn) -> None: correct = ( await conn.execute( text("SELECT id, weight FROM spool_catalog WHERE UPPER(name) = :name ORDER BY id LIMIT 1"), - {"name": "BAMBU LAB - PLASTIC LOW TEMP"}, + {"name": BAMBU_PLASTIC_SPOOL_CATALOG_NAME.upper()}, ) ).fetchone() correct_id = correct[0] if correct else None - correct_weight = correct[1] if correct else 250 + correct_weight = correct[1] if correct else BAMBU_PLASTIC_SPOOL_CORE_WEIGHT bambu_weights = { row[0] diff --git a/backend/app/services/spool_tag_matcher.py b/backend/app/services/spool_tag_matcher.py index 0548e412e..4fb7476e9 100644 --- a/backend/app/services/spool_tag_matcher.py +++ b/backend/app/services/spool_tag_matcher.py @@ -20,6 +20,13 @@ logger = logging.getLogger(__name__) ZERO_TAG_UID = "0000000000000000" ZERO_TRAY_UUID = "00000000000000000000000000000000" +# Spool catalog row describing the reusable plastic spool Bambu Lab ships filament +# on, and the weight to assume when that row is absent. DEFAULT_SPOOL_CATALOG holds +# three "Bambu Lab%" rows (High Temp 216, Low Temp 250, White 253); this is the one +# that matches the spool an RFID roll actually arrives on. +BAMBU_PLASTIC_SPOOL_CATALOG_NAME = "Bambu Lab - Plastic Low Temp" +BAMBU_PLASTIC_SPOOL_CORE_WEIGHT = 250 + def is_valid_tag(tag_uid: str, tray_uuid: str) -> bool: """Check if a tag/UUID pair contains a non-zero, non-empty value.""" @@ -168,13 +175,30 @@ async def create_spool_from_tray(db: AsyncSession, tray_data: dict) -> Spool: continue break - # Look up core weight from spool catalog - core_weight = 250 # Default for Bambu Lab plastic spools - cat_result = await db.execute(select(SpoolCatalogEntry).where(SpoolCatalogEntry.name.ilike("Bambu Lab%")).limit(10)) - for entry in cat_result.scalars().all(): - # Pick the best match (prefer exact, fallback to first Bambu Lab entry) - core_weight = entry.weight - break + # Look up core weight from the spool catalog by exact name. The previous + # "Bambu Lab%" prefix query had no matching step and no ORDER BY, so it took + # whichever of the three Bambu Lab rows the database returned first — High Temp + # (216 g) on SQLite, undefined on Postgres once the table has seen updates. + # core_weight is the tare in SpoolBuddy's weigh flow, so a wrong value here + # silently biases every scale weighing of an RFID-created spool. See #2909. + # + # Falling back to the constant rather than to another catalog row keeps a + # missing or renamed entry from reintroducing the arbitrary pick, and matching + # by name means a user who has corrected that row to their own measurement gets + # their value. + core_weight = BAMBU_PLASTIC_SPOOL_CORE_WEIGHT + core_weight_catalog_id = None + cat_query = ( + select(SpoolCatalogEntry) + .where(func.upper(SpoolCatalogEntry.name) == BAMBU_PLASTIC_SPOOL_CATALOG_NAME.upper()) + .order_by(SpoolCatalogEntry.id) + .limit(1) + ) + cat_result = await db.execute(cat_query) + catalog_entry = cat_result.scalar_one_or_none() + if catalog_entry: + core_weight = catalog_entry.weight + core_weight_catalog_id = catalog_entry.id # Resolve slicer filament name from builtin table slicer_filament_name = None @@ -210,6 +234,7 @@ async def create_spool_from_tray(db: AsyncSession, tray_data: dict) -> Spool: brand="Bambu Lab", label_weight=label_weight, core_weight=core_weight, + core_weight_catalog_id=core_weight_catalog_id, weight_used=weight_used, slicer_filament=tray_info_idx or None, slicer_filament_name=slicer_filament_name, diff --git a/backend/tests/unit/services/test_spool_tag_matcher.py b/backend/tests/unit/services/test_spool_tag_matcher.py index 6c3e20426..edfaf80df 100644 --- a/backend/tests/unit/services/test_spool_tag_matcher.py +++ b/backend/tests/unit/services/test_spool_tag_matcher.py @@ -1,11 +1,12 @@ """Tests for spool_tag_matcher service — RFID auto-assign and relationship loading.""" import pytest -from sqlalchemy import inspect +from sqlalchemy import inspect, select from backend.app.models.color_catalog import ColorCatalogEntry from backend.app.models.spool import Spool from backend.app.models.spool_assignment import SpoolAssignment +from backend.app.models.spool_catalog import SpoolCatalogEntry from backend.app.services.spool_tag_matcher import ( auto_assign_spool, create_spool_from_tray, @@ -1344,6 +1345,147 @@ async def test_find_matching_untagged_gradient_no_match_basic(db_session): assert found is None +# -- core weight catalog lookup (#2909) -------------------------------------- + + +async def _seed_bambu_spool_catalog(db_session): + """Seed the three Bambu Lab rows in DEFAULT_SPOOL_CATALOG order. + + High Temp is inserted first on purpose: that is the row the old prefix query + returned, so any test that expects 250 here would have failed before the fix. + """ + for name, weight in ( + ("Bambu Lab - Plastic High Temp", 216), + ("Bambu Lab - Plastic Low Temp", 250), + ("Bambu Lab - Plastic White", 253), + ): + db_session.add(SpoolCatalogEntry(name=name, weight=weight, is_default=True)) + await db_session.flush() + + +@pytest.mark.asyncio +async def test_core_weight_matches_low_temp_row_not_first_bambu_row(db_session): + """Regression for #2909 — the lookup must select 'Bambu Lab - Plastic Low Temp' + by name, not take whichever 'Bambu Lab%' row the database returns first. + + Before the fix this asserted 216 (High Temp), because the query had neither a + matching step nor an ORDER BY. core_weight is the tare in SpoolBuddy's weigh + flow, so the 34 g error propagated into every scale weighing of an RFID-created + spool. + """ + await _seed_bambu_spool_catalog(db_session) + + spool = await create_spool_from_tray(db_session, SAMPLE_TRAY) + + assert spool.core_weight == 250 + + +@pytest.mark.asyncio +async def test_core_weight_records_the_catalog_row_it_used(db_session): + """core_weight_catalog_id records which catalog row supplied the weight. + + The RFID path never set it, and the spool form does not leave that blank: it + auto-selects whenever exactly one catalog row matches the weight, and shows + the first matching row's name otherwise. So an arbitrary tare was displayed + as a named row and written back as that row's id on the next save of any + field -- a wrong number laundered into what reads like a deliberate pick. + Naming the row here is what stops the form having to infer it. + """ + await _seed_bambu_spool_catalog(db_session) + + spool = await create_spool_from_tray(db_session, SAMPLE_TRAY) + + low_temp = ( + await db_session.execute( + select(SpoolCatalogEntry).where(SpoolCatalogEntry.name == "Bambu Lab - Plastic Low Temp") + ) + ).scalar_one() + assert spool.core_weight_catalog_id == low_temp.id + + +@pytest.mark.asyncio +async def test_core_weight_honours_a_user_edited_catalog_row(db_session): + """The catalog is user-editable, and someone who has weighed their own empty + spool should get their number rather than the shipped default. + """ + await _seed_bambu_spool_catalog(db_session) + row = ( + await db_session.execute( + select(SpoolCatalogEntry).where(SpoolCatalogEntry.name == "Bambu Lab - Plastic Low Temp") + ) + ).scalar_one() + row.weight = 244 + await db_session.flush() + + spool = await create_spool_from_tray(db_session, SAMPLE_TRAY) + + assert spool.core_weight == 244 + assert spool.core_weight_catalog_id == row.id + + +@pytest.mark.asyncio +async def test_core_weight_falls_back_to_default_when_row_is_missing(db_session): + """A renamed or deleted row must fall back to the 250 g constant, not to some + other Bambu Lab row — falling back to a row is how the arbitrary pick started. + """ + db_session.add(SpoolCatalogEntry(name="Bambu Lab - Plastic High Temp", weight=216, is_default=True)) + await db_session.flush() + + spool = await create_spool_from_tray(db_session, SAMPLE_TRAY) + + assert spool.core_weight == 250 + assert spool.core_weight_catalog_id is None + + +@pytest.mark.asyncio +async def test_core_weight_is_deterministic_when_the_name_is_duplicated(db_session): + """The catalogue is user-editable and nothing stops two rows sharing a name. + + Without order_by(id).limit(1) this is not merely non-deterministic: + scalar_one_or_none() raises MultipleResultsFound, so an AMS read would fail + outright rather than pick badly. The lowest id wins, which is the same + deterministic tiebreak the colour catalogue lookup above already uses. + """ + await _seed_bambu_spool_catalog(db_session) + first = ( + await db_session.execute( + select(SpoolCatalogEntry).where(SpoolCatalogEntry.name == "Bambu Lab - Plastic Low Temp") + ) + ).scalar_one() + db_session.add(SpoolCatalogEntry(name="Bambu Lab - Plastic Low Temp", weight=999, is_default=False)) + await db_session.flush() + + spool = await create_spool_from_tray(db_session, SAMPLE_TRAY) + + assert spool.core_weight == 250 + assert spool.core_weight_catalog_id == first.id + + +@pytest.mark.asyncio +async def test_core_weight_matches_the_row_name_case_insensitively(db_session): + """Matching mirrors the colour catalogue lookup above, which compares through + func.upper(). A user who has retyped the row's name in different case still + gets their row rather than silently dropping to the fallback constant. + """ + db_session.add(SpoolCatalogEntry(name="bambu lab - plastic low temp", weight=248, is_default=False)) + await db_session.flush() + + spool = await create_spool_from_tray(db_session, SAMPLE_TRAY) + + assert spool.core_weight == 248 + + +@pytest.mark.asyncio +async def test_core_weight_falls_back_on_empty_catalog(db_session): + """No catalog at all (fresh install before seeding) still produces a usable + spool rather than raising. + """ + spool = await create_spool_from_tray(db_session, SAMPLE_TRAY) + + assert spool.core_weight == 250 + assert spool.core_weight_catalog_id is None + + # -- auto_assign_spool: live cali_idx fallback (P9-3) -------------------------