From 5bda16ffa18c25abf202ad22230c47ad75456904 Mon Sep 17 00:00:00 2001 From: Guilherme Costa Date: Mon, 28 Sep 2026 15:49:10 +0100 Subject: [PATCH 1/2] fix(connection): auto-reconnect until klipper is ready, drop retry button --- BlocksScreen/lib/moonrakerComm.py | 120 ++++++----------- BlocksScreen/lib/panels/mainWindow.py | 40 ++++-- .../widgets/MainWindow/connectionPage.py | 7 - tests/lib/test_moonraker_comm_unit.py | 124 +++++++++++++++--- tests/panels/test_main_window_unit.py | 40 +++++- tests/widgets/test_connection_page.py | 7 + 6 files changed, 211 insertions(+), 127 deletions(-) diff --git a/BlocksScreen/lib/moonrakerComm.py b/BlocksScreen/lib/moonrakerComm.py index 5353e43b9..c59603629 100644 --- a/BlocksScreen/lib/moonrakerComm.py +++ b/BlocksScreen/lib/moonrakerComm.py @@ -7,7 +7,6 @@ import websocket from events import ( WebSocketDisconnected, - WebSocketError, WebSocketMessageReceived, WebSocketOpen, ) @@ -31,8 +30,10 @@ class MoonWebSocket(QtCore.QObject, threading.Thread): """MoonWebSocket class object for creating a websocket connection to Moonraker.""" QUERY_KLIPPY_TIMEOUT: int = 2 - max_retries = 3 timeout = 3 + # A silently dead socket never reaches on_close without client pings + PING_INTERVAL: int = 20 + PING_TIMEOUT: int = 10 # Moonraker's own pong timeout (tornado 6.5 default) connecting_signal = QtCore.pyqtSignal([int], [str], name="websocket_connecting") connected_signal = QtCore.pyqtSignal(name="websocket-connected") @@ -55,13 +56,14 @@ def __init__(self, parent: QtCore.QObject) -> None: self.daemon = True self.connected = False - self.connecting = False self.disconnected = False self._reconnect_count = 0 + self._klippy_state: str | None = None self.callback_table: dict = {} self._state_lock = threading.RLock() self._request_lock = threading.Lock() + self._connect_lock = threading.Lock() self._host = parent.config.get("host", parser=str, default="localhost") self._port = parent.config.get("port", parser=int, default=7125) @@ -71,12 +73,10 @@ def __init__(self, parent: QtCore.QObject) -> None: self._wst = None self._request_id = 0 self.request_table: dict = {} - self._klippy_retry_count = 0 self._moonRest = MoonRest(host=self._host, port=self._port) self.api: MoonAPI = MoonAPI(self) self._retry_timer: RepeatedTimer | None = None websocket.setdefaulttimeout(self.timeout) - self._intentional_disconnect: bool = False self.query_server_info_signal.connect(self.api.api_query_server_info) self.query_klippy_status_timer = RepeatedTimer( @@ -94,55 +94,31 @@ def moonRest(self) -> MoonRest: @QtCore.pyqtSlot(name="retry_wb_conn") def retry_wb_conn(self): """Retry websocket connection""" - with self._state_lock: - if self.connecting is True and self.connected is False: - return False - self.try_connection() + return self.try_connection() @QtCore.pyqtSlot(name="try_connection") def try_connection(self): - """Try connecting to websocket""" - with self._state_lock: - self.connecting = True - self._reconnect_count = 0 - if self._retry_timer is not None: - self._retry_timer.stopTimer() - self._retry_timer = RepeatedTimer(self.timeout, self.reconnect) - return self.connect() + """Arm the reconnect watchdog and attempt a connection now""" + if self._retry_timer is None: + self._retry_timer = RepeatedTimer(self.timeout, self.reconnect) + else: + self._retry_timer.startTimer() + return self.reconnect() def reconnect(self) -> bool: - """Reconnect to websocket""" - with self._state_lock: - if self.connected: - return True - over_limit = self._reconnect_count >= self.max_retries - - if over_limit: - if self._retry_timer is not None: - self._retry_timer.stopTimer() - unable_to_connect_event = WebSocketError( - data="Unable to establish connection to Websocket" - ) - self.connecting_signal[int].emit(0) - with self._state_lock: - self.connecting = False - try: - instance = QtWidgets.QApplication.instance() - if instance is not None: - instance.postEvent(self.parent(), unable_to_connect_event) - else: - raise TypeError("QApplication.instance expected ad non-None value") - except Exception as e: - logger.error( - "Error on sending Event %s | Error message: %s", - unable_to_connect_event.__class__.__name__, - e, - ) - logger.warning( - "Maximum number of connection retries reached, Unable to establish connection with Moonraker" - ) + """Watchdog tick, retries forever per Moonraker's client startup sequence""" + # Non-blocking: a tick and a Qt-thread call must not open two sockets + if not self._connect_lock.acquire(blocking=False): return False - return self.connect() + try: + with self._state_lock: + if self.connected: + return True + if self._wst is not None and self._wst.is_alive(): + return False # Previous attempt still in flight + return self.connect() + finally: + self._connect_lock.release() def connect(self) -> bool: """Connect to websocket""" @@ -178,6 +154,10 @@ def connect(self) -> bool: self._wst = threading.Thread( name="websocket.run_forever", target=self.ws.run_forever, + kwargs={ + "ping_interval": self.PING_INTERVAL, + "ping_timeout": self.PING_TIMEOUT, + }, daemon=True, ) try: @@ -191,8 +171,9 @@ def connect(self) -> bool: def wb_disconnect(self) -> None: """Websocket disconnect""" + if self._retry_timer is not None: + self._retry_timer.stopTimer() if self._wst is not None and self.ws is not None: - self._intentional_disconnect = True self.ws.close() if self._wst.is_alive(): self._wst.join(timeout=self.timeout + 1) @@ -241,11 +222,6 @@ def on_close(self, *args) -> None: logger.info( f"Websocket closed, code: {_close_status_code}, message: {_close_message}" ) - if not self.connecting and not self._intentional_disconnect: - QtCore.QMetaObject.invokeMethod( - self, "try_connection", QtCore.Qt.ConnectionType.QueuedConnection - ) - self._intentional_disconnect = False @QtCore.pyqtSlot(name="evaluate_klippy_status") def evaluate_klippy_status(self) -> None: @@ -261,10 +237,9 @@ def on_open(self, *args) -> None: """ _ws = args[0] if len(args) == 1 else None with self._state_lock: - self.connecting = False self.connected = True - self._klippy_retry_count = 0 self._reconnect_count = 0 + self._klippy_state = None # Re-emit the state after every reconnect self.evaluate_klippy_status() open_event = WebSocketOpen(data="Connected") try: @@ -277,8 +252,6 @@ def on_open(self, *args) -> None: logger.info("Unexpected error opening websocket: %s", e) self.connected_signal.emit() - if self._retry_timer is not None: - self._retry_timer.stopTimer() logger.info("Connection to websocket achieved on %s", _ws) def on_message(self, *args) -> None: @@ -314,23 +287,17 @@ def on_message(self, *args) -> None: return if _klippy_state == "ready": self.query_klippy_status_timer.stopTimer() - self._klippy_retry_count = 0 - self.api.update_status() # Request update status immediately after klippy ready DEVDEBT - elif _klippy_state in ("startup", "disconnected"): - self._klippy_retry_count += 1 - if self._klippy_retry_count >= 30: - self.query_klippy_status_timer.stopTimer() - logger.error( - "Klippy startup sequence timed out after %d retries (state=%s)", - self._klippy_retry_count, - _klippy_state, - ) - elif not self.query_klippy_status_timer.running: - self.query_klippy_status_timer.startTimer() + else: + self.query_klippy_status_timer.startTimer() self.klippy_connected_signal.emit( _result.get("klippy_connected", False) ) - self.klippy_state_signal.emit(_klippy_state) + if _klippy_state != self._klippy_state: + # Poll repeats the state; consumers act on transitions only + self._klippy_state = _klippy_state + if _klippy_state == "ready": + self.api.update_status() # Request update status immediately after klippy ready DEVDEBT + self.klippy_state_signal.emit(_klippy_state) return else: _callback = _entry[2] if len(_entry) > 2 else None @@ -356,14 +323,9 @@ def on_message(self, *args) -> None: ) elif "method" in response: if response["method"] in self._KLIPPY_NOTIFY_METHODS: - self._klippy_retry_count = 0 + if response["method"] == "notify_klippy_disconnected": + self._klippy_state = None # Restart may end in the same state self.evaluate_klippy_status() - elif ( - response["method"] == "notify_klippy_ready" - and not self.query_klippy_status_timer.running - ): - # Poll may have hit its retry cap; re-query so the ready path still runs - self.query_server_info_signal.emit() message_event = ( WebSocketMessageReceived( # mainly used to pass websocket notifications method=response["method"], diff --git a/BlocksScreen/lib/panels/mainWindow.py b/BlocksScreen/lib/panels/mainWindow.py index 40684e4c8..2f108641c 100644 --- a/BlocksScreen/lib/panels/mainWindow.py +++ b/BlocksScreen/lib/panels/mainWindow.py @@ -139,6 +139,11 @@ def __init__(self): self._klipper_restart_timeout.setSingleShot(True) self._klipper_restart_timeout.setInterval(30_000) self._klipper_restart_timeout.timeout.connect(self._on_klipper_restart_timeout) + # Outlasts a RESTART's socket gap and klipper.service RestartSec=10 + self._klipper_disconnect_grace = QtCore.QTimer(self) + self._klipper_disconnect_grace.setSingleShot(True) + self._klipper_disconnect_grace.setInterval(15_000) + self._klipper_disconnect_grace.timeout.connect(self._auto_restart_klipper) usb_config = self.config.get_section("usb_manager", fallback=None) gdir = None @@ -230,7 +235,6 @@ def __init__(self): 2, ) ) - self.conn_window.retry_connection_clicked.connect(slot=self.ws.retry_wb_conn) self.conn_window.firmware_restart_clicked.connect( slot=self.mc.restart_klipper_mcu_service ) @@ -299,7 +303,6 @@ def __init__(self): ) self.updater_worker.rollback_done.connect(self.update_page.handle_rollback_done) self.updater_worker.recover_done.connect(self.update_page.handle_recover_done) - self.ws.klippy_state_signal.connect(self._on_klippy_state) self.utilitiesPanel.show_update_page.connect(self.show_update_page) self.conn_window.update_button_clicked.connect(self.show_update_page) self.extruder_temp_display.display_format = "upper_downer" @@ -441,22 +444,14 @@ def show_update_page(self, fullscreen: bool): @QtCore.pyqtSlot(str, name="on-klippy-state") def _on_klippy_state(self, state: str) -> None: self._klippy_ready = state == "ready" + if state == "disconnected": + self._klipper_disconnect_grace.start() + else: + self._klipper_disconnect_grace.stop() if state == "shutdown": if self._update_in_progress: _logger.warning("Klipper E-stop detected — cancelling active update") self.updater_worker.trigger_cancel() - elif ( - state == "disconnected" - and not self._klipper_auto_restart_pending - and not self._update_in_progress - and not self.conn_window.manual_restart_pending - ): - _logger.info("Klipper disconnected — auto-restarting service") - self._klipper_auto_restart_pending = True - self.loadwidget.set_status_message("Restarting Klipper...") - self.loadscreen.show() - self._klipper_restart_timeout.start() - self.ws.api.restart_service("klipper") elif state == "ready" and self._klipper_auto_restart_pending: _logger.info("Klipper back online after auto-restart") self._klipper_auto_restart_pending = False @@ -464,6 +459,23 @@ def _on_klippy_state(self, state: str) -> None: if not self._post_update_reconnect: self.loadscreen.hide() + @QtCore.pyqtSlot(name="auto-restart-klipper") + def _auto_restart_klipper(self) -> None: + """Restart the klipper service once it stays disconnected past the grace.""" + if ( + self._klipper_auto_restart_pending + or self._update_in_progress + or self.conn_window.manual_restart_pending + or not self.ws.connected + ): + return + _logger.info("Klipper disconnected — auto-restarting service") + self._klipper_auto_restart_pending = True + self.loadwidget.set_status_message("Restarting Klipper...") + self.loadscreen.show() + self._klipper_restart_timeout.start() + self.ws.api.restart_service("klipper") + @QtCore.pyqtSlot(name="arm-health-bless") def _arm_health_bless(self) -> None: """On first moonraker connect, arm a one-shot to bless this build as healthy.""" diff --git a/BlocksScreen/lib/panels/widgets/MainWindow/connectionPage.py b/BlocksScreen/lib/panels/widgets/MainWindow/connectionPage.py index bc8aa8d21..a84d418ba 100644 --- a/BlocksScreen/lib/panels/widgets/MainWindow/connectionPage.py +++ b/BlocksScreen/lib/panels/widgets/MainWindow/connectionPage.py @@ -80,7 +80,6 @@ class ConnectionPage(QtWidgets.QFrame): _RESTART_LABELS: typing.ClassVar[dict[ConnectionState, str]] = { ConnectionState.KLIPPER_ERROR: "Firmware Restart", ConnectionState.KLIPPER_SHUTDOWN: "Firmware Restart", - ConnectionState.WEBSOCKET_LOST: "Retry Connection", } _KLIPPY_STATE_MAP: typing.ClassVar[dict[str, ConnectionState]] = { @@ -91,9 +90,6 @@ class ConnectionPage(QtWidgets.QFrame): "ready": ConnectionState.KLIPPER_READY, } - retry_connection_clicked: typing.ClassVar[QtCore.pyqtSignal] = QtCore.pyqtSignal( - name="retry_connection_clicked" - ) wifi_button_clicked: typing.ClassVar[QtCore.pyqtSignal] = QtCore.pyqtSignal( name="call_network_page" ) @@ -277,9 +273,6 @@ def _is_auto_recovering_context(raw: str) -> bool: ) def _on_restart_clicked(self) -> None: - if self._state == ConnectionState.WEBSOCKET_LOST: - self.retry_connection_clicked.emit() - return is_firmware = self._state in self._FIRMWARE_RESTART_STATES self._last_restart_was_firmware = is_firmware self._escalated_to_klipper_restart = False diff --git a/tests/lib/test_moonraker_comm_unit.py b/tests/lib/test_moonraker_comm_unit.py index cd615ca13..77e9608ce 100644 --- a/tests/lib/test_moonraker_comm_unit.py +++ b/tests/lib/test_moonraker_comm_unit.py @@ -5,9 +5,10 @@ from unittest.mock import MagicMock import pytest +from PyQt6 import QtCore + from BlocksScreen.lib import moonrakerComm from BlocksScreen.lib.moonrakerComm import OneShotTokenError -from PyQt6 import QtCore class TestOneShotTokenError: @@ -45,29 +46,21 @@ def _notify(sock, method: str) -> None: class TestKlippyNotifications: - def test_ready_requeries_after_poll_cap(self, ws): + def test_ready_does_not_requery(self, ws): + # Moonraker sets READY before notifying, so the poll already saw it ws.query_klippy_status_timer.running = False queries = [] ws.query_server_info_signal.connect(lambda: queries.append(1)) _notify(ws, "notify_klippy_ready") - assert queries == [1] - - def test_ready_skipped_while_polling(self, ws): - ws.query_klippy_status_timer.running = True - queries = [] - ws.query_server_info_signal.connect(lambda: queries.append(1)) - _notify(ws, "notify_klippy_ready") assert queries == [] @pytest.mark.parametrize( "method", ["notify_klippy_disconnected", "notify_klippy_shutdown"] ) def test_leaving_ready_restarts_poll_without_synthetic_state(self, ws, method): - ws._klippy_retry_count = 30 states = [] ws.klippy_state_signal.connect(states.append) _notify(ws, method) - assert ws._klippy_retry_count == 0 ws.query_klippy_status_timer.startTimer.assert_called_once() assert states == [] # server.info reply reports the real state @@ -89,22 +82,113 @@ def test_error_reply_emits_nothing(self, ws): self._reply(ws, {"error": {"code": 500, "message": "boom"}}) assert seen == [] + @pytest.mark.parametrize("state", ["startup", "disconnected", "shutdown", "error"]) + def test_poll_never_gives_up_before_ready(self, ws, state): + # Klippy may sit in startup for 90s waiting on the MCU (serialhdl.py) + for _ in range(100): + self._reply(ws, {"result": {"klippy_state": state}}) + ws.query_klippy_status_timer.stopTimer.assert_not_called() + self._reply(ws, {"result": {"klippy_state": "ready"}}) + ws.query_klippy_status_timer.stopTimer.assert_called_once() + + def test_state_emitted_on_change_only(self, ws, monkeypatch): + # A repeated shutdown would cancel the restart overlay mid-restart + update = MagicMock() + monkeypatch.setattr(ws.api, "update_status", update) + states = [] + ws.klippy_state_signal.connect(states.append) + for state in ["shutdown"] * 3 + ["startup"] * 3 + ["ready"] * 2: + self._reply(ws, {"result": {"klippy_state": state}}) + assert states == ["shutdown", "startup", "ready"] + update.assert_called_once() + + def test_klippy_restart_reemits_unchanged_state(self, ws): + # A failed FIRMWARE_RESTART goes shutdown to shutdown faster than the poll + states = [] + ws.klippy_state_signal.connect(states.append) + self._reply(ws, {"result": {"klippy_state": "shutdown"}}) + _notify(ws, "notify_klippy_disconnected") + self._reply(ws, {"result": {"klippy_state": "shutdown"}}) + assert states == ["shutdown", "shutdown"] + + def test_reconnect_reemits_unchanged_state(self, ws): + states = [] + ws.klippy_state_signal.connect(states.append) + self._reply(ws, {"result": {"klippy_state": "ready"}}) + ws.on_open(None) + self._reply(ws, {"result": {"klippy_state": "ready"}}) + assert states == ["ready", "ready"] + class TestReconnectCycle: - def test_gives_up_without_another_attempt(self, ws, monkeypatch): - connect = MagicMock() + def test_retries_forever_until_connected(self, ws, monkeypatch): + connect = MagicMock(return_value=False) monkeypatch.setattr(ws, "connect", connect) ws._retry_timer = MagicMock() - ws.connecting = True - ws._reconnect_count = ws.max_retries + ws._reconnect_count = 100 + for _ in range(10): + ws.reconnect() + assert connect.call_count == 10 + ws._retry_timer.stopTimer.assert_not_called() + + def test_reconnect_noop_once_connected(self, ws, monkeypatch): + connect = MagicMock() + monkeypatch.setattr(ws, "connect", connect) + ws.connected = True + assert ws.reconnect() is True + connect.assert_not_called() + + def test_reconnect_waits_for_attempt_in_flight(self, ws, monkeypatch): + connect = MagicMock() + monkeypatch.setattr(ws, "connect", connect) + ws._wst = MagicMock(is_alive=MagicMock(return_value=True)) assert ws.reconnect() is False connect.assert_not_called() - assert ws.connecting is False + + def test_reconnect_skips_while_another_attempt_starts(self, ws, monkeypatch): + connect = MagicMock() + monkeypatch.setattr(ws, "connect", connect) + with ws._connect_lock: + assert ws.reconnect() is False + connect.assert_not_called() + + def test_connect_pings_to_detect_dead_socket(self, ws, monkeypatch): + app = MagicMock() + monkeypatch.setattr(moonrakerComm.websocket, "WebSocketApp", app) + assert ws.connect() is True + ws._wst.join(1) + app.return_value.run_forever.assert_called_once_with( + ping_interval=ws.PING_INTERVAL, ping_timeout=ws.PING_TIMEOUT + ) + assert ws.PING_INTERVAL > ws.PING_TIMEOUT # else run_forever raises + + def test_disconnect_stops_retry_timer(self, ws): + ws._retry_timer = MagicMock() + ws.wb_disconnect() ws._retry_timer.stopTimer.assert_called_once() - def test_try_connection_starts_fresh_cycle(self, ws, monkeypatch): - monkeypatch.setattr(ws, "connect", MagicMock(return_value=True)) - ws._reconnect_count = ws.max_retries + 4 + def test_try_connection_arms_one_watchdog(self, ws, monkeypatch): + timers = MagicMock() + monkeypatch.setattr(moonrakerComm, "RepeatedTimer", timers) + connect = MagicMock(return_value=True) + monkeypatch.setattr(ws, "connect", connect) + ws.try_connection() ws.try_connection() + timers.assert_called_once_with(ws.timeout, ws.reconnect) + timers.return_value.startTimer.assert_called_once() + assert connect.call_count == 2 + + def test_open_resets_count_and_keeps_watchdog(self, ws): + ws._retry_timer = MagicMock() + ws._reconnect_count = 5 + ws.on_open(None) assert ws._reconnect_count == 0 - moonrakerComm.RepeatedTimer.assert_called_with(ws.timeout, ws.reconnect) + ws._retry_timer.stopTimer.assert_not_called() + + def test_close_leaves_reconnect_to_watchdog(self, ws, monkeypatch, qtbot): + connect = MagicMock() + monkeypatch.setattr(ws, "connect", connect) + ws.ws = MagicMock() + ws.on_close(None, 1006, "gone") + qtbot.wait(50) + connect.assert_not_called() diff --git a/tests/panels/test_main_window_unit.py b/tests/panels/test_main_window_unit.py index 260c8067e..4409f0b7a 100644 --- a/tests/panels/test_main_window_unit.py +++ b/tests/panels/test_main_window_unit.py @@ -8,35 +8,61 @@ from BlocksScreen.lib.panels.mainWindow import MainWindow -def _make_fake_window(*, manual_restart_pending: bool) -> SimpleNamespace: +def _make_fake_window( + *, manual_restart_pending: bool = False, ws_connected: bool = True +) -> SimpleNamespace: return SimpleNamespace( _klippy_ready=False, _update_in_progress=False, _klipper_auto_restart_pending=False, _post_update_reconnect=False, _klipper_restart_timeout=MagicMock(), + _klipper_disconnect_grace=MagicMock(), updater_worker=MagicMock(), conn_window=SimpleNamespace(manual_restart_pending=manual_restart_pending), loadwidget=MagicMock(), loadscreen=MagicMock(), - ws=MagicMock(), + ws=MagicMock(connected=ws_connected), ) class TestKlippyDisconnectedAutoRestartGuard: - def test_auto_restarts_when_no_manual_restart_pending(self) -> None: - fake_window = _make_fake_window(manual_restart_pending=False) + def test_disconnected_only_arms_grace(self) -> None: + # RESTART and SAVE_CONFIG pass through disconnected for about a second + fake_window = _make_fake_window() MainWindow._on_klippy_state(fake_window, "disconnected") + fake_window._klipper_disconnect_grace.start.assert_called_once() + fake_window.ws.api.restart_service.assert_not_called() + + @pytest.mark.parametrize("state", ["startup", "ready", "shutdown", "error"]) + def test_leaving_disconnected_cancels_grace(self, state) -> None: + fake_window = _make_fake_window() + + MainWindow._on_klippy_state(fake_window, state) + + fake_window._klipper_disconnect_grace.stop.assert_called_once() + fake_window._klipper_disconnect_grace.start.assert_not_called() + + def test_auto_restarts_once_grace_expires(self) -> None: + fake_window = _make_fake_window() + + MainWindow._auto_restart_klipper(fake_window) + fake_window.ws.api.restart_service.assert_called_once_with("klipper") fake_window.loadscreen.show.assert_called_once() assert fake_window._klipper_auto_restart_pending is True - def test_skips_auto_restart_when_manual_restart_pending(self) -> None: - fake_window = _make_fake_window(manual_restart_pending=True) + @pytest.mark.parametrize( + ("manual_restart_pending", "ws_connected"), [(True, True), (False, False)] + ) + def test_skips_auto_restart(self, manual_restart_pending, ws_connected) -> None: + fake_window = _make_fake_window( + manual_restart_pending=manual_restart_pending, ws_connected=ws_connected + ) - MainWindow._on_klippy_state(fake_window, "disconnected") + MainWindow._auto_restart_klipper(fake_window) fake_window.ws.api.restart_service.assert_not_called() fake_window.loadscreen.show.assert_not_called() diff --git a/tests/widgets/test_connection_page.py b/tests/widgets/test_connection_page.py index bcfe7932b..a6f8f542d 100644 --- a/tests/widgets/test_connection_page.py +++ b/tests/widgets/test_connection_page.py @@ -168,6 +168,13 @@ def test_normal_label_says_restart_klipper(self, page, qtbot): page._set_state(ConnectionState.KLIPPER_DISCONNECTED) assert page.restart_klipper_button.text() == "Restart Printer" + def test_websocket_lost_offers_restart_not_retry(self, page, qtbot): + # Reconnect is automatic, so the button only restarts Klipper + page._set_state(ConnectionState.WEBSOCKET_LOST) + assert page.restart_klipper_button.text() == "Restart Printer" + with qtbot.waitSignal(page.restart_klipper_clicked): + page._on_restart_clicked() + class TestSignalWiring: def test_reboot_button(self, page, qtbot): From ede6ec152aa64aa8f15d59aebe4361d9678f7c82 Mon Sep 17 00:00:00 2001 From: Guilherme Costa Date: Mon, 28 Sep 2026 16:39:05 +0100 Subject: [PATCH 2/2] fix(connection): show animated "Attempting to reconnect" and keep the dots on a steady beat --- .../widgets/MainWindow/connectionPage.py | 35 ++++++++++--------- tests/widgets/test_connection_page.py | 35 ++++++++++--------- 2 files changed, 37 insertions(+), 33 deletions(-) diff --git a/BlocksScreen/lib/panels/widgets/MainWindow/connectionPage.py b/BlocksScreen/lib/panels/widgets/MainWindow/connectionPage.py index a84d418ba..a758d72bc 100644 --- a/BlocksScreen/lib/panels/widgets/MainWindow/connectionPage.py +++ b/BlocksScreen/lib/panels/widgets/MainWindow/connectionPage.py @@ -31,7 +31,7 @@ class ConnectionPage(QtWidgets.QFrame): _MESSAGES: typing.ClassVar[dict[ConnectionState, str]] = { ConnectionState.DISCONNECTED: "The printer is offline.\nReconnecting automatically…", - ConnectionState.CONNECTING: "Connecting to the printer\nAttempt {n}", + ConnectionState.CONNECTING: "Connecting to the printer\nAttempting to reconnect", ConnectionState.WEBSOCKET_LOST: "Connection interrupted.\nAttempting to reconnect…", ConnectionState.MOONRAKER_CONNECTED: "Connection established.\nWaiting for the printer to initialise…", ConnectionState.KLIPPER_STARTUP: "The printer is starting up.\nPlease wait…", @@ -196,15 +196,19 @@ def _set_state(self, state: ConnectionState, context: str = "") -> None: return if self._handle_pending_restart(state): return - self.dot_timer.stop() - self._state = state - message = self._MESSAGES[state].format(n=context, message=context) - self.status_label.setText(message) + message = self._MESSAGES[state].format(message=context) if state == ConnectionState.CONNECTING: - self.base_text = message - self.dot_count = 0 - self.dot_timer.start() - elif state == ConnectionState.KLIPPER_READY: + # Attempts land every 3s off the 1s beat, restarting would desync the dots + if not self.dot_timer.isActive(): + self.base_text = message + self.dot_count = 0 + self._add_dot() + self.dot_timer.start() + else: + self.dot_timer.stop() + self.status_label.setText(message) + self._state = state + if state == ConnectionState.KLIPPER_READY: self._stop_restart_overlay() self._last_shutdown_context = "" self.hide() @@ -220,11 +224,8 @@ def _stop_restart_overlay(self) -> None: self.call_load_panel.emit(False, "", False) def _add_dot(self) -> None: - self.dot_count += 1 - if self.dot_count > 3: - self.dot_count = 0 - dots = "." * self.dot_count - self.status_label.setText(f"{self.base_text}{dots}") + self.dot_count = self.dot_count % 3 + 1 + self.status_label.setText(f"{self.base_text}{'.' * self.dot_count}") def _update_restart_label(self, state: ConnectionState) -> None: """Set restart_klipper_button label based on current state.""" @@ -317,10 +318,10 @@ def on_klippy_state(self, state: str) -> None: else: logger.warning("Unknown Klipper state: %s", state) - @QtCore.pyqtSlot(int, name="on_websocket_connecting") - def on_websocket_connecting(self, attempt: int) -> None: + @QtCore.pyqtSlot(name="on_websocket_connecting") + def on_websocket_connecting(self) -> None: """Handle websocket reconnection attempts.""" - self._set_state(ConnectionState.CONNECTING, context=str(attempt)) + self._set_state(ConnectionState.CONNECTING) @QtCore.pyqtSlot(name="on_websocket_connection_achieved") def on_websocket_connection_achieved(self) -> None: diff --git a/tests/widgets/test_connection_page.py b/tests/widgets/test_connection_page.py index a6f8f542d..40fc9c708 100644 --- a/tests/widgets/test_connection_page.py +++ b/tests/widgets/test_connection_page.py @@ -36,9 +36,9 @@ def test_disconnected(self, page): assert "offline" in page.status_label.text() def test_connecting(self, page): - page._set_state(ConnectionState.CONNECTING, context="3") + page.on_websocket_connecting() assert page._state == ConnectionState.CONNECTING - assert "Attempt 3" in page.status_label.text() + assert page.status_label.text().endswith("Attempting to reconnect.") def test_websocket_lost(self, page): page._set_state(ConnectionState.WEBSOCKET_LOST) @@ -94,30 +94,33 @@ def test_real_shutdown_reason_can_be_overwritten_by_another_real_reason(self, pa class TestDotTimer: def test_connecting_starts_timer(self, page): - page._set_state(ConnectionState.CONNECTING, context="1") + page._set_state(ConnectionState.CONNECTING) assert page.dot_timer.isActive() def test_non_connecting_stops_timer(self, page): - page._set_state(ConnectionState.CONNECTING, context="1") + page._set_state(ConnectionState.CONNECTING) page._set_state(ConnectionState.MOONRAKER_CONNECTED) assert not page.dot_timer.isActive() def test_add_dot_cycles(self, page): page.base_text = "Connecting" page.dot_count = 0 - page._add_dot() - assert page.status_label.text() == "Connecting." - page._add_dot() - assert page.status_label.text() == "Connecting.." - page._add_dot() - assert page.status_label.text() == "Connecting..." - page._add_dot() - assert page.status_label.text() == "Connecting" + for dots in [".", "..", "...", "."]: + page._add_dot() + assert page.status_label.text() == f"Connecting{dots}" def test_dot_count_resets_on_connecting(self, page): page.dot_count = 2 - page._set_state(ConnectionState.CONNECTING, context="1") - assert page.dot_count == 0 + page._set_state(ConnectionState.CONNECTING) + assert page.dot_count == 1 + + def test_repeat_attempts_keep_dot_beat(self, page): + # Attempts come every 3s from the retry thread, off the 1s dot beat + page.on_websocket_connecting() + page._add_dot() + page.on_websocket_connecting() + assert page.status_label.text().endswith("reconnect..") + assert page.dot_count == 2 class TestVisibility: @@ -221,8 +224,8 @@ def test_no_self_panel_attribute(self, page): def test_dot_count_resets_on_connecting(self, page): page.dot_count = 3 - page._set_state(ConnectionState.CONNECTING, context=1) - assert page.dot_count == 0 + page._set_state(ConnectionState.CONNECTING) + assert page.dot_count == 1 def test_websocket_lost_does_not_show_when_toggle_off(self, page): page.conn_toggle = False