Ver código fonte

Take an RFID spool's core weight from the row that names it (issue #2909) (#2923)

Kouki Ojima 1 semana atrás
pai
commit
f95c81e6cd

+ 11 - 2
backend/app/core/database.py

@@ -4548,6 +4548,15 @@ async def _migrate_repair_rfid_core_weight(conn) -> None:
     """
     """
     from sqlalchemy import bindparam, text
     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"
     flag = "_backfill_2909_rfid_core_weight_done"
 
 
     async with conn.begin_nested():
     async with conn.begin_nested():
@@ -4563,11 +4572,11 @@ async def _migrate_repair_rfid_core_weight(conn) -> None:
         correct = (
         correct = (
             await conn.execute(
             await conn.execute(
                 text("SELECT id, weight FROM spool_catalog WHERE UPPER(name) = :name ORDER BY id LIMIT 1"),
                 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()
         ).fetchone()
         correct_id = correct[0] if correct else None
         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 = {
         bambu_weights = {
             row[0]
             row[0]

+ 32 - 7
backend/app/services/spool_tag_matcher.py

@@ -20,6 +20,13 @@ logger = logging.getLogger(__name__)
 ZERO_TAG_UID = "0000000000000000"
 ZERO_TAG_UID = "0000000000000000"
 ZERO_TRAY_UUID = "00000000000000000000000000000000"
 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:
 def is_valid_tag(tag_uid: str, tray_uuid: str) -> bool:
     """Check if a tag/UUID pair contains a non-zero, non-empty value."""
     """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
                 continue
             break
             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
     # Resolve slicer filament name from builtin table
     slicer_filament_name = None
     slicer_filament_name = None
@@ -210,6 +234,7 @@ async def create_spool_from_tray(db: AsyncSession, tray_data: dict) -> Spool:
         brand="Bambu Lab",
         brand="Bambu Lab",
         label_weight=label_weight,
         label_weight=label_weight,
         core_weight=core_weight,
         core_weight=core_weight,
+        core_weight_catalog_id=core_weight_catalog_id,
         weight_used=weight_used,
         weight_used=weight_used,
         slicer_filament=tray_info_idx or None,
         slicer_filament=tray_info_idx or None,
         slicer_filament_name=slicer_filament_name,
         slicer_filament_name=slicer_filament_name,

+ 143 - 1
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."""
 """Tests for spool_tag_matcher service — RFID auto-assign and relationship loading."""
 
 
 import pytest
 import pytest
-from sqlalchemy import inspect
+from sqlalchemy import inspect, select
 
 
 from backend.app.models.color_catalog import ColorCatalogEntry
 from backend.app.models.color_catalog import ColorCatalogEntry
 from backend.app.models.spool import Spool
 from backend.app.models.spool import Spool
 from backend.app.models.spool_assignment import SpoolAssignment
 from backend.app.models.spool_assignment import SpoolAssignment
+from backend.app.models.spool_catalog import SpoolCatalogEntry
 from backend.app.services.spool_tag_matcher import (
 from backend.app.services.spool_tag_matcher import (
     auto_assign_spool,
     auto_assign_spool,
     create_spool_from_tray,
     create_spool_from_tray,
@@ -1344,6 +1345,147 @@ async def test_find_matching_untagged_gradient_no_match_basic(db_session):
     assert found is None
     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) -------------------------
 # -- auto_assign_spool: live cali_idx fallback (P9-3) -------------------------