Improve rainbird error handling (#98239)

homeassistant/components/rainbird/coordinator.py

@@ -8,7 +8,11 @@ import logging
 from typing import TypeVar
 
 import async_timeout
-from pyrainbird.async_client import AsyncRainbirdController, RainbirdApiException
+from pyrainbird.async_client import (
+    AsyncRainbirdController,
+    RainbirdApiException,
+    RainbirdDeviceBusyException,
+)
 from pyrainbird.data import ModelAndVersion
 
 from homeassistant.core import HomeAssistant
@@ -84,8 +88,10 @@ class RainbirdUpdateCoordinator(DataUpdateCoordinator[RainbirdDeviceState]):
         try:
             async with async_timeout.timeout(TIMEOUT_SECONDS):
                 return await self._fetch_data()
+        except RainbirdDeviceBusyException as err:
+            raise UpdateFailed("Rain Bird device is busy") from err
         except RainbirdApiException as err:
-            raise UpdateFailed(f"Error communicating with Device: {err}") from err
+            raise UpdateFailed("Rain Bird device failure") from err
 
     async def _fetch_data(self) -> RainbirdDeviceState:
         """Fetch data from the Rain Bird device.

homeassistant/components/rainbird/number.py

@@ -3,10 +3,13 @@ from __future__ import annotations
 
 import logging
 
+from pyrainbird.exceptions import RainbirdApiException, RainbirdDeviceBusyException
+
 from homeassistant.components.number import NumberEntity
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import UnitOfTime
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers.entity_platform import AddEntitiesCallback
 from homeassistant.helpers.update_coordinator import CoordinatorEntity
 
@@ -58,4 +61,11 @@ class RainDelayNumber(CoordinatorEntity[RainbirdUpdateCoordinator], NumberEntity
 
     async def async_set_native_value(self, value: float) -> None:
         """Update the current value."""
-        await self.coordinator.controller.set_rain_delay(value)
+        try:
+            await self.coordinator.controller.set_rain_delay(value)
+        except RainbirdDeviceBusyException as err:
+            raise HomeAssistantError(
+                "Rain Bird device is busy; Wait and try again"
+            ) from err
+        except RainbirdApiException as err:
+            raise HomeAssistantError("Rain Bird device failure") from err

homeassistant/components/rainbird/switch.py

@@ -3,11 +3,13 @@ from __future__ import annotations
 
 import logging
 
+from pyrainbird.exceptions import RainbirdApiException, RainbirdDeviceBusyException
 import voluptuous as vol
 
 from homeassistant.components.switch import SwitchEntity
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers import config_validation as cv, entity_platform
 from homeassistant.helpers.device_registry import DeviceInfo
 from homeassistant.helpers.entity_platform import AddEntitiesCallback
@@ -86,15 +88,30 @@ class RainBirdSwitch(CoordinatorEntity[RainbirdUpdateCoordinator], SwitchEntity)
 
     async def async_turn_on(self, **kwargs):
         """Turn the switch on."""
-        await self.coordinator.controller.irrigate_zone(
-            int(self._zone),
-            int(kwargs.get(ATTR_DURATION, self._duration_minutes)),
-        )
+        try:
+            await self.coordinator.controller.irrigate_zone(
+                int(self._zone),
+                int(kwargs.get(ATTR_DURATION, self._duration_minutes)),
+            )
+        except RainbirdDeviceBusyException as err:
+            raise HomeAssistantError(
+                "Rain Bird device is busy; Wait and try again"
+            ) from err
+        except RainbirdApiException as err:
+            raise HomeAssistantError("Rain Bird device failure") from err
+
         await self.coordinator.async_request_refresh()
 
     async def async_turn_off(self, **kwargs):
         """Turn the switch off."""
-        await self.coordinator.controller.stop_irrigation()
+        try:
+            await self.coordinator.controller.stop_irrigation()
+        except RainbirdDeviceBusyException as err:
+            raise HomeAssistantError(
+                "Rain Bird device is busy; Wait and try again"
+            ) from err
+        except RainbirdApiException as err:
+            raise HomeAssistantError("Rain Bird device failure") from err
         await self.coordinator.async_request_refresh()
 
     @property

tests/components/rainbird/conftest.py

@@ -72,11 +72,6 @@ CONFIG_ENTRY_DATA = {
 }
 
 
-UNAVAILABLE_RESPONSE = AiohttpClientMockResponse(
-    "POST", URL, status=HTTPStatus.SERVICE_UNAVAILABLE
-)
-
-
 @pytest.fixture
 def platforms() -> list[Platform]:
     """Fixture to specify platforms to test."""
@@ -150,6 +145,13 @@ def mock_response(data: str) -> AiohttpClientMockResponse:
     return AiohttpClientMockResponse("POST", URL, response=rainbird_response(data))
 
 
+def mock_response_error(
+    status: HTTPStatus = HTTPStatus.SERVICE_UNAVAILABLE,
+) -> AiohttpClientMockResponse:
+    """Create a fake AiohttpClientMockResponse."""
+    return AiohttpClientMockResponse("POST", URL, status=status)
+
+
 @pytest.fixture(name="stations_response")
 def mock_station_response() -> str:
     """Mock response to return available stations."""

tests/components/rainbird/test_init.py

@@ -2,13 +2,21 @@
 
 from __future__ import annotations
 
+from http import HTTPStatus
+
 import pytest
 
 from homeassistant.components.rainbird import DOMAIN
 from homeassistant.config_entries import ConfigEntryState
 from homeassistant.core import HomeAssistant
 
-from .conftest import CONFIG_ENTRY_DATA, UNAVAILABLE_RESPONSE, ComponentSetup
+from .conftest import (
+    CONFIG_ENTRY_DATA,
+    MODEL_AND_VERSION_RESPONSE,
+    ComponentSetup,
+    mock_response,
+    mock_response_error,
+)
 
 from tests.test_util.aiohttp import AiohttpClientMockResponse
 
@@ -44,16 +52,50 @@ async def test_init_success(
 @pytest.mark.parametrize(
     ("yaml_config", "config_entry_data", "responses", "config_entry_states"),
     [
-        ({}, CONFIG_ENTRY_DATA, [UNAVAILABLE_RESPONSE], [ConfigEntryState.SETUP_RETRY]),
+        (
+            {},
+            CONFIG_ENTRY_DATA,
+            [mock_response_error(HTTPStatus.SERVICE_UNAVAILABLE)],
+            [ConfigEntryState.SETUP_RETRY],
+        ),
+        (
+            {},
+            CONFIG_ENTRY_DATA,
+            [mock_response_error(HTTPStatus.INTERNAL_SERVER_ERROR)],
+            [ConfigEntryState.SETUP_RETRY],
+        ),
+        (
+            {},
+            CONFIG_ENTRY_DATA,
+            [
+                mock_response(MODEL_AND_VERSION_RESPONSE),
+                mock_response_error(HTTPStatus.SERVICE_UNAVAILABLE),
+            ],
+            [ConfigEntryState.SETUP_RETRY],
+        ),
+        (
+            {},
+            CONFIG_ENTRY_DATA,
+            [
+                mock_response(MODEL_AND_VERSION_RESPONSE),
+                mock_response_error(HTTPStatus.INTERNAL_SERVER_ERROR),
+            ],
+            [ConfigEntryState.SETUP_RETRY],
+        ),
+    ],
+    ids=[
+        "unavailable",
+        "server-error",
+        "coordinator-unavailable",
+        "coordinator-server-error",
     ],
-    ids=["config_entry_failure"],
 )
 async def test_communication_failure(
     hass: HomeAssistant,
     setup_integration: ComponentSetup,
     config_entry_states: list[ConfigEntryState],
 ) -> None:
-    """Test unable to talk to server on startup, which permanently fails setup."""
+    """Test unable to talk to device on startup, which fails setup."""
 
     assert await setup_integration()
 

tests/components/rainbird/test_number.py

@@ -1,5 +1,6 @@
 """Tests for rainbird number platform."""
 
+from http import HTTPStatus
 
 import pytest
 
@@ -8,6 +9,7 @@ from homeassistant.components.rainbird import DOMAIN
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import ATTR_ENTITY_ID, Platform
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers import device_registry as dr
 
 from .conftest import (
@@ -17,6 +19,7 @@ from .conftest import (
     SERIAL_NUMBER,
     ComponentSetup,
     mock_response,
+    mock_response_error,
 )
 
 from tests.test_util.aiohttp import AiohttpClientMocker
@@ -87,3 +90,40 @@ async def test_set_value(
     )
 
     assert len(aioclient_mock.mock_calls) == 1
+
+
+@pytest.mark.parametrize(
+    ("status", "expected_msg"),
+    [
+        (HTTPStatus.SERVICE_UNAVAILABLE, "Rain Bird device is busy"),
+        (HTTPStatus.INTERNAL_SERVER_ERROR, "Rain Bird device failure"),
+    ],
+)
+async def test_set_value_error(
+    hass: HomeAssistant,
+    setup_integration: ComponentSetup,
+    aioclient_mock: AiohttpClientMocker,
+    responses: list[str],
+    config_entry: ConfigEntry,
+    status: HTTPStatus,
+    expected_msg: str,
+) -> None:
+    """Test an error while talking to the device."""
+
+    assert await setup_integration()
+
+    aioclient_mock.mock_calls.clear()
+    responses.append(mock_response_error(status=status))
+
+    with pytest.raises(HomeAssistantError, match=expected_msg):
+        await hass.services.async_call(
+            number.DOMAIN,
+            number.SERVICE_SET_VALUE,
+            {
+                ATTR_ENTITY_ID: "number.rain_bird_controller_rain_delay",
+                number.ATTR_VALUE: 3,
+            },
+            blocking=True,
+        )
+
+    assert len(aioclient_mock.mock_calls) == 1

tests/components/rainbird/test_switch.py

@@ -1,11 +1,13 @@
 """Tests for rainbird sensor platform."""
 
+from http import HTTPStatus
 
 import pytest
 
 from homeassistant.components.rainbird import DOMAIN
 from homeassistant.const import ATTR_ENTITY_ID, Platform
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import HomeAssistantError
 
 from .conftest import (
     ACK_ECHO,
@@ -19,6 +21,7 @@ from .conftest import (
     ZONE_OFF_RESPONSE,
     ComponentSetup,
     mock_response,
+    mock_response_error,
 )
 
 from tests.components.switch import common as switch_common
@@ -240,3 +243,36 @@ async def test_yaml_imported_config(
     assert hass.states.get("switch.back_yard")
     assert not hass.states.get("switch.rain_bird_sprinkler_2")
     assert hass.states.get("switch.rain_bird_sprinkler_3")
+
+
+@pytest.mark.parametrize(
+    ("status", "expected_msg"),
+    [
+        (HTTPStatus.SERVICE_UNAVAILABLE, "Rain Bird device is busy"),
+        (HTTPStatus.INTERNAL_SERVER_ERROR, "Rain Bird device failure"),
+    ],
+)
+async def test_switch_error(
+    hass: HomeAssistant,
+    setup_integration: ComponentSetup,
+    aioclient_mock: AiohttpClientMocker,
+    responses: list[AiohttpClientMockResponse],
+    status: HTTPStatus,
+    expected_msg: str,
+) -> None:
+    """Test an error talking to the device."""
+
+    assert await setup_integration()
+
+    aioclient_mock.mock_calls.clear()
+    responses.append(mock_response_error(status=status))
+
+    with pytest.raises(HomeAssistantError, match=expected_msg):
+        await switch_common.async_turn_on(hass, "switch.rain_bird_sprinkler_3")
+        await hass.async_block_till_done()
+
+    responses.append(mock_response_error(status=status))
+
+    with pytest.raises(HomeAssistantError, match=expected_msg):
+        await switch_common.async_turn_off(hass, "switch.rain_bird_sprinkler_3")
+        await hass.async_block_till_done()