Преглед изворни кода

Fix queue print command not reaching printer on reconnect (#778)

  When targeting a specific printer, the scheduler's power-on-wait loop
  created new MQTT clients on each attempt. Each new client re-tried the
  request topic subscription, which some brokers (e.g. A1) reject by
  disconnecting. This caused a 170s thrash loop leaving the connection
  fragile, so the eventual print command silently failed to reach the
  printer.

  Cache request topic support per serial number at the class level so
  new client instances inherit the knowledge and skip the subscription.
maziggy пре 2 месеци
родитељ
комит
750accb17d
3 измењених фајлова са 92 додато и 2 уклоњено
  1. 1 0
      CHANGELOG.md
  2. 11 2
      backend/app/services/bambu_mqtt.py
  3. 80 0
      backend/tests/unit/services/test_bambu_mqtt.py

+ 1 - 0
CHANGELOG.md

@@ -10,6 +10,7 @@ All notable changes to Bambuddy will be documented in this file.
 - **Per-User Email Notifications** ([#693](https://github.com/maziggy/bambuddy/pull/693)) — When Advanced Authentication is enabled, individual users can now receive email notifications for their own print jobs. A new "Notifications" page lets each user toggle notifications for print start, complete, failed, and stopped events. Only prints submitted by that user trigger their email — other users' prints are not affected. Requires SMTP to be configured and the "User Notifications" toggle enabled in Settings → Notifications. Administrators and Operators have access by default; Viewers do not. Contributed by @cadtoolbox.
 
 ### Fixed
+- **Queue Print Command Not Reaching Printer** ([#778](https://github.com/maziggy/bambuddy/issues/778)) — When a queue item targeted a specific printer and the scheduler's power-on-wait loop triggered, each reconnection attempt created a new MQTT client that re-attempted subscribing to the request topic. On printers whose broker rejects this subscription (e.g. A1), this caused repeated connect/disconnect cycles for up to 170 seconds, leaving the MQTT connection in a fragile state where the print command could silently fail to reach the printer. Fixed by caching request topic support state per serial number at the class level, so new client instances skip the subscription immediately instead of rediscovering the rejection. Reported by @RubenKremer.
 - **AMS Slot Search Shows Unrelated Profiles** ([#681](https://github.com/maziggy/bambuddy/issues/681)) — Searching for a non-existent filament profile in the AMS slot configuration showed unrelated profiles instead of an empty result. The saved preset bypassed the search filter entirely, so stale mappings (e.g. a slot previously configured with "Bambu PLA Matte" that now holds a Silk spool) would always appear regardless of the search query. The saved preset now only bypasses the printer model filter, not the search filter. Reported by @RosdasHH.
 - **Virtual Printer FTP Routed to Wrong VP** ([#735](https://github.com/maziggy/bambuddy/issues/735)) — When running multiple virtual printers with different access codes on separate bind IPs, FTP connections were routed to the wrong VP. Root cause: the iptables `REDIRECT` rule rewrites the destination IP to the incoming interface's primary address, so all FTP traffic went to the first VP regardless of the intended target. Fix: FTP server now binds directly to port 990 (standard implicit FTPS), eliminating the need for iptables redirect. Requires `CAP_NET_BIND_SERVICE` (already set in the systemd service and Docker image). Also removed a global `set_exception_handler()` in the MQTT server that caused spurious error messages when running multiple VPs. See `docs/migration-vp-ftp-port.md` for migration steps. Reported by @VREmma.
 - **X1C Virtual Printer Not Accepting Sends** ([#735](https://github.com/maziggy/bambuddy/issues/735)) — X1C (and X1) virtual printers were advertised with legacy SSDP model codes (`3DPrinter-X1-Carbon` / `3DPrinter-X1`) that BambuStudio doesn't recognize, causing "incompatible printer preset" when sending. Fixed to use the correct codes (`BL-P001` / `BL-P002`). Also fixed proxy mode auto-inherit storing the printer's display name (e.g. `X1C`) instead of the SSDP code. Existing VPs are automatically migrated on startup. Reported by @RosdasHH.

+ 11 - 2
backend/app/services/bambu_mqtt.py

@@ -265,6 +265,10 @@ class BambuMQTTClient:
 
     MQTT_PORT = 8883
 
+    # Class-level cache: serial_number -> False when request topic is known unsupported.
+    # Persists across client instances so reconnects don't re-trigger failed subscriptions.
+    _request_topic_cache: dict[str, bool] = {}
+
     def __init__(
         self,
         ip_address: str,
@@ -332,9 +336,10 @@ class BambuMQTTClient:
         self._captured_ams_mapping: list[int] | None = None
 
         # Request topic subscription tracking
-        # Some printer MQTT brokers (e.g. P1S) reject subscriptions to the request
+        # Some printer MQTT brokers (e.g. P1S, A1) reject subscriptions to the request
         # topic by killing the TCP connection. We detect this and gracefully degrade.
-        self._request_topic_supported: bool = True
+        # Check class-level cache first so new client instances don't retry known-bad subscriptions.
+        self._request_topic_supported: bool = BambuMQTTClient._request_topic_cache.get(self.serial_number, True)
         self._request_topic_sub_mid: int | None = None
         self._request_topic_sub_time: float = 0.0
         self._request_topic_confirmed: bool = False
@@ -387,6 +392,7 @@ class BambuMQTTClient:
                         self.serial_number,
                     )
                     self._request_topic_supported = False
+                    BambuMQTTClient._request_topic_cache[self.serial_number] = False
             # Request full status update (includes nozzle info in push_status response)
             self._request_push_all()
             # Request firmware version info
@@ -414,6 +420,7 @@ class BambuMQTTClient:
                         rc.getName(),
                     )
                     self._request_topic_supported = False
+                    BambuMQTTClient._request_topic_cache[self.serial_number] = False
                 else:
                     logger.info(
                         "[%s] Request topic subscription accepted. "
@@ -421,6 +428,7 @@ class BambuMQTTClient:
                         self.serial_number,
                     )
                     self._request_topic_confirmed = True
+                    BambuMQTTClient._request_topic_cache[self.serial_number] = True
             self._request_topic_sub_mid = None
             self._request_topic_sub_time = 0.0
 
@@ -449,6 +457,7 @@ class BambuMQTTClient:
                 self.serial_number,
             )
             self._request_topic_supported = False
+            BambuMQTTClient._request_topic_cache[self.serial_number] = False
         self._request_topic_sub_mid = None
         self._request_topic_sub_time = 0.0
 

+ 80 - 0
backend/tests/unit/services/test_bambu_mqtt.py

@@ -992,6 +992,13 @@ class TestNozzleRackData:
 class TestRequestTopicFailSafe:
     """Tests for graceful degradation when broker rejects request topic subscription."""
 
+    @pytest.fixture(autouse=True)
+    def clear_request_topic_cache(self):
+        """Clear class-level cache before each test to avoid cross-test pollution."""
+        from backend.app.services.bambu_mqtt import BambuMQTTClient
+
+        BambuMQTTClient._request_topic_cache.clear()
+
     @pytest.fixture
     def mqtt_client(self):
         from backend.app.services.bambu_mqtt import BambuMQTTClient
@@ -1099,6 +1106,79 @@ class TestRequestTopicFailSafe:
         assert len(subscribe_calls) == 1
         assert subscribe_calls[0] == mqtt_client.topic_subscribe
 
+    def test_cache_persists_across_instances(self):
+        """New client instance inherits request topic unsupported state from cache."""
+        from backend.app.services.bambu_mqtt import BambuMQTTClient
+
+        client1 = BambuMQTTClient(
+            ip_address="192.168.1.100",
+            serial_number="TEST_CACHE",
+            access_code="12345678",
+        )
+        assert client1._request_topic_supported is True
+
+        # Simulate disconnect-after-subscribe disabling the topic
+        client1._request_topic_sub_time = __import__("time").time()
+        client1._request_topic_confirmed = False
+        client1._last_message_time = 0.0
+        client1._on_disconnect(None, None)
+        assert client1._request_topic_supported is False
+
+        # New instance for same serial should inherit the cached state
+        client2 = BambuMQTTClient(
+            ip_address="192.168.1.100",
+            serial_number="TEST_CACHE",
+            access_code="12345678",
+        )
+        assert client2._request_topic_supported is False
+
+    def test_cache_does_not_affect_different_serial(self):
+        """Cache is per-serial — different printer is unaffected."""
+        from backend.app.services.bambu_mqtt import BambuMQTTClient
+
+        BambuMQTTClient._request_topic_cache["SERIAL_A"] = False
+
+        client = BambuMQTTClient(
+            ip_address="192.168.1.100",
+            serial_number="SERIAL_B",
+            access_code="12345678",
+        )
+        assert client._request_topic_supported is True
+
+    def test_cache_updated_on_suback_success(self):
+        """Successful SUBACK caches positive confirmation."""
+        from paho.mqtt.reasoncodes import ReasonCode
+
+        from backend.app.services.bambu_mqtt import BambuMQTTClient
+
+        client = BambuMQTTClient(
+            ip_address="192.168.1.100",
+            serial_number="TEST_SUBACK",
+            access_code="12345678",
+        )
+        client._request_topic_sub_mid = 42
+        rc = ReasonCode(9, identifier=0)  # Success
+        client._on_subscribe(None, None, 42, [rc], None)
+
+        assert BambuMQTTClient._request_topic_cache["TEST_SUBACK"] is True
+
+    def test_cache_updated_on_suback_rejection(self):
+        """SUBACK rejection caches negative state."""
+        from paho.mqtt.reasoncodes import ReasonCode
+
+        from backend.app.services.bambu_mqtt import BambuMQTTClient
+
+        client = BambuMQTTClient(
+            ip_address="192.168.1.100",
+            serial_number="TEST_REJECT",
+            access_code="12345678",
+        )
+        client._request_topic_sub_mid = 42
+        rc = ReasonCode(9, identifier=0x80)  # Failure
+        client._on_subscribe(None, None, 42, [rc], None)
+
+        assert BambuMQTTClient._request_topic_cache["TEST_REJECT"] is False
+
 
 class TestRequestTopicAmsMapping:
     """Tests for capturing ams_mapping from the MQTT request topic."""