Merge branch 'main' into keepalive

Author: allenporter

CHANGELOG.md

@@ -1,6 +1,72 @@
 # CHANGELOG
 
 
+## v2.9.4 (2025-01-21)
+
+### Bug Fixes
+
+- Set unique sequence numbers on outgoing messages
+  ([#300](https://github.com/Python-roborock/python-roborock/pull/300),
+  [`14f03c7`](https://github.com/Python-roborock/python-roborock/commit/14f03c7df1c574ab87ea056227bb95f9150f4832))
+
+- Bump paho-mqtt from 1.6.1 to 2.1.0
+  ([#288](https://github.com/Python-roborock/python-roborock/pull/288),
+  [`777b736`](https://github.com/Python-roborock/python-roborock/commit/777b736440a3633c089bf09ab9d7240e54e0fb0e))
+
+Bumps [paho-mqtt](https://github.com/eclipse/paho.mqtt.python) from 1.6.1 to 2.1.0. - [Release
+  notes](https://github.com/eclipse/paho.mqtt.python/releases) -
+  [Changelog](https://github.com/eclipse-paho/paho.mqtt.python/blob/master/ChangeLog.txt) -
+  [Commits](https://github.com/eclipse/paho.mqtt.python/compare/v1.6.1...v2.1.0)
+
+--- updated-dependencies: - dependency-name: paho-mqtt dependency-type: direct:production
+  update-type: version-update:semver-major ...
+
+Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot]
+  <49699333+dependabot[bot]@users.noreply.github.com>
+
+### Chores
+
+- Fix flaky tests by cleaning up threads
+  ([#303](https://github.com/Python-roborock/python-roborock/pull/303),
+  [`6e29e74`](https://github.com/Python-roborock/python-roborock/commit/6e29e7440f61ddde9a67b25c87864ed0cbf1a097))
+
+* chore: set log level to debug to aid in tracking down flaky tests
+
+* test: update log format to include timestamps and dates
+
+test: update logmessage with package name chore: fix tests to use valid zeo codes
+
+* test: fix zeo test assertion
+
+* test: add logging when updating future
+
+* test: make the client read socket always available for reading to avoid getting blocked
+
+* test: revert socket changes
+
+* test: set function loop scope
+
+* test: add pytest-timeout with a 20 second hard timeout
+
+* test: explicitly disconnect threads
+
+* test: fix formatting
+
+* test: fix lint errors
+
+* fix: stop the mqtt loop on disconnect
+
+* fix: release the mqtt thread on release
+
+* test: revert log changes
+
+* chore: cleanup/revert changes
+
+* chore: revert mqtt client check
+
+* fix: always stop the event loop when disconnecting
+
+
 ## v2.9.3 (2025-01-21)
 
 ### Bug Fixes

poetry.lock

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "python-roborock"
-version = "2.9.3"
+version = "2.9.4"
 description = "A package to control Roborock vacuums."
 authors = ["humbertogontijo <humbertogontijo@users.noreply.github.com>"]
 license = "GPL-3.0-only"
@@ -46,6 +46,7 @@ codespell = "*"
 pyshark = "^0.6"
 aioresponses = "^0.7.7"
 freezegun = "^1.5.1"
+pytest-timeout = "^2.3.1"
 
 [tool.semantic_release]
 branch = "main"
@@ -71,3 +72,5 @@ select=["E", "F", "UP", "I"]
 
 [tool.pytest.ini_options]
 asyncio_mode = "auto"
+asyncio_default_fixture_loop_scope = "function"
+timeout = 20

pyproject.toml

@@ -82,6 +82,8 @@ def _mqtt_on_connect(self, *args, **kwargs):
             self._logger.error(message)
             if connection_queue:
                 connection_queue.set_exception(VacuumError(message))
+            else:
+                self._logger.debug("Failed to notify connect future, not in queue")
             return
         self._logger.info(f"Connected to mqtt {self._mqtt_host}:{self._mqtt_port}")
         topic = f"rr/m/o/{self._mqtt_user}/{self._hashed_user}/{self.device_info.device.duid}"
@@ -154,10 +156,9 @@ def sync_connect(self) -> Any:
     async def async_disconnect(self) -> None:
         async with self._mutex:
             if disconnected_future := self.sync_disconnect():
-                try:
-                    await disconnected_future
-                except VacuumError as err:
-                    raise RoborockException(err) from err
+                # There are no errors set on this future
+                await disconnected_future
+            await self.event_loop.run_in_executor(None, self._mqtt_client.loop_stop)
 
     async def async_connect(self) -> None:
         async with self._mutex:

roborock/cloud_api.py

@@ -128,8 +128,8 @@ def on_message_received(self, messages: list[RoborockMessage]) -> None:
                     continue
                 payload_json = json.loads(payload.decode())
                 for data_point_number, data_point in payload_json.get("dps").items():
-                    self._logger.debug("data point number=%s", data_point_number)
                     data_point_protocol: RoborockDyadDataProtocol | RoborockZeoProtocol
+                    self._logger.debug("received msg with dps, protocol: %s, %s", data_point_number, protocol)
                     entries: dict
                     if self.category == RoborockCategory.WET_DRY_VAC:
                         data_point_protocol = RoborockDyadDataProtocol(int(data_point_number))

roborock/version_a01_apis/roborock_client_a01.py

@@ -3,7 +3,7 @@
 import logging
 import re
 from asyncio import Protocol
-from collections.abc import Callable, Generator
+from collections.abc import AsyncGenerator, Callable, Generator
 from queue import Queue
 from typing import Any
 from unittest.mock import Mock, patch
@@ -138,16 +138,22 @@ def handle_select(rlist: list, wlist: list, *args: Any) -> list:
 
 
 @pytest.fixture(name="mqtt_client")
-def mqtt_client(mock_create_connection: None, mock_select: None) -> Generator[RoborockMqttClientV1, None, None]:
+async def mqtt_client(mock_create_connection: None, mock_select: None) -> AsyncGenerator[RoborockMqttClientV1, None]:
     user_data = UserData.from_dict(USER_DATA)
     home_data = HomeData.from_dict(HOME_DATA_RAW)
     device_info = DeviceData(
         device=home_data.devices[0],
         model=home_data.products[0].model,
     )
     client = RoborockMqttClientV1(user_data, device_info)
-    yield client
-    # Clean up any resources after the test
+    try:
+        yield client
+    finally:
+        if not client.is_connected():
+            try:
+                await client.async_release()
+            except Exception:
+                pass
 
 
 @pytest.fixture(name="mock_rest", autouse=True)
@@ -226,11 +232,19 @@ def handle_write(data: bytes) -> None:
 
 
 @pytest.fixture(name="local_client")
-def local_client_fixture(mock_create_local_connection: None) -> Generator[RoborockLocalClientV1, None, None]:
+async def local_client_fixture(mock_create_local_connection: None) -> AsyncGenerator[RoborockLocalClientV1, None]:
     home_data = HomeData.from_dict(HOME_DATA_RAW)
     device_info = DeviceData(
         device=home_data.devices[0],
         model=home_data.products[0].model,
         host=TEST_LOCAL_API_HOST,
     )
-    yield RoborockLocalClientV1(device_info)
+    client = RoborockLocalClientV1(device_info)
+    try:
+        yield client
+    finally:
+        if not client.is_connected():
+            try:
+                await client.async_release()
+            except Exception:
+                pass

tests/conftest.py

@@ -36,7 +36,9 @@
 
 
 @pytest.fixture(name="a01_mqtt_client")
-async def a01_mqtt_client_fixture(mock_create_connection: None, mock_select: None) -> RoborockMqttClientA01:
+async def a01_mqtt_client_fixture(
+    mock_create_connection: None, mock_select: None
+) -> AsyncGenerator[RoborockMqttClientA01, None]:
     user_data = UserData.from_dict(USER_DATA)
     home_data = HomeData.from_dict(
         {
@@ -49,7 +51,15 @@ async def a01_mqtt_client_fixture(mock_create_connection: None, mock_select: Non
         device=home_data.devices[0],
         model=home_data.products[0].model,
     )
-    return RoborockMqttClientA01(user_data, device_info, RoborockCategory.WASHING_MACHINE)
+    client = RoborockMqttClientA01(user_data, device_info, RoborockCategory.WASHING_MACHINE)
+    try:
+        yield client
+    finally:
+        if not client.is_connected():
+            try:
+                await client.async_release()
+            except Exception:
+                pass
 
 
 @pytest.fixture(name="connected_a01_mqtt_client")
@@ -174,13 +184,13 @@ async def test_update_values(
 
     message = build_rpc_response(
         {
-            203: [[16, "2362048"], [17, "2362044"]],
+            203: 6,  # spinning
         }
     )
     response_queue.put(mqtt_packet.gen_publish(MQTT_PUBLISH_TOPIC, payload=message))
 
     data = await connected_a01_mqtt_client.update_values([RoborockZeoProtocol.STATE])
-    assert data.get(RoborockZeoProtocol.STATE) == "standby"
+    assert data.get(RoborockZeoProtocol.STATE) == "spinning"
 
 
 async def test_publish_failure(

tests/test_a01_api.py

@@ -1,5 +1,6 @@
 import asyncio
 import json
+import logging
 from collections.abc import AsyncGenerator
 from queue import Queue
 from typing import Any
@@ -168,7 +169,7 @@ async def test_async_connect(received_requests: Queue, connected_mqtt_client: Ro
     assert received_requests.qsize() >= 2  # Connect and Subscribe
 
 
-async def test_connect_failure(
+async def test_connect_failure_response(
     received_requests: Queue, response_queue: Queue, mqtt_client: RoborockMqttClientV1
 ) -> None:
     """Test the broker responding with a connect failure."""
@@ -203,6 +204,24 @@ async def test_disconnect_failure(connected_mqtt_client: RoborockMqttClientV1) -
         await connected_mqtt_client.async_disconnect()
 
 
+async def test_disconnect_failure_response(
+    received_requests: Queue,
+    response_queue: Queue,
+    connected_mqtt_client: RoborockMqttClientV1,
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """Test the broker responding with a connect failure."""
+
+    # Enqueue a failed message -- however, the client does not process any
+    # further messages and there is no parsing error, and no failed log messages.
+    response_queue.put(mqtt_packet.gen_disconnect(reason_code=1))
+    assert connected_mqtt_client.is_connected()
+    with caplog.at_level(logging.ERROR, logger="homeassistant.components.nest"):
+        await connected_mqtt_client.async_disconnect()
+        assert not connected_mqtt_client.is_connected()
+        assert not caplog.records
+
+
 async def test_async_release(connected_mqtt_client: RoborockMqttClientV1) -> None:
     """Test the async_release API will disconnect the client."""
     await connected_mqtt_client.async_release()