Improve Roborock error handling (#124267)

homeassistant/components/roborock/number.py

@@ -13,9 +13,10 @@ from roborock.version_1_apis.roborock_client_v1 import AttributeCache
 from homeassistant.components.number import NumberEntity, NumberEntityDescription
 from homeassistant.const import PERCENTAGE, EntityCategory
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers.entity_platform import AddEntitiesCallback
 
-from . import RoborockConfigEntry
+from . import DOMAIN, RoborockConfigEntry
 from .coordinator import RoborockDataUpdateCoordinator
 from .entity import RoborockEntityV1
 
@@ -107,6 +108,12 @@ class RoborockNumberEntity(RoborockEntityV1, NumberEntity):
 
     async def async_set_native_value(self, value: float) -> None:
         """Set number value."""
-        await self.entity_description.update_value(
+        try:
-            self.get_cache(self.entity_description.cache_key), value
+            await self.entity_description.update_value(
-        )
+                self.get_cache(self.entity_description.cache_key), value
+            )
+        except RoborockException as err:
+            raise HomeAssistantError(
+                translation_domain=DOMAIN,
+                translation_key="update_options_failed",
+            ) from err

homeassistant/components/roborock/strings.json

@@ -419,6 +419,9 @@
     },
     "no_coordinators": {
       "message": "No devices were able to successfully setup"
+    },
+    "update_options_failed": {
+      "message": "Failed to update Roborock options"
     }
   },
   "services": {

homeassistant/components/roborock/switch.py

@@ -9,14 +9,16 @@ import logging
 from typing import Any
 
 from roborock.command_cache import CacheableAttribute
+from roborock.exceptions import RoborockException
 from roborock.version_1_apis.roborock_client_v1 import AttributeCache
 
 from homeassistant.components.switch import SwitchEntity, SwitchEntityDescription
 from homeassistant.const import EntityCategory
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers.entity_platform import AddEntitiesCallback
 
-from . import RoborockConfigEntry
+from . import DOMAIN, RoborockConfigEntry
 from .coordinator import RoborockDataUpdateCoordinator
 from .entity import RoborockEntityV1
 
@@ -149,15 +151,27 @@ class RoborockSwitch(RoborockEntityV1, SwitchEntity):
 
     async def async_turn_off(self, **kwargs: Any) -> None:
         """Turn off the switch."""
-        await self.entity_description.update_value(
+        try:
-            self.get_cache(self.entity_description.cache_key), False
+            await self.entity_description.update_value(
-        )
+                self.get_cache(self.entity_description.cache_key), False
+            )
+        except RoborockException as err:
+            raise HomeAssistantError(
+                translation_domain=DOMAIN,
+                translation_key="update_options_failed",
+            ) from err
 
     async def async_turn_on(self, **kwargs: Any) -> None:
         """Turn on the switch."""
-        await self.entity_description.update_value(
+        try:
-            self.get_cache(self.entity_description.cache_key), True
+            await self.entity_description.update_value(
-        )
+                self.get_cache(self.entity_description.cache_key), True
+            )
+        except RoborockException as err:
+            raise HomeAssistantError(
+                translation_domain=DOMAIN,
+                translation_key="update_options_failed",
+            ) from err
 
     @property
     def is_on(self) -> bool | None:

homeassistant/components/roborock/time.py

@@ -15,9 +15,10 @@ from roborock.version_1_apis.roborock_client_v1 import AttributeCache
 from homeassistant.components.time import TimeEntity, TimeEntityDescription
 from homeassistant.const import EntityCategory
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers.entity_platform import AddEntitiesCallback
 
-from . import RoborockConfigEntry
+from . import DOMAIN, RoborockConfigEntry
 from .coordinator import RoborockDataUpdateCoordinator
 from .entity import RoborockEntityV1
 
@@ -172,6 +173,12 @@ class RoborockTimeEntity(RoborockEntityV1, TimeEntity):
 
     async def async_set_value(self, value: time) -> None:
         """Set the time."""
-        await self.entity_description.update_value(
+        try:
-            self.get_cache(self.entity_description.cache_key), value
+            await self.entity_description.update_value(
-        )
+                self.get_cache(self.entity_description.cache_key), value
+            )
+        except RoborockException as err:
+            raise HomeAssistantError(
+                translation_domain=DOMAIN,
+                translation_key="update_options_failed",
+            ) from err

tests/components/roborock/test_button.py

@@ -3,9 +3,11 @@
 from unittest.mock import patch
 
 import pytest
+import roborock
 
 from homeassistant.components.button import SERVICE_PRESS
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import HomeAssistantError
 
 from tests.common import MockConfigEntry
 
@@ -16,7 +18,7 @@ from tests.common import MockConfigEntry
         ("button.roborock_s7_maxv_reset_sensor_consumable"),
         ("button.roborock_s7_maxv_reset_air_filter_consumable"),
         ("button.roborock_s7_maxv_reset_side_brush_consumable"),
-        "button.roborock_s7_maxv_reset_main_brush_consumable",
+        ("button.roborock_s7_maxv_reset_main_brush_consumable"),
     ],
 )
 @pytest.mark.freeze_time("2023-10-30 08:50:00")
@@ -41,3 +43,37 @@ async def test_update_success(
         )
     assert mock_send_message.assert_called_once
     assert hass.states.get(entity_id).state == "2023-10-30T08:50:00+00:00"
+
+
+@pytest.mark.parametrize(
+    ("entity_id"),
+    [
+        ("button.roborock_s7_maxv_reset_air_filter_consumable"),
+    ],
+)
+@pytest.mark.freeze_time("2023-10-30 08:50:00")
+@pytest.mark.usefixtures("entity_registry_enabled_by_default")
+async def test_update_failure(
+    hass: HomeAssistant,
+    bypass_api_fixture,
+    setup_entry: MockConfigEntry,
+    entity_id: str,
+) -> None:
+    """Test failure while pressing the button entity."""
+    # Ensure that the entity exist, as these test can pass even if there is no entity.
+    assert hass.states.get(entity_id).state == "unknown"
+    with (
+        patch(
+            "homeassistant.components.roborock.coordinator.RoborockLocalClientV1.send_message",
+            side_effect=roborock.exceptions.RoborockTimeout,
+        ) as mock_send_message,
+        pytest.raises(HomeAssistantError, match="Error while calling RESET_CONSUMABLE"),
+    ):
+        await hass.services.async_call(
+            "button",
+            SERVICE_PRESS,
+            blocking=True,
+            target={"entity_id": entity_id},
+        )
+    assert mock_send_message.assert_called_once
+    assert hass.states.get(entity_id).state == "2023-10-30T08:50:00+00:00"

tests/components/roborock/test_number.py

@@ -3,9 +3,11 @@
 from unittest.mock import patch
 
 import pytest
+import roborock
 
 from homeassistant.components.number import ATTR_VALUE, SERVICE_SET_VALUE
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import HomeAssistantError
 
 from tests.common import MockConfigEntry
 
@@ -37,3 +39,36 @@ async def test_update_success(
             target={"entity_id": entity_id},
         )
     assert mock_send_message.assert_called_once
+
+
+@pytest.mark.parametrize(
+    ("entity_id", "value"),
+    [
+        ("number.roborock_s7_maxv_volume", 3.0),
+    ],
+)
+async def test_update_failed(
+    hass: HomeAssistant,
+    bypass_api_fixture,
+    setup_entry: MockConfigEntry,
+    entity_id: str,
+    value: float,
+) -> None:
+    """Test allowed changing values for number entities."""
+    # Ensure that the entity exist, as these test can pass even if there is no entity.
+    assert hass.states.get(entity_id) is not None
+    with (
+        patch(
+            "homeassistant.components.roborock.coordinator.RoborockLocalClientV1.send_message",
+            side_effect=roborock.exceptions.RoborockTimeout,
+        ) as mock_send_message,
+        pytest.raises(HomeAssistantError, match="Failed to update Roborock options"),
+    ):
+        await hass.services.async_call(
+            "number",
+            SERVICE_SET_VALUE,
+            service_data={ATTR_VALUE: value},
+            blocking=True,
+            target={"entity_id": entity_id},
+        )
+    assert mock_send_message.assert_called_once

tests/components/roborock/test_select.py

@@ -59,7 +59,7 @@ async def test_update_failure(
             "homeassistant.components.roborock.coordinator.RoborockLocalClientV1.send_message",
             side_effect=RoborockException(),
         ),
-        pytest.raises(HomeAssistantError),
+        pytest.raises(HomeAssistantError, match="Error while calling SET_MOP_MOD"),
     ):
         await hass.services.async_call(
             "select",

tests/components/roborock/test_switch.py

@@ -3,9 +3,11 @@
 from unittest.mock import patch
 
 import pytest
+import roborock
 
 from homeassistant.components.switch import SERVICE_TURN_OFF, SERVICE_TURN_ON
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import HomeAssistantError
 
 from tests.common import MockConfigEntry
 
@@ -49,3 +51,37 @@ async def test_update_success(
             target={"entity_id": entity_id},
         )
     assert mock_send_message.assert_called_once
+
+
+@pytest.mark.parametrize(
+    ("entity_id", "service"),
+    [
+        ("switch.roborock_s7_maxv_status_indicator_light", SERVICE_TURN_ON),
+        ("switch.roborock_s7_maxv_status_indicator_light", SERVICE_TURN_OFF),
+    ],
+)
+async def test_update_failed(
+    hass: HomeAssistant,
+    bypass_api_fixture,
+    setup_entry: MockConfigEntry,
+    entity_id: str,
+    service: str,
+) -> None:
+    """Test a failure while updating a switch."""
+    # Ensure that the entity exist, as these test can pass even if there is no entity.
+    assert hass.states.get(entity_id) is not None
+    with (
+        patch(
+            "homeassistant.components.roborock.coordinator.RoborockLocalClientV1._send_command",
+            side_effect=roborock.exceptions.RoborockTimeout,
+        ) as mock_send_message,
+        pytest.raises(HomeAssistantError, match="Failed to update Roborock options"),
+    ):
+        await hass.services.async_call(
+            "switch",
+            service,
+            service_data=None,
+            blocking=True,
+            target={"entity_id": entity_id},
+        )
+    assert mock_send_message.assert_called_once

tests/components/roborock/test_time.py

@@ -4,9 +4,11 @@ from datetime import time
 from unittest.mock import patch
 
 import pytest
+import roborock
 
 from homeassistant.components.time import SERVICE_SET_VALUE
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import HomeAssistantError
 
 from tests.common import MockConfigEntry
 
@@ -38,3 +40,35 @@ async def test_update_success(
             target={"entity_id": entity_id},
         )
     assert mock_send_message.assert_called_once
+
+
+@pytest.mark.parametrize(
+    ("entity_id"),
+    [
+        ("time.roborock_s7_maxv_do_not_disturb_begin"),
+    ],
+)
+async def test_update_failure(
+    hass: HomeAssistant,
+    bypass_api_fixture,
+    setup_entry: MockConfigEntry,
+    entity_id: str,
+) -> None:
+    """Test turning switch entities on and off."""
+    # Ensure that the entity exist, as these test can pass even if there is no entity.
+    assert hass.states.get(entity_id) is not None
+    with (
+        patch(
+            "homeassistant.components.roborock.coordinator.RoborockLocalClientV1._send_command",
+            side_effect=roborock.exceptions.RoborockTimeout,
+        ) as mock_send_message,
+        pytest.raises(HomeAssistantError, match="Failed to update Roborock options"),
+    ):
+        await hass.services.async_call(
+            "time",
+            SERVICE_SET_VALUE,
+            service_data={"time": time(hour=1, minute=1)},
+            blocking=True,
+            target={"entity_id": entity_id},
+        )
+    assert mock_send_message.assert_called_once