From 4f2d29892b666c5c7b7c6653d589436faf557d6f Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Thu, 2 Jul 2026 13:38:34 -0700 Subject: [PATCH 1/3] fix: verify MQTT connection requires successful subscription --- roborock/devices/rpc/v1_channel.py | 2 +- tests/devices/rpc/test_v1_channel.py | 53 ++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/roborock/devices/rpc/v1_channel.py b/roborock/devices/rpc/v1_channel.py index 3197c6b3..0de87d38 100644 --- a/roborock/devices/rpc/v1_channel.py +++ b/roborock/devices/rpc/v1_channel.py @@ -205,7 +205,7 @@ def is_local_connected(self) -> bool: @property def is_mqtt_connected(self) -> bool: """Return whether MQTT connection is available.""" - return self._mqtt_channel.is_connected + return self._mqtt_channel.is_connected and self._mqtt_unsub is not None @property def rpc_channel(self) -> V1RpcChannel: diff --git a/tests/devices/rpc/test_v1_channel.py b/tests/devices/rpc/test_v1_channel.py index 24e63c77..77954ca0 100644 --- a/tests/devices/rpc/test_v1_channel.py +++ b/tests/devices/rpc/test_v1_channel.py @@ -233,6 +233,59 @@ async def test_v1_channel_mqtt_disconnected( assert not mock_mqtt_channel.subscribers +async def test_v1_channel_mqtt_subscription_fails_local_succeeds( + v1_channel: V1Channel, + mock_mqtt_channel: FakeChannel, + mock_local_channel: FakeChannel, + mock_local_session: Mock, + device_cache: DeviceCache, +) -> None: + """Test MQTT subscription failure while local connection succeeds.""" + # Pre-populate cache so we don't query network info via MQTT + device_cache_data = await device_cache.get() + device_cache_data.network_info = TEST_NETWORKING_INFO + await device_cache.set(device_cache_data) + + # Simulate MQTT subscription failing + mock_mqtt_channel.subscribe.side_effect = RoborockException("MQTT subscription failed") + + # Subscribe should succeed via local fallback + callback = Mock() + unsub = await v1_channel.subscribe(callback) + + # Verify MQTT is not reported as connected, but local is + assert not v1_channel.is_mqtt_connected + assert v1_channel.is_local_connected + assert v1_channel.is_connected + + unsub() + + +async def test_v1_channel_all_connection_attempts_fail( + v1_channel: V1Channel, + mock_mqtt_channel: FakeChannel, + mock_local_channel: FakeChannel, + mock_local_session: Mock, + device_cache: DeviceCache, +) -> None: + """Test when both local connect and MQTT subscribe fail.""" + # Pre-populate cache so we don't query network info via MQTT + device_cache_data = await device_cache.get() + device_cache_data.network_info = TEST_NETWORKING_INFO + await device_cache.set(device_cache_data) + + mock_local_channel.connect.side_effect = RoborockException("local down") + mock_mqtt_channel.subscribe.side_effect = RoborockException("MQTT subscription failed") + + with pytest.raises(RoborockException): + await v1_channel.subscribe(Mock()) + + # After a failed subscription, properties should reflect no active connection + assert not v1_channel.is_mqtt_connected + assert not v1_channel.is_local_connected + assert not v1_channel.is_connected + + async def test_v1_channel_subscribe_local_success( v1_channel: V1Channel, mock_mqtt_channel: Mock, From a595411c43f48486382d0cd5e64306d1eccef0e6 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Thu, 2 Jul 2026 13:43:21 -0700 Subject: [PATCH 2/3] docs: clarify is_mqtt_connected docstring with motivation --- roborock/devices/rpc/v1_channel.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/roborock/devices/rpc/v1_channel.py b/roborock/devices/rpc/v1_channel.py index 0de87d38..63028e6b 100644 --- a/roborock/devices/rpc/v1_channel.py +++ b/roborock/devices/rpc/v1_channel.py @@ -204,7 +204,13 @@ def is_local_connected(self) -> bool: @property def is_mqtt_connected(self) -> bool: - """Return whether MQTT connection is available.""" + """Return whether MQTT connection is available for the device. + + This requires the MQTT session to be connected to the broker and + the subscription to the device's topic to have successfully + been established (to handle cases where the device is offline + or deleted). + """ return self._mqtt_channel.is_connected and self._mqtt_unsub is not None @property From f6f0c5acf3b78309b1062c6427a80aaf9fb853f9 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Thu, 2 Jul 2026 14:07:43 -0700 Subject: [PATCH 3/3] refactor: remove unused fixtures in tests --- tests/devices/rpc/test_v1_channel.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/devices/rpc/test_v1_channel.py b/tests/devices/rpc/test_v1_channel.py index 77954ca0..278a796b 100644 --- a/tests/devices/rpc/test_v1_channel.py +++ b/tests/devices/rpc/test_v1_channel.py @@ -236,8 +236,6 @@ async def test_v1_channel_mqtt_disconnected( async def test_v1_channel_mqtt_subscription_fails_local_succeeds( v1_channel: V1Channel, mock_mqtt_channel: FakeChannel, - mock_local_channel: FakeChannel, - mock_local_session: Mock, device_cache: DeviceCache, ) -> None: """Test MQTT subscription failure while local connection succeeds.""" @@ -265,7 +263,6 @@ async def test_v1_channel_all_connection_attempts_fail( v1_channel: V1Channel, mock_mqtt_channel: FakeChannel, mock_local_channel: FakeChannel, - mock_local_session: Mock, device_cache: DeviceCache, ) -> None: """Test when both local connect and MQTT subscribe fail."""