refactor: inject logger into dps_listeners, improve callback exception handling.

This also renames discover_features to start in v1 properties.

Author: allenporter

roborock/callbacks.py

@@ -26,7 +26,7 @@ def wrapper(value: V) -> None:
         try:
             callback(value)
         except Exception as ex:  # noqa: BLE001
-            logger.error("Uncaught error in callback '%s': %s", callback.__name__, ex)
+            logger.error("Uncaught error in callback '%s': %s", getattr(callback, "__name__", "Unknown"), ex)
 
     return wrapper
 

roborock/cli.py

@@ -419,7 +419,7 @@ async def _v1_trait(context: RoborockContext, device_id: str, display_func: Call
     device = await device_manager.get_device(device_id)
     if device.v1_properties is None:
         raise RoborockUnsupportedFeature(f"Device {device.name} does not support V1 protocol")
-    await device.v1_properties.discover_features()
+    await device.v1_properties.start()
     trait = display_func(device.v1_properties)
     if trait is None:
         raise RoborockUnsupportedFeature("Trait not supported by device")

roborock/devices/rpc/v1_channel.py

@@ -190,7 +190,7 @@ def __init__(
         self._device_cache = device_cache
         self._reconnect_task: asyncio.Task[None] | None = None
         self._last_network_info_refresh: datetime.datetime | None = None
-        self._dps_listeners = CallbackList[dict[RoborockDataProtocol, Any]]()
+        self._dps_listeners = CallbackList[dict[RoborockDataProtocol, Any]](self._logger)
 
     @property
     def is_connected(self) -> bool:
@@ -452,10 +452,7 @@ def _on_mqtt_message(self, message: RoborockMessage) -> None:
             return
 
         if datapoints:
-            try:
-                self._dps_listeners(datapoints)
-            except Exception:
-                self._logger.exception("Error in DPS listener callback")
+            self._dps_listeners(datapoints)
 
     def _on_local_message(self, message: RoborockMessage) -> None:
         """Handle incoming local messages."""

roborock/devices/traits/v1/__init__.py

@@ -248,7 +248,9 @@ def close(self) -> None:
     def _on_dps_update(self, dps: dict[RoborockDataProtocol, Any]) -> None:
         """Handle incoming messages from the device.
 
-        This will notify all traits of the new values.
+        This will notify all traits of the new values. This can be improved in
+        the future to be dynamic when we have more traits that support dynamic
+        updates but for now we just invoke them manually.
         """
         _LOGGER.debug("Received message from device: %s", dps)
         self.status.update_from_dps(dps)

tests/devices/rpc/test_v1_channel.py

@@ -612,3 +612,33 @@ async def test_v1_channel_add_dps_listener(
     dps_listener.reset_mock()
     v1_channel._on_mqtt_message(push_message)
     dps_listener.assert_not_called()
+
+
+async def test_v1_channel_dps_listener_raises_exception(
+    v1_channel: V1Channel,
+    mock_mqtt_channel: FakeChannel,
+) -> None:
+    """Test that DPS listener that raises exceptions is ignored."""
+    mock_mqtt_channel.response_queue.append(TEST_NETWORK_INFO_RESPONSE)
+    await v1_channel.subscribe(Mock())
+
+    # Create a mock listener for DPS updates
+    dps_listener1 = Mock()
+    dps_listener1.side_effect = Exception("DPS listener failed")
+    dps_listener2 = Mock()
+    unsub_dps1 = v1_channel.add_dps_listener(dps_listener1)
+    unsub_dps2 = v1_channel.add_dps_listener(dps_listener2)
+
+    # Simulate an incoming MQTT message with data protocol payload.
+    dps_payload = json.dumps({"dps": {"121": 5}}).encode()
+    push_message = RoborockMessage(
+        protocol=RoborockMessageProtocol.GENERAL_REQUEST,
+        payload=dps_payload,
+    )
+    mock_mqtt_channel.notify_subscribers(push_message)
+
+    dps_listener1.assert_called_once()
+    dps_listener2.assert_called_once()
+
+    unsub_dps1()
+    unsub_dps2()