diff --git a/README.md b/README.md index 2a059a2..c727cc7 100644 --- a/README.md +++ b/README.md @@ -77,7 +77,6 @@ $ python -m aiocomfoconnect set-property --host 192.168.1.213 30 1 4 6 120 # Se - Properties that are an enum, like `PROPERTY_SENSOR_VENTILATION_TEMP_PASSIVE`, `PROPERTY_SENSOR_VENTILATION_HUMIDITY_COMFORT` and `PROPERTY_SENSOR_VENTILATION_HUMIDITY_PROTECTION` (off / auto / on), `PROPERTY_TEMPERATURE_PROFILE_MODE` (adaptive / fixed), `PROPERTY_CLIME_SEASON` and `PROPERTY_CLIME_TEMPERATURE_PROFILE`, have `options`: `get_property` returns the option and `set_property` takes it. `get_property_constraints` still uses the raw values. - Temperatures in tenths of a degree, like `PROPERTY_RMOT_HEATING_THRESHOLD`, `PROPERTY_PROFILE_TEMPERATURE_NORMAL` and the ComfoClime temperatures, have `scale=0.1`: `get_property` and `get_property_constraints` return °C (215 becomes 21.5) and `set_property` takes °C. - `async get_property_constraints(prop, node_id=None)`: Get the value of a numeric property, together with the range and step size the unit accepts. -- `bridge_type`: The type of the bridge (`GatewayType.LANC`, `SEASON` or `PRO`). Discovery reports it, and a ComfoConnect Pro is also recognised once a session has started. ### Low-level API diff --git a/aiocomfoconnect/__main__.py b/aiocomfoconnect/__main__.py index adef55b..d440eaa 100644 --- a/aiocomfoconnect/__main__.py +++ b/aiocomfoconnect/__main__.py @@ -43,7 +43,7 @@ async def connected(args, *, connect: bool = True, **kwargs) -> AsyncIterator[Co if not bridges: raise BridgeNotFoundException("No bridge found") - comfoconnect = ComfoConnect(bridges[0].host, bridges[0].uuid, bridge_type=bridges[0].bridge_type, **kwargs) + comfoconnect = ComfoConnect(bridges[0].host, bridges[0].uuid, **kwargs) try: if connect: try: @@ -82,14 +82,11 @@ async def run_register(args): """Register an app on the bridge.""" async with connected(args, connect=False) as comfoconnect: try: - registered = await comfoconnect.register(args.uuid, args.name, args.pin) + await comfoconnect.register(args.uuid, args.name, args.pin) except ComfoConnectNotAllowed as exc: raise CliError("Registration failed. Please check the PIN.") from exc - if registered: - print(f"UUID {args.uuid} is now registered.") - else: - print(f"UUID {args.uuid} is already registered.") + print(f"UUID {args.uuid} is registered.") await print_registered_apps(comfoconnect) diff --git a/aiocomfoconnect/bridge.py b/aiocomfoconnect/bridge.py index 858c921..436c095 100644 --- a/aiocomfoconnect/bridge.py +++ b/aiocomfoconnect/bridge.py @@ -14,7 +14,7 @@ from google.protobuf.message import DecodeError from google.protobuf.message import Message as ProtobufMessage -from .const import BRIDGE_PRODUCT_IDS, NUMERIC_PROPERTY_TYPES, VENTILATION_UNIT_PRODUCT_IDS, GatewayType, ProductId +from .const import BRIDGE_PRODUCT_IDS, NUMERIC_PROPERTY_TYPES, VENTILATION_UNIT_PRODUCT_IDS, ProductId from .exceptions import ( AioComfoConnectNotConnected, AioComfoConnectNotReachable, @@ -101,12 +101,9 @@ def __init__( self, host: str, uuid: str, - bridge_type: int = 0, ): self.host: str = host self.uuid: str = uuid - # One of GatewayType. Discovery reports it, and a ComfoConnect Pro also identifies itself when a session starts. - self.bridge_type: int = bridge_type self._local_uuid: Optional[str] = None self._reader: Optional[StreamReader] = None @@ -151,27 +148,16 @@ async def connect(self, uuid: str): """Connect to the bridge and start reading messages.""" await self._open_connection(uuid) - async def register(self, uuid: str, name: str, pin: int) -> bool: + async def register(self, uuid: str, name: str, pin: int) -> None: """Register this app on the bridge and start a session. - For LAN C bridges, attempts to start a session first; if the session - succeeds the app is already registered and no re-registration occurs. - For Pro bridges, registers directly to avoid a connection timeout that - the Pro issues when the app is not yet registered. - - Returns True if the app was newly registered, False if it was already registered. + Like the official app, this registers directly: a bridge accepts an app that is registered already, as long as + the PIN is right. We don't start a session first to find out, since a ComfoConnect Pro doesn't answer that for an + app that isn't registered. ComfoConnectNotAllowed is raised for a wrong PIN. """ await self._open_connection(uuid) - if self.bridge_type != GatewayType.PRO: - # LAN C: check whether we are already registered by starting a session. - try: - await self.cmd_start_session() - return False # Already registered; session is now active. - except ComfoConnectNotAllowed: - pass # Not registered yet; fall through to register below. await self.cmd_register_app(uuid, name, pin) await self.cmd_start_session() - return True async def _open_connection(self, uuid: str): """Open TCP connection to the bridge and start reading messages.""" @@ -496,16 +482,11 @@ async def cmd_start_session(self, take_over: bool = False) -> Message: """Starts the session on the device by logging in and optionally disconnecting an already existing session.""" _LOGGER.debug("StartSessionRequest") # pylint: disable=no-member - reply = await self._send( + return await self._send( zehnder_pb2.StartSessionRequest, zehnder_pb2.GatewayOperation.StartSessionRequestType, {"takeover": take_over}, ) - # A ComfoConnect Pro sets the devicename of a successful session to "ccpro", a LAN C leaves it empty. This also - # tells us the type of a bridge that was connected to by IP address, without discovery. - if getattr(reply, "devicename", None) == "ccpro": - self.bridge_type = GatewayType.PRO - return reply def cmd_close_session(self) -> Awaitable[Message]: """Stops the current session.""" diff --git a/aiocomfoconnect/comfoconnect.py b/aiocomfoconnect/comfoconnect.py index b6e0260..da52c5a 100644 --- a/aiocomfoconnect/comfoconnect.py +++ b/aiocomfoconnect/comfoconnect.py @@ -146,7 +146,6 @@ def __init__( alarm_callback=None, sensor_delay=5, connect_timeout=30, - bridge_type: int = 0, heartbeat_interval: Optional[float] = HEARTBEAT_INTERVAL, connection_state_callback: Optional[Callable[[str], None]] = None, node_callback: Optional[Callable[[Node], None]] = None, @@ -162,7 +161,7 @@ def __init__( The node_callback is called with a Node every time the bridge announces one: at the start of every session, already while connect() runs and often more than once, and when a node joins or goes offline (see Node.is_offline). """ - super().__init__(host, uuid, bridge_type) + super().__init__(host, uuid) self.set_sensor_callback(self._sensor_callback) # Set the callback to our _sensor_callback method, so we can proces the callbacks. self.set_alarm_callback(self._alarm_callback) # Set the callback to our _alarm_callback method, so we can proces the callbacks. diff --git a/aiocomfoconnect/const.py b/aiocomfoconnect/const.py index d03faf1..d7c2352 100644 --- a/aiocomfoconnect/const.py +++ b/aiocomfoconnect/const.py @@ -36,14 +36,6 @@ class ProductId: # PDO Types -class GatewayType: - """Defines the type of a bridge, with the values of the GatewayType in the discovery response.""" - - LANC = 0 # ComfoConnect LAN C - SEASON = 1 # The built-in WiFi module of a ComfoAir Flex - PRO = 2 # ComfoConnect Pro - - class ConnectionState: """The state of the session with the bridge, as reported to the connection state callback.""" diff --git a/aiocomfoconnect/discovery.py b/aiocomfoconnect/discovery.py index da0a827..3ddc60a 100644 --- a/aiocomfoconnect/discovery.py +++ b/aiocomfoconnect/discovery.py @@ -77,7 +77,6 @@ def datagram_received(self, data: Union[bytes, str], addr: tuple[str | Any, int] Bridge( host=parser.searchGatewayResponse.ipaddress, uuid=uuid, - bridge_type=parser.searchGatewayResponse.type, ) ) diff --git a/docs/PROTOCOL.md b/docs/PROTOCOL.md index c5b3bd8..e906318 100644 --- a/docs/PROTOCOL.md +++ b/docs/PROTOCOL.md @@ -197,6 +197,12 @@ result: NOT_ALLOWED reference: 15 ``` +Registering a UUID that is registered already succeeds as well, with the right PIN, and doesn't add another entry to +the registration list; the bridge keeps the devicename of the first registration. With a wrong PIN it fails with +`NOT_ALLOWED`, also for a UUID that is registered already. (Seen on a LAN C.) The official app doesn't start a session +first to find out whether it is registered: it registers a new UUID directly, with PIN `0` first and then the PIN the +user enters. + #### StartSession (`StartSessionRequestType` and `StartSessionConfirmType`) The client logs in by sending a `type: StartSessionRequestType`. Only one client can be logged in at the same time. diff --git a/tests/test_bridge.py b/tests/test_bridge.py index c438bf1..ffe5d13 100644 --- a/tests/test_bridge.py +++ b/tests/test_bridge.py @@ -2,13 +2,13 @@ import asyncio import gc -from unittest.mock import AsyncMock, MagicMock, Mock, patch +from unittest.mock import AsyncMock, MagicMock, Mock, call, patch import pytest from aiocomfoconnect.bridge import Bridge, Message, Node from aiocomfoconnect.comfoconnect import ComfoConnect -from aiocomfoconnect.const import GatewayType, ProductId +from aiocomfoconnect.const import ProductId from aiocomfoconnect.protobuf import zehnder_pb2 from tests.conftest import create_sensor, node_notification @@ -176,144 +176,38 @@ async def test_rmi_request_when_not_connected(self, bridge): class TestBridgeRegister: """Tests for Bridge.register().""" - @pytest.fixture - def lanc_bridge(self): - """A Bridge with bridge_type=0 (LAN C, the default).""" - return Bridge("192.168.1.100", LOCAL_UUID, bridge_type=0) - - @pytest.fixture - def pro_bridge(self): - """A Bridge with bridge_type=GatewayType.PRO.""" - return Bridge("192.168.1.100", LOCAL_UUID, bridge_type=GatewayType.PRO) - - @pytest.mark.asyncio - async def test_lanc_already_registered(self, lanc_bridge): - """LAN C: session starts successfully → already registered, no cmd_register_app call.""" - with ( - patch.object(lanc_bridge, "_open_connection", new_callable=AsyncMock) as mock_open, - patch.object(lanc_bridge, "cmd_start_session", new_callable=AsyncMock) as mock_session, - patch.object(lanc_bridge, "cmd_register_app", new_callable=AsyncMock) as mock_register, - ): - - result = await lanc_bridge.register(LOCAL_UUID, "test-app", 1234) - - assert result is False - mock_open.assert_called_once_with(LOCAL_UUID) - mock_session.assert_called_once_with() - mock_register.assert_not_called() - - @pytest.mark.asyncio - async def test_lanc_not_registered(self, lanc_bridge): - """LAN C: session returns NotAllowed → registers then starts session, returns True.""" - with ( - patch.object(lanc_bridge, "_open_connection", new_callable=AsyncMock) as mock_open, - patch.object(lanc_bridge, "cmd_start_session", new_callable=AsyncMock) as mock_session, - patch.object(lanc_bridge, "cmd_register_app", new_callable=AsyncMock) as mock_register, - ): - - mock_session.side_effect = [ComfoConnectNotAllowed("not registered"), None] - - result = await lanc_bridge.register(LOCAL_UUID, "test-app", 1234) - - assert result is True - mock_open.assert_called_once_with(LOCAL_UUID) - mock_register.assert_called_once_with(LOCAL_UUID, "test-app", 1234) - assert mock_session.call_count == 2 - @pytest.mark.asyncio - async def test_lanc_wrong_pin(self, lanc_bridge): - """LAN C: not registered, wrong PIN → cmd_register_app fails, exception propagates.""" - with ( - patch.object(lanc_bridge, "_open_connection", new_callable=AsyncMock), - patch.object(lanc_bridge, "cmd_start_session", new_callable=AsyncMock) as mock_session, - patch.object(lanc_bridge, "cmd_register_app", new_callable=AsyncMock) as mock_register, - ): - - mock_session.side_effect = ComfoConnectNotAllowed("not registered") - mock_register.side_effect = ComfoConnectNotAllowed("wrong pin") - - with pytest.raises(ComfoConnectNotAllowed): - await lanc_bridge.register(LOCAL_UUID, "test-app", 9999) - - mock_session.assert_called_once_with() # checked once, not retried after failed register - mock_register.assert_called_once_with(LOCAL_UUID, "test-app", 9999) - - @pytest.mark.asyncio - async def test_pro_registers_directly_without_session_check(self, pro_bridge): - """Pro: skips the session-first check, calls cmd_register_app then cmd_start_session.""" - call_order = [] - - async def track_register(*args): - call_order.append("register") - - async def track_session(*args): - call_order.append("session") - + async def test_registers_then_starts_session(self): + """A bridge registers directly, also when the app is registered already, and then starts a session.""" + bridge = Bridge("192.168.1.100", LOCAL_UUID) + calls = Mock() + calls.attach_mock(AsyncMock(), "register") + calls.attach_mock(AsyncMock(), "session") with ( - patch.object(pro_bridge, "_open_connection", new_callable=AsyncMock) as mock_open, - patch.object(pro_bridge, "cmd_register_app", side_effect=track_register) as mock_register, - patch.object(pro_bridge, "cmd_start_session", side_effect=track_session) as mock_session, + patch.object(bridge, "_open_connection", new_callable=AsyncMock) as mock_open, + patch.object(bridge, "cmd_register_app", calls.register), + patch.object(bridge, "cmd_start_session", calls.session), ): + await bridge.register(LOCAL_UUID, "test-app", 1234) - result = await pro_bridge.register(LOCAL_UUID, "test-app", 1234) - - assert result is True - mock_open.assert_called_once_with(LOCAL_UUID) - mock_register.assert_called_once_with(LOCAL_UUID, "test-app", 1234) - mock_session.assert_called_once_with() - assert call_order == ["register", "session"] + mock_open.assert_awaited_once_with(LOCAL_UUID) + assert calls.mock_calls == [call.register(LOCAL_UUID, "test-app", 1234), call.session()] @pytest.mark.asyncio - async def test_pro_wrong_pin(self, pro_bridge): - """Pro: cmd_register_app fails with wrong PIN → exception propagates, session never started.""" + async def test_wrong_pin(self): + """A wrong PIN fails the registration, and no session is started.""" + bridge = Bridge("192.168.1.100", LOCAL_UUID) with ( - patch.object(pro_bridge, "_open_connection", new_callable=AsyncMock), - patch.object(pro_bridge, "cmd_register_app", new_callable=AsyncMock) as mock_register, - patch.object(pro_bridge, "cmd_start_session", new_callable=AsyncMock) as mock_session, + patch.object(bridge, "_open_connection", new_callable=AsyncMock), + patch.object(bridge, "cmd_register_app", new_callable=AsyncMock, side_effect=ComfoConnectNotAllowed("wrong pin")), + patch.object(bridge, "cmd_start_session", new_callable=AsyncMock) as mock_session, ): - - mock_register.side_effect = ComfoConnectNotAllowed("wrong pin") - with pytest.raises(ComfoConnectNotAllowed): - await pro_bridge.register(LOCAL_UUID, "test-app", 9999) + await bridge.register(LOCAL_UUID, "test-app", 9999) mock_session.assert_not_called() -class TestGatewayTypeFromSession: - """Tests for recognising a ComfoConnect Pro when the session starts.""" - - @pytest.mark.asyncio - async def test_lanc_session(self): - """A LAN C leaves the devicename of the session confirmation empty, so the type stays LAN C.""" - bridge = Bridge("192.168.1.100", LOCAL_UUID) - with patch.object(bridge, "_send", AsyncMock(return_value=zehnder_pb2.StartSessionConfirm())): - await bridge.cmd_start_session(True) - - assert bridge.bridge_type == GatewayType.LANC - - @pytest.mark.asyncio - async def test_pro_session(self): - """A Pro identifies itself with devicename "ccpro", also when it wasn't found by discovery.""" - bridge = Bridge("192.168.1.100", LOCAL_UUID) - - with patch.object(bridge, "_send", AsyncMock(return_value=zehnder_pb2.StartSessionConfirm(devicename="ccpro"))) as mock_send: - reply = await bridge.cmd_start_session(True) - - assert bridge.bridge_type == GatewayType.PRO - assert reply.devicename == "ccpro" - assert mock_send.call_args[0][2] == {"takeover": True} - - @pytest.mark.asyncio - async def test_discovered_type_is_kept(self): - """A session without "ccpro" keeps the type from discovery, e.g. the WiFi module of a ComfoAir Flex.""" - bridge = Bridge("192.168.1.100", LOCAL_UUID, bridge_type=GatewayType.SEASON) - with patch.object(bridge, "_send", AsyncMock(return_value=zehnder_pb2.StartSessionConfirm())): - await bridge.cmd_start_session(True) - - assert bridge.bridge_type == GatewayType.SEASON - - class TestResultCodes: """Test how the result code of a reply is turned into an exception for the request that waits for it.""" diff --git a/tests/test_cli.py b/tests/test_cli.py index 62ab340..62bf5eb 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -35,7 +35,7 @@ def bridge(): comfoconnect.cmd_deregister_app = AsyncMock() comfoconnect.cmd_list_registered_apps = AsyncMock(return_value=SimpleNamespace(apps=[])) - discovered = [SimpleNamespace(host=FAKE_HOST, uuid=FAKE_BRIDGE_UUID, bridge_type=0)] + discovered = [SimpleNamespace(host=FAKE_HOST, uuid=FAKE_BRIDGE_UUID)] with ( patch.object(cli, "discover_bridges", AsyncMock(return_value=discovered)), patch.object(cli, "ComfoConnect", MagicMock(return_value=comfoconnect)), @@ -169,7 +169,7 @@ async def test_uses_discovered_bridge(self, bridge): pass cli.discover_bridges.assert_awaited_once_with(FAKE_HOST) - cli.ComfoConnect.assert_called_once_with(FAKE_HOST, FAKE_BRIDGE_UUID, bridge_type=0) + cli.ComfoConnect.assert_called_once_with(FAKE_HOST, FAKE_BRIDGE_UUID) class TestCommands: