소스 검색

fix(ams): stop reading the printer's command acks as status (issue #3040)

Every project_file carried "cfg": "0" — the device-config bitmask, which
Bambu Studio has never sent and the firmware ignores. The printer echoes a
command's fields back in its ack, and the ack was ingested as telemetry, so
bit 18 read as "AMS Filament Backup off" 25 ms after every dispatch.

Families that repeat cfg in their periodic status (P2S, H2C, X2D) corrected
themselves a second later; the P1S, A1, A1 Mini and A2L send it only in a
full status dump, so the wrong value stuck and silently disabled the
prefer-lowest-remaining gate. The A1 family, which reports no cfg at all and
is meant to stay "unknown", was pinned to a definite "off".

Acks are no longer read as status, for the backup bit or the per-job
timelapse flag they also echo, and cfg is gone from the print command.
maziggy 3 일 전
부모
커밋
0b830ac35c
3개의 변경된 파일112개의 추가작업 그리고 5개의 파일을 삭제
  1. 1 0
      CHANGELOG.md
  2. 32 4
      backend/app/services/bambu_mqtt.py
  3. 79 1
      backend/tests/unit/services/test_bambu_mqtt.py

+ 1 - 0
CHANGELOG.md

@@ -29,6 +29,7 @@ All notable changes to Bambuddy will be documented in this file.
 - **Every FTP session Bambuddy opens now records how it closed (#3009, reported by @grengojbo)** — the report traced a print completion that opened two FTP connections to the printer, deleted one file and then, as far as the log showed, did nothing else until the printer was powered off 21 minutes later, and concluded the connections were being left open. They were not: the post-print SD-card cleanup opens one connection per candidate filename and closes each in a `finally`, which a run against a real FTPS server confirms at the server end for both the delete and the 550 not-here case. The trouble is that nothing in the log could have said so. Neither the clean close nor the hard socket drop logged anything at any level, so a session closed properly and a socket genuinely abandoned produced the same output — none — and the only way to tell them apart was to read the source. Both now log one DEBUG line naming the printer, whether QUIT was acknowledged or the socket had to be dropped without it, why, and how long the session was held. Every connect in a debug log is now paired with a close, so the next person suspecting a leaked FTP connection can settle it from a support bundle rather than by inference. Nothing about the connection handling itself changed, and at default log level nothing new is printed. This does not explain the SD-card read/write error in that report or in #645; it only removes one theory from the list by making it checkable.
 
 ### Fixed
+- **AMS Filament Backup switched itself off with every print started from the queue (#3040, reported by @frnzzle)** — it never actually did: the printer had auto-refill on the whole time, and Bambuddy was reading its own request back as telemetry. Every `project_file` Bambuddy sent carried `"cfg": "0"`, a field Bambu Studio has never sent and the firmware ignores, but the printer echoes a command's fields back in its acknowledgement — and `cfg` is the device-config bitmask whose bit 18 is auto-refill. The acknowledgement was ingested as though it were a status frame, so 25 ms after every dispatch the badge flipped to off. On a P2S, H2C or X2D it flickered back a second later, because those repeat `cfg` in their periodic status; the P1S, A1, A1 Mini and A2L send it only in a full status dump, which arrives on connect and on Force Refresh and otherwise not at all, so there the wrong value stood until someone toggled it by hand. That mattered beyond the badge: the "prefer lowest remaining" sort is deliberately skipped while the printer reports backup off, since without a second spool to fall back on, feeding a print from the emptiest one risks running dry mid-job — which is why near-empty spools were being left untouched, the symptom that opened the report. On the A1 family it was worse still: they report no `cfg` at all, and the state is meant to stay "unknown" so that behaviour is left exactly as it was before the feature existed; the echo turned that into a definite "off" on the first queue print. Command acknowledgements are no longer read as status — for the backup bit or for the per-job timelapse flag they also echo — and the `cfg` field is gone from the print command, which is not a place to be writing printer settings from.
 - **K values missing on a second AMS, and its slots unconfigurable (#3044, reported by @Zib-Astian)** — an X2D with two AMS 2 Pro, one per hotend, showed K on every slot of the first and nothing on any slot of the second. Configure Slot was worse than blank there: the picker offered no matching profile, the slot read as though it had none bound, and choosing one changed nothing visible. Both came from the same rule. A calibration index can mean two different profiles on a dual-nozzle printer — on the maintainer's H2C, index 16 is the left hotend's black PLA at K=0.018 and 15 is the right's at K=0.020 — so Bambuddy resolves it against the slot's own hotend and shows nothing rather than the wrong number. But a printer does not always file one profile per hotend. This one files one per filament, so the second AMS's slots point at the same entries as the first, all tagged with a single extruder, and requiring a match found nothing at all. The hotend now has to actually appear in the table the printer sent before it is used to narrow anything; where it does not, the index stands on its own, which is what BambuStudio does for the same card. Printers that do file per hotend are unaffected — the wrong-nozzle number still cannot be shown, and the other hotend's profiles stay under **Other K profiles**. Also fixed alongside it: on a dual-nozzle printer the K-profile dropdown named the hotend twice on every matching option and three times on every other one.
 - **The Timeline ignored Shortest Job First (#3043)** — turning SJF on reordered the pending list and the scheduler itself, but the Timeline tab kept drawing the queue in its pre-SJF order, indefinitely. It chained each swimlane's bars by queue position alone and was never told the setting existed, so the one view whose entire job is to say *when* each print will run was the one view answering for an order the scheduler had no intention of using. Bars now chain in the scheduler's own order — jumped items first, then shortest print time, then position — and the starvation guard is visible there too, so a long print that has finally come up shows as such instead of staying buried. All three surfaces that claim to show queue order (the pending list, the "if started now" ETA, and the Timeline) now share one comparator rather than three hand-copied ones. Along the way the pending list stopped folding a model name down to its first letter when grouping lanes, which had been interleaving *Any X1C* with *Any X2D* (and *Any P1S* with *Any P1P*) into a single run of rows.
 - **Preheat & Heat Soak delayed PLA prints by minutes with nothing to preheat for (#3041)** — with the feature on, a queued PLA print sat for five to seven minutes before the file was even uploaded, and turning the feature off started it immediately. The per-filament map does what it says and gives PLA a chamber target of 0, and the chamber phase duly skipped — but the stage then heated the bed to the print's own temperature, waited for it, and held the full soak anyway, because the soak had no idea it was holding for a chamber that was never asked for. That is pure delay: the print's G-code sets the bed the moment it starts, so preheating it here only moved the warm-up ahead of the upload instead of overlapping with it, and there was no chamber to condition. A chamber target of 0 derived from the filament map now skips the whole stage, so PLA, PETG, TPU and PVA prints dispatch straight away with the feature left on. The one thing the skip still does is put the airduct flap back to cooling, which is a single command and no wait — an H2D left in heating mode by the ABS job before it would otherwise cook the PLA that follows. Explicit instructions are untouched: a chamber target of **0** typed into a print's own override still heats the bed and runs the soak, which is what the queue documentation has always promised it does, as does forcing a print's Preheat override to **On**. Prints that genuinely want chamber heat are unaffected, including on the P1S/P1P/A1 tier where the bed and the soak timer are the whole mechanism.

+ 32 - 4
backend/app/services/bambu_mqtt.py

@@ -84,6 +84,20 @@ def parse_ams_filament_backup_from_cfg(cfg_raw: object) -> bool | None:
         return None
 
 
+def is_printer_status_frame(print_data: dict) -> bool:
+    """True when a ``print`` payload is the printer reporting its own state.
+
+    Bambu firmware echoes a command's fields back in its acknowledgement, so a
+    `project_file` ack carries whatever Bambuddy put on the wire — including
+    the `cfg` bitmask and the per-job `timelapse` flag. Ingesting those as
+    telemetry means reading our own request back as the printer's state
+    (#3040). Only `push_status` (and the odd firmware that omits `command`
+    entirely on a status frame) describes the printer.
+    """
+    command = print_data.get("command")
+    return command is None or command == "push_status"
+
+
 # ── A2L "AMS Lite" unit-id normalisation (issue capture 2026-07-20) ──────────
 # The A2L reports its 4-slot AMS Lite as physical unit **id 16**, but the
 # firmware is internally inconsistent about it:
@@ -2200,7 +2214,15 @@ class BambuMQTTClient:
             # next 1-2 push_status frames may still carry the printer's OLD cfg
             # for ~3 s before the firmware reflects the change. Without this
             # gate the UI would flicker ON→OFF→ON. Same pattern xcam uses.
-            new_backup = parse_ams_filament_backup_from_cfg(print_data.get("cfg"))
+            # Only from a status frame: a project_file ack echoes our own
+            # `"cfg": "0"` back, which read as "printer says backup is OFF" and
+            # stuck on every family that doesn't repeat `cfg` in its periodic
+            # frames — P1S, A1, A1 Mini, A2L (#3040).
+            new_backup = (
+                parse_ams_filament_backup_from_cfg(print_data.get("cfg"))
+                if is_printer_status_frame(print_data)
+                else None
+            )
             if new_backup is not None and new_backup != self.state.ams_filament_backup:
                 hold_start = self._xcam_hold_start.get("print_option_auto_switch_filament")
                 if hold_start is not None and (time.time() - hold_start) <= self._xcam_hold_time:
@@ -4947,8 +4969,10 @@ class BambuMQTTClient:
             except (ValueError, TypeError):
                 logger.debug("[%s] could not parse stat field: %r", self.serial_number, data["stat"])
 
-        # Parse timelapse status (recording active during print)
-        if "timelapse" in data:
+        # Parse timelapse status (recording active during print). Status frames
+        # only — the project_file ack echoes back the per-job timelapse flag we
+        # asked for, which is a request, not the recorder's state (#3040).
+        if "timelapse" in data and is_printer_status_frame(data):
             logger.debug("[%s] timelapse field: %s", self.serial_number, data["timelapse"])
             self.state.timelapse = data["timelapse"] is True
             # Track if timelapse was ever active during this print
@@ -6001,7 +6025,11 @@ class BambuMQTTClient:
                     "vibration_cali": vibration_cali,
                     "layer_inspect": layer_inspect,
                     "use_ams": use_ams,
-                    "cfg": "0",
+                    # No "cfg": it is the printer's device-config bitmask
+                    # (auto-refill, detect-on-insert, chamber light, ...), not a
+                    # per-job field — BambuStudio's PrintParams has no such
+                    # member. We used to send "0"; firmware ignores it, but it
+                    # comes straight back in the project_file ack (#3040).
                     # extrude_cali_flag gates flow-dynamics calibration:
                     # 0 = never, 1 = force every print, 2 = auto (run only if the
                     # filament wasn't calibrated recently). #1721 saw stage 8

+ 79 - 1
backend/tests/unit/services/test_bambu_mqtt.py

@@ -4699,8 +4699,10 @@ class TestStartPrintUniqueIdentityFields:
         assert cmd["url"] == "ftp://test.3mf"
         assert cmd["file"] == "test.3mf"
         assert cmd["profile_id"] == "0"
-        assert cmd["cfg"] == "0"
         assert cmd["subtask_name"] == "test"
+        # The device-config bitmask is not a per-job field and is no longer
+        # sent; the printer echoed it back and we read it as telemetry (#3040).
+        assert "cfg" not in cmd
 
 
 class TestDeleteKProfileDualNozzleDetection:
@@ -6951,6 +6953,82 @@ class TestAmsFilamentBackupHoldTimer:
         assert mqtt_client._xcam_hold_start["print_option_auto_switch_filament"] == before_hold
 
 
+class TestCommandAckIsNotTelemetry:
+    """Regression (#3040): a printer's command acknowledgement echoes the
+    fields Bambuddy sent, so ingesting one as status reads our own request
+    back as the printer's state.
+
+    Bambuddy used to put ``"cfg": "0"`` in every project_file. The ack came
+    back carrying it, bit 18 read as "AMS Filament Backup OFF", and on the
+    families that don't repeat ``cfg`` in their periodic frames (P1S, A1,
+    A1 Mini, A2L) the wrong value stuck until the user toggled it — which
+    silently disabled the prefer-lowest-remaining gate for the rest of the day.
+    """
+
+    @pytest.fixture
+    def mqtt_client(self):
+        from unittest.mock import MagicMock
+
+        from backend.app.services.bambu_mqtt import BambuMQTTClient
+
+        client = BambuMQTTClient(
+            ip_address="192.168.1.100",
+            serial_number="TEST123",
+            access_code="12345678",
+        )
+        client.state.connected = True
+        client._client = MagicMock()
+        return client
+
+    def test_project_file_ack_does_not_clear_backup_state(self, mqtt_client):
+        mqtt_client.state.ams_filament_backup = True
+
+        mqtt_client._process_message(
+            {"print": {"command": "project_file", "sequence_id": "20000", "cfg": "0", "result": "success"}}
+        )
+
+        assert mqtt_client.state.ams_filament_backup is True
+
+    def test_project_file_ack_leaves_unknown_backup_unknown(self, mqtt_client):
+        """A1 / A1 Mini never report cfg, so the state must stay None ("unknown")
+        — the value the prefer-lowest gate reads as "preserve old behaviour"."""
+        assert mqtt_client.state.ams_filament_backup is None
+
+        mqtt_client._process_message({"print": {"command": "project_file", "cfg": "0"}})
+
+        assert mqtt_client.state.ams_filament_backup is None
+
+    def test_push_status_still_updates_backup_state(self, mqtt_client):
+        mqtt_client.state.ams_filament_backup = True
+
+        mqtt_client._process_message({"print": {"command": "push_status", "cfg": "C0340BC219"}})  # bit18=0
+
+        assert mqtt_client.state.ams_filament_backup is False
+
+    def test_status_frame_without_command_still_updates_backup_state(self, mqtt_client):
+        """Some firmwares omit `command` on a status frame; those stay trusted."""
+        mqtt_client.state.ams_filament_backup = False
+
+        mqtt_client._process_message({"print": {"cfg": "C0340FC219"}})  # bit18=1
+
+        assert mqtt_client.state.ams_filament_backup is True
+
+    def test_project_file_ack_does_not_clear_timelapse_state(self, mqtt_client):
+        """The ack echoes the per-job timelapse request, not the recorder."""
+        mqtt_client.state.timelapse = True
+
+        mqtt_client._process_message({"print": {"command": "project_file", "timelapse": False}})
+
+        assert mqtt_client.state.timelapse is True
+
+    def test_push_status_still_updates_timelapse_state(self, mqtt_client):
+        mqtt_client.state.timelapse = True
+
+        mqtt_client._process_message({"print": {"command": "push_status", "timelapse": False}})
+
+        assert mqtt_client.state.timelapse is False
+
+
 # ---------------------------------------------------------------------------
 # 2c. Single-nozzle H2S — external-spool tray_now override (#1822)
 # ---------------------------------------------------------------------------