Parcourir la source

fix(spoolman): per-print 3MF tracking is the only weight writer (#1119)

  Spoolman had two mutually-exclusive weight paths gated on the
  `disable_weight_sync` flag. The default (False) used AMS remain%
  x tray_weight auto-sync, which silently dropped non-BL spools
  because the AMS doesn't report tray_weight without RFID. The
  inventory_remaining fallback would have covered it, but the
  spool_assignment table it reads from is wiped on Spoolman
  activation, so non-BL spools got no weight updates at all.

  Match the internal Filament Inventory: per-print tracking always
  runs, AMS auto-sync no longer writes remaining_weight (it still
  maintains spool metadata and slot assignments). The setting
  becomes a no-op; left in the schema and UI for backwards compat.

  - store_print_data: drop the disable_weight_sync early return
  - sync_ams_tray callsites in main.py + routes/spoolman.py: force
    disable_weight_sync=True so weight is never written by AMS sync
  - new regression test confirming tracking runs with flag=false
maziggy il y a 2 semaines
Parent
commit
b334d7edc9

Fichier diff supprimé car celui-ci est trop grand
+ 1 - 0
CHANGELOG.md


+ 10 - 4
backend/app/api/routes/spoolman.py

@@ -178,8 +178,9 @@ async def sync_printer_ams(
 ):
     """Sync AMS data from a specific printer to Spoolman."""
     # Check if Spoolman is enabled and connected
+    # disable_weight_sync is deprecated (#1119); weight comes from per-print tracking.
     sm = await get_spoolman_settings(db)
-    enabled, url, disable_weight_sync = sm["enabled"], sm["url"], sm["disable_weight_sync"]
+    enabled, url = sm["enabled"], sm["url"]
     if not enabled:
         raise HTTPException(status_code=400, detail="Spoolman integration is not enabled")
 
@@ -318,7 +319,9 @@ async def sync_printer_ams(
                 sync_result = await client.sync_ams_tray(
                     tray,
                     printer.name,
-                    disable_weight_sync=disable_weight_sync,
+                    # Per-print tracking owns weight updates (#1119); manual sync
+                    # only refreshes spool metadata + slot assignments here.
+                    disable_weight_sync=True,
                     cached_spools=cached_spools,
                     inventory_remaining=inv_remaining,
                     spoolman_spool_id_hint=hint,
@@ -394,8 +397,9 @@ async def sync_all_printers(
 ):
     """Sync AMS data from all connected printers to Spoolman."""
     # Check if Spoolman is enabled
+    # disable_weight_sync is deprecated (#1119); weight comes from per-print tracking.
     sm = await get_spoolman_settings(db)
-    enabled, url, disable_weight_sync = sm["enabled"], sm["url"], sm["disable_weight_sync"]
+    enabled, url = sm["enabled"], sm["url"]
     if not enabled:
         raise HTTPException(status_code=400, detail="Spoolman integration is not enabled")
 
@@ -517,7 +521,9 @@ async def sync_all_printers(
                     sync_result = await client.sync_ams_tray(
                         tray,
                         printer.name,
-                        disable_weight_sync=disable_weight_sync,
+                        # Per-print tracking owns weight updates (#1119); manual
+                        # sync-all only refreshes spool metadata + slot assignments.
+                        disable_weight_sync=True,
                         cached_spools=cached_spools,
                         inventory_remaining=inv_remaining,
                         spoolman_spool_id_hint=hint,

+ 8 - 4
backend/app/main.py

@@ -1342,9 +1342,10 @@ async def on_ams_change(printer_id: int, ams_data: list):
             if sync_mode and sync_mode != "auto":
                 return  # Only sync on auto mode
 
-            # Check if weight sync is disabled
-            disable_weight_sync_str = await get_setting(db, "spoolman_disable_weight_sync")
-            disable_weight_sync = disable_weight_sync_str and disable_weight_sync_str.lower() == "true"
+            # `spoolman_disable_weight_sync` is deprecated (#1119) — weight is now
+            # always owned by per-print tracking, never by AMS auto-sync. The
+            # setting is still read by the settings UI for backwards compat but
+            # has no effect on the sync path here.
 
             # Get Spoolman URL
             spoolman_url = await get_setting(db, "spoolman_url")
@@ -1450,7 +1451,10 @@ async def on_ams_change(printer_id: int, ams_data: list):
                         result = await client.sync_ams_tray(
                             tray,
                             printer_name,
-                            disable_weight_sync=disable_weight_sync,
+                            # Per-print tracking is the only weight writer (#1119).
+                            # AMS auto-sync still maintains spool metadata / slot
+                            # assignments but no longer touches remaining_weight.
+                            disable_weight_sync=True,
                             cached_spools=cached_spools,
                             inventory_remaining=inv_remaining,
                             spoolman_spool_id_hint=hint,

+ 4 - 9
backend/app/services/spoolman_tracking.py

@@ -166,8 +166,10 @@ async def store_print_data(
 ):
     """Store Spoolman tracking data at print start (persisted to database).
 
-    Only stores data when Spoolman is enabled and AMS weight sync is disabled
-    (i.e., we're using per-usage tracking instead of AMS percentage estimates).
+    Per-print tracking is the primary weight-update path for Spoolman, mirroring
+    how the internal Filament Inventory works. The legacy AMS-remain%-based sync
+    is no longer used as a weight writer (#1119), so this runs whenever Spoolman
+    is enabled regardless of the deprecated `spoolman_disable_weight_sync` flag.
     """
     from backend.app.api.routes.settings import get_setting
     from backend.app.models.active_print_spoolman import ActivePrintSpoolman
@@ -183,13 +185,6 @@ async def store_print_data(
     if not spoolman_enabled or spoolman_enabled.lower() != "true":
         return
 
-    # Only store tracking data if "Disable AMS Weight Sync" is enabled
-    disable_weight_sync_str = await get_setting(db, "spoolman_disable_weight_sync")
-    disable_weight_sync = disable_weight_sync_str and disable_weight_sync_str.lower() == "true"
-    if not disable_weight_sync:
-        logger.debug("[SPOOLMAN] Weight sync enabled, skipping per-usage tracking data storage")
-        return
-
     # Get 3MF file path
     full_path = app_settings.base_dir / file_path
     if not full_path.exists():

+ 50 - 0
backend/tests/unit/services/test_spoolman_tracking.py

@@ -221,3 +221,53 @@ class TestStorePrintData:
         tracking = db.add.call_args.args[0]
         assert tracking.slot_to_tray == [1, -1, -1, -1]
         db.execute.assert_called_once()
+
+    @pytest.mark.asyncio
+    async def test_stores_tracking_when_disable_weight_sync_is_false(self):
+        """#1119: per-print tracking must run regardless of disable_weight_sync.
+
+        Previously store_print_data short-circuited when the deprecated
+        `spoolman_disable_weight_sync` flag was off, leaving non-BL spools
+        with no weight-update path at all. Per-print tracking is now the
+        only weight writer for Spoolman, so it must run whenever Spoolman
+        is enabled.
+        """
+        db = AsyncMock()
+        db.execute = AsyncMock(return_value=MagicMock())
+        db.add = MagicMock()
+        db.commit = AsyncMock()
+
+        printer_manager = MagicMock()
+        printer_manager.get_status.return_value = SimpleNamespace(
+            raw_data={"ams": [{"id": 0, "tray": [{"id": 0, "tray_type": "PLA"}]}]}
+        )
+
+        mock_settings = MagicMock()
+        mock_path = MagicMock()
+        mock_path.exists.return_value = True
+        mock_settings.base_dir.__truediv__.return_value = mock_path
+
+        # Only spoolman_enabled is consulted now (disable_weight_sync is no
+        # longer read). The single side_effect entry proves no extra
+        # get_setting calls slip back in.
+        with (
+            patch("backend.app.services.spoolman_tracking.app_settings", mock_settings),
+            patch("backend.app.api.routes.settings.get_setting", AsyncMock(side_effect=["true"])),
+            patch(
+                "backend.app.utils.threemf_tools.extract_filament_usage_from_3mf",
+                return_value=[{"slot_id": 1, "used_g": 5.0, "type": "PLA", "color": "#FF0000"}],
+            ),
+            patch("backend.app.utils.threemf_tools.extract_layer_filament_usage_from_3mf", return_value=None),
+            patch("backend.app.utils.threemf_tools.extract_filament_properties_from_3mf", return_value={}),
+        ):
+            await store_print_data(
+                printer_id=1,
+                archive_id=20,
+                file_path="archives/test.3mf",
+                db=db,
+                printer_manager=printer_manager,
+                ams_mapping=[0],
+            )
+
+        # Tracking row was inserted — the fix is working.
+        db.add.assert_called_once()

Certains fichiers n'ont pas été affichés car il y a eu trop de fichiers modifiés dans ce diff