chore: add some tests and adress comments

Author: Lash-L

roborock/devices/local_channel.py

@@ -60,14 +60,13 @@ def __init__(self, host: str, local_key: str):
         self._local_key = local_key
         self._local_protocol_version: LocalProtocolVersion | None = None
         self._connect_nonce = get_next_int(10000, 32767)
-        self._ack_nonce: int | None = None
+        self._params: LocalChannelParams | None = None
         self._update_encoder_decoder()
 
     def _update_encoder_decoder(self, params: LocalChannelParams | None = None):
         if params is None:
-            params = LocalChannelParams(
-                local_key=self._local_key, connect_nonce=self._connect_nonce, ack_nonce=self._ack_nonce
-            )
+            params = LocalChannelParams(local_key=self._local_key, connect_nonce=self._connect_nonce, ack_nonce=None)
+        self._params = params
         self._encoder = create_local_encoder(
             local_key=params.local_key, connect_nonce=params.connect_nonce, ack_nonce=params.ack_nonce
         )
@@ -91,7 +90,7 @@ async def _do_hello(self, local_protocol_version: LocalProtocolVersion) -> Local
             seq=1,
         )
         try:
-            response = await self.send_message(
+            response = await self._send_message(
                 roborock_message=request,
                 request_id=request.seq,
                 response_protocol=RoborockMessageProtocol.HELLO_RESPONSE,
@@ -123,7 +122,6 @@ async def _hello(self):
         for version in attempt_versions:
             params = await self._do_hello(version)
             if params is not None:
-                self._ack_nonce = params.ack_nonce
                 self._local_protocol_version = version
                 self._update_encoder_decoder(params)
                 return
@@ -200,7 +198,7 @@ async def publish(self, message: RoborockMessage) -> None:
             logging.exception("Uncaught error sending command")
             raise RoborockException(f"Failed to send message: {message}") from err
 
-    async def send_message(
+    async def _send_message(
         self,
         roborock_message: RoborockMessage,
         request_id: int,

tests/devices/test_local_channel.py

@@ -8,7 +8,7 @@
 import pytest
 
 from roborock.devices.local_channel import LocalChannel, LocalChannelParams
-from roborock.exceptions import RoborockConnectionException
+from roborock.exceptions import RoborockConnectionException, RoborockException
 from roborock.protocol import create_local_decoder, create_local_encoder
 from roborock.protocols.v1_protocol import LocalProtocolVersion
 from roborock.roborock_message import RoborockMessage, RoborockMessageProtocol
@@ -61,7 +61,7 @@ async def setup_local_channel_with_hello_mock() -> LocalChannel:
     """Fixture to set up the local channel with automatic hello mocking."""
     channel = LocalChannel(host=TEST_HOST, local_key=TEST_LOCAL_KEY)
 
-    async def mock_do_hello(local_protocol_version):
+    async def mock_do_hello(_: LocalProtocolVersion):
         """Mock _do_hello to return successful params without sending actual request."""
         return LocalChannelParams(local_key=channel._local_key, connect_nonce=channel._connect_nonce, ack_nonce=54321)
 
@@ -269,5 +269,96 @@ async def mock_do_hello(local_protocol_version: LocalProtocolVersion) -> LocalCh
 
     # Verify that the channel is using L01 protocol
     assert channel._local_protocol_version == LocalProtocolVersion.L01
-    assert channel._ack_nonce == 54321
+    assert channel._params is not None
+    assert channel._params.ack_nonce == 54321
+    assert channel._is_connected is True
+
+
+async def test_hello_success_with_v1_protocol_first(mock_loop: Mock, mock_transport: Mock) -> None:
+    """Test that when V1 protocol succeeds on first attempt, we use V1."""
+
+    # Create a channel without the automatic hello mocking
+    channel = LocalChannel(host=TEST_HOST, local_key=TEST_LOCAL_KEY)
+
+    # Mock _do_hello to succeed for V1 on first attempt
+    async def mock_do_hello(local_protocol_version: LocalProtocolVersion) -> LocalChannelParams | None:
+        if local_protocol_version == LocalProtocolVersion.V1:
+            # V1 succeeds on first attempt
+            return LocalChannelParams(
+                local_key=channel._local_key, connect_nonce=channel._connect_nonce, ack_nonce=67890
+            )
+        elif local_protocol_version == LocalProtocolVersion.L01:
+            # L01 would succeed but we shouldn't reach it
+            return LocalChannelParams(
+                local_key=channel._local_key, connect_nonce=channel._connect_nonce, ack_nonce=99999
+            )
+        return None
+
+    # Replace the _do_hello method
+    setattr(channel, "_do_hello", mock_do_hello)
+
+    # Connect and verify V1 protocol is used
+    await channel.connect()
+
+    # Verify that the channel is using V1 protocol
+    assert channel._local_protocol_version == LocalProtocolVersion.V1
+    assert channel._params is not None
+    assert channel._params.ack_nonce == 67890
+    assert channel._is_connected is True
+
+
+async def test_hello_both_protocols_fail(mock_loop: Mock, mock_transport: Mock) -> None:
+    """Test that when both V1 and L01 protocols fail, connection fails."""
+
+    # Create a channel without the automatic hello mocking
+    channel = LocalChannel(host=TEST_HOST, local_key=TEST_LOCAL_KEY)
+
+    # Mock _do_hello to fail for both protocols
+    async def mock_do_hello(_: LocalProtocolVersion) -> LocalChannelParams | None:
+        # Both protocols fail
+        return None
+
+    # Replace the _do_hello method
+    setattr(channel, "_do_hello", mock_do_hello)
+
+    # Connect should raise an exception
+    with pytest.raises(RoborockException, match="Failed to connect to device with any known protocol"):
+        await channel.connect()
+
+    # Verify that the channel is not connected and cleaned up
+    assert channel._is_connected is False
+    assert channel._transport is None
+
+
+async def test_hello_preferred_protocol_version_ordering(mock_loop: Mock, mock_transport: Mock) -> None:
+    """Test that preferred protocol version is tried first."""
+
+    # Create a channel with preferred L01 protocol
+    channel = LocalChannel(host=TEST_HOST, local_key=TEST_LOCAL_KEY)
+    channel._local_protocol_version = LocalProtocolVersion.L01
+
+    # Track which protocols were attempted and in what order
+    attempted_protocols: list[LocalProtocolVersion] = []
+
+    # Mock _do_hello to track attempts and succeed on L01
+    async def mock_do_hello(local_protocol_version: LocalProtocolVersion) -> LocalChannelParams | None:
+        attempted_protocols.append(local_protocol_version)
+        if local_protocol_version == LocalProtocolVersion.L01:
+            # L01 succeeds
+            return LocalChannelParams(
+                local_key=channel._local_key, connect_nonce=channel._connect_nonce, ack_nonce=11111
+            )
+        return None
+
+    # Replace the _do_hello method
+    setattr(channel, "_do_hello", mock_do_hello)
+
+    # Connect and verify L01 is tried first
+    await channel.connect()
+
+    # Verify that L01 was tried first (preferred version)
+    assert attempted_protocols == [LocalProtocolVersion.L01]
+    assert channel._local_protocol_version == LocalProtocolVersion.L01
+    assert channel._params is not None
+    assert channel._params.ack_nonce == 11111
     assert channel._is_connected is True