Handle CB3 not sending the correct add_neighbor response (#247)

* Handle CB3 not sending the correct response

* Fix broken unit test

* Fix remaining broken unit tests

Author: puddly

tests/test_api.py

@@ -755,7 +755,7 @@ async def test_bad_command_parsing(api, caplog):
 
     assert 0xFF not in deconz_api.COMMAND_SCHEMAS
 
-    with caplog.at_level(logging.WARNING):
+    with caplog.at_level(logging.DEBUG):
         api.data_received(
             bytes.fromhex(
                 "172c002f0028002e02000000020000000000"
@@ -961,6 +961,65 @@ async def test_add_neighbour(api, mock_command_rsp):
     ]
 
 
+async def test_add_neighbour_conbee3_success(api):
+    api._command = AsyncMock(wraps=api._command)
+    api._uart = AsyncMock()
+
+    # Simulate a good but invalid response from the Conbee III
+    asyncio.get_running_loop().call_later(
+        0.001,
+        lambda: api.data_received(
+            b"\x1d" + bytes([api._seq - 1]) + b"\x00\x06\x00\x01"
+        ),
+    )
+
+    await api.add_neighbour(
+        nwk=0x1234,
+        ieee=t.EUI64.convert("aa:bb:cc:dd:11:22:33:44"),
+        mac_capability_flags=0x12,
+    )
+
+    assert api._command.mock_calls == [
+        call(
+            deconz_api.CommandId.update_neighbor,
+            action=deconz_api.UpdateNeighborAction.ADD,
+            nwk=0x1234,
+            ieee=t.EUI64.convert("aa:bb:cc:dd:11:22:33:44"),
+            mac_capability_flags=0x12,
+        )
+    ]
+
+
+async def test_add_neighbour_conbee3_failure(api):
+    api._command = AsyncMock(wraps=api._command)
+    api._uart = AsyncMock()
+
+    # Simulate a bad response from the Conbee III
+    asyncio.get_running_loop().call_later(
+        0.001,
+        lambda: api.data_received(
+            b"\x1d" + bytes([api._seq - 1]) + b"\x01\x06\x00\x01"
+        ),
+    )
+
+    with pytest.raises(deconz_api.CommandError):
+        await api.add_neighbour(
+            nwk=0x1234,
+            ieee=t.EUI64.convert("aa:bb:cc:dd:11:22:33:44"),
+            mac_capability_flags=0x12,
+        )
+
+    assert api._command.mock_calls == [
+        call(
+            deconz_api.CommandId.update_neighbor,
+            action=deconz_api.UpdateNeighborAction.ADD,
+            nwk=0x1234,
+            ieee=t.EUI64.convert("aa:bb:cc:dd:11:22:33:44"),
+            mac_capability_flags=0x12,
+        )
+    ]
+
+
 async def test_cb3_device_state_callback_bug(api, mock_command_rsp):
     mock_command_rsp(
         command_id=deconz_api.CommandId.version,

tests/test_application.py

@@ -216,7 +216,7 @@ async def test_disconnect_no_api(app):
 
 async def test_disconnect_close_error(app):
     app._api.write_parameter = MagicMock(
-        side_effect=zigpy_deconz.exception.CommandError(1, "Error")
+        side_effect=zigpy_deconz.exception.CommandError("Error", status=1, command=None)
     )
     await app.disconnect()
 
@@ -399,13 +399,17 @@ def mock_add_neighbour(nwk, ieee, mac_capability_flags):
 
         if max_neighbors < 0:
             raise zigpy_deconz.exception.CommandError(
-                deconz_api.Status.FAILURE, "Failure"
+                "Failure",
+                status=deconz_api.Status.FAILURE,
+                command=None,
             )
 
     p = patch.object(app, "_api", spec_set=zigpy_deconz.api.Deconz(None, None))
 
     with p as api_mock:
-        err = zigpy_deconz.exception.CommandError(deconz_api.Status.FAILURE, "Failure")
+        err = zigpy_deconz.exception.CommandError(
+            "Failure", status=deconz_api.Status.FAILURE, command=None
+        )
         api_mock.add_neighbour = AsyncMock(side_effect=[None, err, err, err])
 
         with caplog.at_level(logging.DEBUG):
@@ -483,7 +487,9 @@ async def read_param(param_id, index):
 
         if index not in slots:
             raise zigpy_deconz.exception.CommandError(
-                deconz_api.Status.UNSUPPORTED, "Unsupported"
+                "Unsupported",
+                status=deconz_api.Status.UNSUPPORTED,
+                command=None,
             )
         else:
             return deconz_api.IndexedEndpoint(index=index, descriptor=slots[index])
@@ -504,7 +510,9 @@ async def read_param(param_id, index):
         assert index in (0x00, 0x01)
 
         raise zigpy_deconz.exception.CommandError(
-            deconz_api.Status.UNSUPPORTED, "Unsupported"
+            "Unsupported",
+            status=deconz_api.Status.UNSUPPORTED,
+            command=None,
         )
 
     app._api.read_parameter = AsyncMock(side_effect=read_param)
@@ -524,7 +532,9 @@ async def read_param(param_id, index):
 
         if index > 0x01:
             raise zigpy_deconz.exception.CommandError(
-                deconz_api.Status.UNSUPPORTED, "Unsupported"
+                "Unsupported",
+                status=deconz_api.Status.UNSUPPORTED,
+                command=None,
             )
 
         return deconz_api.IndexedEndpoint(

tests/test_exception.py

@@ -7,6 +7,9 @@
 
 def test_command_error():
     ex = zigpy_deconz.exception.CommandError(
-        mock.sentinel.status, mock.sentinel.message
+        mock.sentinel.message,
+        status=mock.sentinel.status,
+        command=mock.sentinel.command,
     )
     assert ex.status is mock.sentinel.status
+    assert ex.command is mock.sentinel.command

tests/test_network_state.py

@@ -123,7 +123,9 @@ async def write_parameter(param, *args):
             and param == zigpy_deconz.api.NetworkParameter.nwk_frame_counter
         ):
             raise zigpy_deconz.exception.CommandError(
-                status=zigpy_deconz.api.Status.UNSUPPORTED
+                "Command is unsupported",
+                status=zigpy_deconz.api.Status.UNSUPPORTED,
+                command=None,
             )
 
         params[param.name] = args
@@ -212,7 +214,9 @@ async def write_parameter(param, *args):
             None,
             {
                 ("nwk_frame_counter",): zigpy_deconz.exception.CommandError(
-                    zigpy_deconz.api.Status.UNSUPPORTED
+                    "Some error",
+                    status=zigpy_deconz.api.Status.UNSUPPORTED,
+                    command=None,
                 )
             },
             {"network_key.tx_counter": 0},

tests/test_send_receive.py

@@ -8,7 +8,7 @@
 import zigpy.exceptions
 import zigpy.types as zigpy_t
 
-from zigpy_deconz.api import TXStatus
+from zigpy_deconz.api import Status, TXStatus
 import zigpy_deconz.exception
 import zigpy_deconz.types as t
 
@@ -23,7 +23,9 @@ async def mock_send(req_id, *args, **kwargs):
             await asyncio.sleep(0)
 
             if fail_enqueue:
-                raise zigpy_deconz.exception.CommandError("Error")
+                raise zigpy_deconz.exception.CommandError(
+                    "Error", status=Status.FAILURE, command=None
+                )
 
             if fail_deliver:
                 app.handle_tx_confirm(

zigpy_deconz/api.py

@@ -27,7 +27,7 @@
 )
 from zigpy.zdo.types import SimpleDescriptor
 
-from zigpy_deconz.exception import APIException, CommandError, MismatchedResponseError
+from zigpy_deconz.exception import CommandError, MismatchedResponseError, ParsingError
 import zigpy_deconz.types as t
 import zigpy_deconz.uart
 from zigpy_deconz.utils import restart_forever
@@ -568,7 +568,11 @@ async def _command(self, cmd, **kwargs):
 
         if self._uart is None:
             # connection was lost
-            raise CommandError(Status.ERROR, "API is not running")
+            raise CommandError(
+                "API is not running",
+                status=Status.ERROR,
+                command=command,
+            )
 
         async with self._command_lock(priority=self._get_command_priority(command)):
             seq = self._seq
@@ -616,11 +620,15 @@ def data_received(self, data: bytes) -> None:
         try:
             params, rest = t.deserialize_dict(command.payload, rx_schema)
         except Exception:
-            LOGGER.warning("Failed to parse command %s", command, exc_info=True)
+            LOGGER.debug("Failed to parse command %s", command, exc_info=True)
 
             if fut is not None and not fut.done():
                 fut.set_exception(
-                    APIException(f"Failed to deserialize command: {command}")
+                    ParsingError(
+                        f"Failed to parse command: {command}",
+                        status=Status.ERROR,
+                        command=command,
+                    )
                 )
 
             return
@@ -677,7 +685,11 @@ def data_received(self, data: bytes) -> None:
             # Make sure we do not resolve the future
             fut = None
         elif status != Status.SUCCESS:
-            exc = CommandError(status, f"{command.command_id}, status: {status}")
+            exc = CommandError(
+                f"{command.command_id}, status: {status}",
+                status=status,
+                command=command,
+            )
 
         if fut is not None:
             try:
@@ -905,10 +917,21 @@ async def change_network_state(self, new_state: NetworkState) -> None:
     async def add_neighbour(
         self, nwk: t.NWK, ieee: t.EUI64, mac_capability_flags: t.uint8_t
     ) -> None:
-        await self.send_command(
-            CommandId.update_neighbor,
-            action=UpdateNeighborAction.ADD,
-            nwk=nwk,
-            ieee=ieee,
-            mac_capability_flags=mac_capability_flags,
-        )
+        try:
+            await self.send_command(
+                CommandId.update_neighbor,
+                action=UpdateNeighborAction.ADD,
+                nwk=nwk,
+                ieee=ieee,
+                mac_capability_flags=mac_capability_flags,
+            )
+        except ParsingError as exc:
+            # Older Conbee III firmwares send back an invalid response
+            status = Status(exc.command.payload[0])
+
+            if status != Status.SUCCESS:
+                raise CommandError(
+                    f"{exc.command.command_id}, status: {status}",
+                    status=status,
+                    command=exc.command,
+                ) from exc

zigpy_deconz/exception.py

@@ -7,18 +7,19 @@
 from zigpy.exceptions import APIException
 
 if typing.TYPE_CHECKING:
-    from zigpy_deconz.api import CommandId
+    from zigpy_deconz.api import Command, CommandId, Status
 
 
 class CommandError(APIException):
-    def __init__(self, status=1, *args, **kwargs):
+    def __init__(self, *args, status: Status, command: Command, **kwargs):
         """Initialize instance."""
-        self._status = status
         super().__init__(*args, **kwargs)
+        self.command = command
+        self.status = status
 
-    @property
-    def status(self):
-        return self._status
+
+class ParsingError(CommandError):
+    pass
 
 
 class MismatchedResponseError(APIException):