Read the node info one property at a time

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:

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():