Просмотр исходного кода

Fix rapid MQTT disconnect/reconnect bouncing (#813)

  The stale connection fix force-closed the socket on every frontend
  status poll, causing a tight close→reconnect→stale→close loop. Added
  a 30-second cooldown between stale reconnect attempts so paho has time
  to re-establish the connection. Also added reconnect backoff (1-30s)
  and unique-per-process MQTT client IDs to prevent broker session
  takeovers.
maziggy 5 месяцев назад
Родитель
Сommit
161015a577
2 измененных файлов с 26 добавлено и 3 удалено
  1. 1 1
      CHANGELOG.md
  2. 25 2
      backend/app/services/bambu_mqtt.py

+ 1 - 1
CHANGELOG.md

@@ -57,7 +57,7 @@ All notable changes to Bambuddy will be documented in this file.
 - **SpoolBuddy Daemon Reports Stale Version** — The SpoolBuddy daemon maintained its own hardcoded `__version__` that was never bumped to `0.2.3b1`, causing the update check to incorrectly show an update from `0.2.2b1` to the latest release. Fixed by reading the version at import time from the backend's `APP_VERSION` in `backend/app/core/config.py` — the single source of truth — so the daemon version is always in sync.
 - **SpoolBuddy Daemon Reports Stale Version** — The SpoolBuddy daemon maintained its own hardcoded `__version__` that was never bumped to `0.2.3b1`, causing the update check to incorrectly show an update from `0.2.2b1` to the latest release. Fixed by reading the version at import time from the backend's `APP_VERSION` in `backend/app/core/config.py` — the single source of truth — so the daemon version is always in sync.
 - **SpoolBuddy Update Columns Missing from Database** — The OTA update feature added `update_status` and `update_message` to the device model but was missing the database migration, causing "no such column" errors on existing installations.
 - **SpoolBuddy Update Columns Missing from Database** — The OTA update feature added `update_status` and `update_message` to the device model but was missing the database migration, causing "no such column" errors on existing installations.
 - **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.
 - **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.
-- **Stale MQTT Connection Not Recovering** ([#813](https://github.com/maziggy/bambuddy/issues/813)) — When a printer's MQTT connection went stale (no messages for 60+ seconds), Bambuddy marked it as disconnected but did not force the underlying TCP socket closed, so paho-mqtt's auto-reconnect never triggered and print commands were silently published into a dead connection. Fixed by force-closing the socket on stale detection so paho's loop thread detects the break and auto-reconnects. The initial fix caused rapid connected/disconnected bouncing in the UI because the force-close triggered paho's disconnect callback which redundantly broadcast state changes; now uses a flag to suppress the redundant callback. Also relaxed MQTT keepalive from 15s to 30s — the aggressive 15s keepalive caused spurious disconnects on transient network hiccups. Error disconnects (`rc.is_failure`) are never suppressed by the spurious-disconnect filter. Additionally, the disconnect event used by `disconnect()` is now fired unconditionally at the top of the callback so that no early-return filter can prevent it from unblocking callers. Reported by @inkdawgz.
+- **Stale MQTT Connection Not Recovering** ([#813](https://github.com/maziggy/bambuddy/issues/813)) — When a printer's MQTT connection went stale (no messages for 60+ seconds), Bambuddy marked it as disconnected but did not force the underlying TCP socket closed, so paho-mqtt's auto-reconnect never triggered and print commands were silently published into a dead connection. Fixed by force-closing the socket on stale detection so paho's loop thread detects the break and auto-reconnects. The initial fix caused rapid connected/disconnected bouncing in the UI because frontend status polls triggered repeated socket force-closes before paho could finish reconnecting; added a 30-second cooldown between stale reconnect attempts so paho has time to re-establish the connection. Also uses a flag to suppress the redundant disconnect callback broadcast. Relaxed MQTT keepalive from 15s to 30s — the aggressive 15s keepalive caused spurious disconnects on transient network hiccups. Added reconnect backoff (1-30s) and unique-per-process MQTT client IDs to prevent broker session takeovers. Error disconnects (`rc.is_failure`) are never suppressed by the spurious-disconnect filter. The disconnect event used by `disconnect()` is fired unconditionally at the top of the callback so that no early-return filter can prevent it from unblocking callers. Reported by @inkdawgz.
 - **P1S/P1P Printer Card Shows "Printing" When Idle** ([#813](https://github.com/maziggy/bambuddy/issues/813)) — Some P1S and P1P firmware versions report `stg_cur=0` when idle, which maps to the "Printing" stage name and overrides the correct "Idle" gcode_state on the printer card. The System Info page was unaffected because it displays the raw gcode_state. Extended the existing A1/A1 Mini workaround for this firmware bug to also cover P1S and P1P models. Reported by @inkdawgz.
 - **P1S/P1P Printer Card Shows "Printing" When Idle** ([#813](https://github.com/maziggy/bambuddy/issues/813)) — Some P1S and P1P firmware versions report `stg_cur=0` when idle, which maps to the "Printing" stage name and overrides the correct "Idle" gcode_state on the printer card. The System Info page was unaffected because it displays the raw gcode_state. Extended the existing A1/A1 Mini workaround for this firmware bug to also cover P1S and P1P models. Reported by @inkdawgz.
 - **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.
 - **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.
 - **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.

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

@@ -10,6 +10,7 @@ but with qos=1 they respond instantly.
 import asyncio
 import asyncio
 import json
 import json
 import logging
 import logging
+import os
 import ssl
 import ssl
 import threading
 import threading
 import time
 import time
@@ -268,6 +269,8 @@ class BambuMQTTClient:
     # Class-level cache: serial_number -> False when request topic is known unsupported.
     # Class-level cache: serial_number -> False when request topic is known unsupported.
     # Persists across client instances so reconnects don't re-trigger failed subscriptions.
     # Persists across client instances so reconnects don't re-trigger failed subscriptions.
     _request_topic_cache: dict[str, bool] = {}
     _request_topic_cache: dict[str, bool] = {}
+    # Counter for generating unique MQTT client IDs across instances.
+    _client_instance_counter: int = 0
 
 
     def __init__(
     def __init__(
         self,
         self,
@@ -347,6 +350,9 @@ class BambuMQTTClient:
         # Set when check_staleness() force-closes the socket to trigger reconnect.
         # Set when check_staleness() force-closes the socket to trigger reconnect.
         # Prevents _on_disconnect from redundantly broadcasting state (already done).
         # Prevents _on_disconnect from redundantly broadcasting state (already done).
         self._stale_reconnecting: bool = False
         self._stale_reconnecting: bool = False
+        # Timestamp of last stale reconnect — prevents rapid-fire socket closes
+        # when the frontend polls status faster than paho can reconnect.
+        self._last_stale_reconnect: float = 0.0
 
 
     @property
     @property
     def topic_subscribe(self) -> str:
     def topic_subscribe(self) -> str:
@@ -366,12 +372,24 @@ class BambuMQTTClient:
         time_since_last = time.time() - self._last_message_time
         time_since_last = time.time() - self._last_message_time
         return time_since_last > self.STALE_TIMEOUT
         return time_since_last > self.STALE_TIMEOUT
 
 
+    # Minimum seconds between stale reconnect attempts.  Frontend polls
+    # status every few seconds — without a cooldown, each poll would
+    # force-close the socket before paho has time to reconnect.
+    STALE_RECONNECT_COOLDOWN = 30.0
+
     def check_staleness(self) -> bool:
     def check_staleness(self) -> bool:
         """Check staleness and update connected state if stale. Returns True if connected."""
         """Check staleness and update connected state if stale. Returns True if connected."""
         if self.state.connected and self.is_stale():
         if self.state.connected and self.is_stale():
+            # Don't force-close again if we already did recently — give paho
+            # time to reconnect and the printer time to send its first message.
+            now = time.time()
+            if now - self._last_stale_reconnect < self.STALE_RECONNECT_COOLDOWN:
+                return self.state.connected
+
             logger.warning(
             logger.warning(
-                f"[{self.serial_number}] Connection stale - no message for {time.time() - self._last_message_time:.1f}s, forcing reconnect"
+                f"[{self.serial_number}] Connection stale - no message for {now - self._last_message_time:.1f}s, forcing reconnect"
             )
             )
+            self._last_stale_reconnect = now
             self.state.connected = False
             self.state.connected = False
             if self.on_state_change:
             if self.on_state_change:
                 self.on_state_change(self.state)
                 self.on_state_change(self.state)
@@ -2629,9 +2647,11 @@ class BambuMQTTClient:
                   If not provided, will try to get the running loop.
                   If not provided, will try to get the running loop.
         """
         """
         self._loop = loop
         self._loop = loop
+        BambuMQTTClient._client_instance_counter += 1
+        client_id = f"bambuddy_{self.serial_number}_{os.getpid()}_{BambuMQTTClient._client_instance_counter}"
         self._client = mqtt.Client(
         self._client = mqtt.Client(
             callback_api_version=mqtt.CallbackAPIVersion.VERSION2,
             callback_api_version=mqtt.CallbackAPIVersion.VERSION2,
-            client_id=f"bambuddy_{self.serial_number}",
+            client_id=client_id,
             protocol=mqtt.MQTTv311,
             protocol=mqtt.MQTTv311,
         )
         )
 
 
@@ -2647,6 +2667,9 @@ class BambuMQTTClient:
         ssl_context.verify_mode = ssl.CERT_NONE
         ssl_context.verify_mode = ssl.CERT_NONE
         self._client.tls_set_context(ssl_context)
         self._client.tls_set_context(ssl_context)
 
 
+        # Backoff reconnects to avoid tight reconnect loops on unstable brokers.
+        self._client.reconnect_delay_set(min_delay=1, max_delay=30)
+
         # Keepalive: paho sends PINGREQs at this interval, broker considers
         # Keepalive: paho sends PINGREQs at this interval, broker considers
         # client dead at 1.5x.  30s is a good balance — fast enough to detect
         # client dead at 1.5x.  30s is a good balance — fast enough to detect
         # real network loss (45s), not so aggressive that transient hiccups
         # real network loss (45s), not so aggressive that transient hiccups