From d85410c74d3f306b4cfd2c6bdca9c8aaba8140bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Arnauts?= Date: Tue, 20 Oct 2026 11:24:33 +0000 Subject: [PATCH] Read the node info one property at a time get_node_info asked for all properties in one request, and read them one by one when that failed with an RMI error, e.g. for a bridge without a model. A ComfoConnect Pro answers that request for its own node with fewer values than we asked for, so get_properties raised a ValueError that nobody caught, and list-nodes crashed on the Pro. Just read the properties one by one: it works for every node, without guessing how a node answers the combined request. Co-Authored-By: Claude Opus 5.5 --- aiocomfoconnect/comfoconnect.py | 19 +++++++++---------- tests/test_comfoconnect.py | 28 +++++++++++++++++----------- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/aiocomfoconnect/comfoconnect.py b/aiocomfoconnect/comfoconnect.py index da52c5a..2611021 100644 --- a/aiocomfoconnect/comfoconnect.py +++ b/aiocomfoconnect/comfoconnect.py @@ -487,26 +487,25 @@ async def get_properties(self, props: List[Property], node_id: Optional[int] = N return values async def get_node_info(self, node_id: Optional[int] = None) -> NodeInfo: - """Get the NodeInfo of a node (the ventilation unit by default), in one request when the node has all its properties. + """Get the NodeInfo of a node (the ventilation unit by default). A property that the node doesn't have is None. - Raises ComfoConnectRmiError when none of the properties can be read, e.g. for a node that doesn't exist. + The properties are read one by one: a node fails a request for several as a whole when it lacks one of them, and + a ComfoConnect Pro answers it with fewer values than asked. Raises ComfoConnectRmiError when none of the + properties can be read, e.g. for a node that doesn't exist. """ props = [PROPERTY_NAME, PROPERTY_MODEL, PROPERTY_ARTICLE, PROPERTY_SERIAL_NUMBER, PROPERTY_FIRMWARE_VERSION, PROPERTY_HARDWARE_VERSION, PROPERTY_PRODUCT_VARIANT] - try: - return NodeInfo(*await self.get_properties(props, node_id)) - except ComfoConnectRmiError as exc: - error = exc + errors: List[ComfoConnectRmiError] = [] - # A request fails as a whole when the node lacks one of the properties, so read them one by one. async def get_or_none(prop: Property) -> Any: try: return await self.get_property(prop, node_id) - except ComfoConnectRmiError: + except ComfoConnectRmiError as exc: + errors.append(exc) return None values = [await get_or_none(prop) for prop in props] - if all(value is None for value in values): - raise error # Nothing could be read, e.g. the node doesn't exist. + if len(errors) == len(props): + raise errors[0] return NodeInfo(*values) async def get_single_property(self, unit: int, subunit: int, property_id: int, property_type: int = None, node_id: Optional[int] = None) -> Any: diff --git a/tests/test_comfoconnect.py b/tests/test_comfoconnect.py index 2696270..0fdc5b1 100644 --- a/tests/test_comfoconnect.py +++ b/tests/test_comfoconnect.py @@ -502,34 +502,40 @@ async def test_get_properties(props, request_hex, reply, expected): @pytest.mark.parametrize("node_id", [None, 5]) async def test_get_node_info(node_id): - """get_node_info reads the name, model, article and serial number, firmware and hardware version, and product variant of a node in one request.""" + """get_node_info reads the name, model, article and serial number, firmware and hardware version, and product variant of a node, one by one.""" comfoconnect = ComfoConnect("127.0.0.1", LOCAL_UUID) - reply = b"My unit\x00" + b"ComfoAir Q450\x00" + b"000000000\x00" + b"FAKE0000000000\x00" + bytes.fromhex("001010c0 02 02") - - with patch.object(comfoconnect, "cmd_rmi_request", AsyncMock(return_value=Mock(message=reply))) as mock_rmi: + values = { + 0x14: b"My unit\x00", + 0x08: b"ComfoAir Q450\x00", + 0x0B: b"000000000\x00", + 0x04: b"FAKE0000000000\x00", + 0x06: bytes.fromhex("001010c0"), + 0x05: b"\x02", + 0x03: b"\x02", + } + + with patch.object(comfoconnect, "cmd_rmi_request", AsyncMock(side_effect=lambda request, node_id=None: Mock(message=values[request[4]]))) as mock_rmi: info = await comfoconnect.get_node_info(node_id) assert info == NodeInfo(name="My unit", model="ComfoAir Q450", article_number="000000000", serial_number="FAKE0000000000", firmware="R1.4.0", hardware_version=2, product_variant=2) - mock_rmi.assert_awaited_once_with(bytes.fromhex("02 01 01 01 17 14 08 0b 04 06 05 03"), node_id=node_id) + assert [call.args[0] for call in mock_rmi.await_args_list] == [bytes([0x01, 0x01, 0x01, 0x10, prop]) for prop in (0x14, 0x08, 0x0B, 0x04, 0x06, 0x05, 0x03)] + assert {call.kwargs["node_id"] for call in mock_rmi.await_args_list} == {node_id} async def test_get_node_info_with_missing_properties(): - """A node without some of the properties (like the bridge, without model and article number) fails the request as a - whole, so they are read one by one, and the missing ones are None.""" + """A property that the node doesn't have, like the model and article number of the bridge, is None.""" comfoconnect = ComfoConnect("127.0.0.1", LOCAL_UUID) values = {0x14: b"My bridge\x00", 0x04: b"FAKE0000000000\x00", 0x06: bytes.fromhex("011410c0"), 0x05: b"\x01", 0x03: b"\x00"} def reply(request, node_id=None): # pylint: disable=unused-argument - if request[0] == 0x02 or request[4] not in values: + if request[4] not in values: raise ComfoConnectRmiError("RMI error") return Mock(message=values[request[4]]) - with patch.object(comfoconnect, "cmd_rmi_request", AsyncMock(side_effect=reply)) as mock_rmi: + with patch.object(comfoconnect, "cmd_rmi_request", AsyncMock(side_effect=reply)): info = await comfoconnect.get_node_info(48) assert info == NodeInfo(name="My bridge", model=None, article_number=None, serial_number="FAKE0000000000", firmware="R1.5.1", hardware_version=1, product_variant=0) - assert mock_rmi.await_count == 8 - assert {call.kwargs["node_id"] for call in mock_rmi.await_args_list} == {48} async def test_get_node_info_of_unknown_node():