Refactor Plugwise command handling (#66202)

2022-02-10 09:53:26 +01:00 · 2022-02-10 09:53:26 +01:00 · b3814aa4e6
commit b3814aa4e6
parent 678e56b8b7
5 changed files with 178 additions and 148 deletions
--- a/homeassistant/components/plugwise/climate.py
+++ b/homeassistant/components/plugwise/climate.py
@ -1,8 +1,6 @@
 """Plugwise Climate component for Home Assistant."""
 from typing import Any

-from plugwise.exceptions import PlugwiseException
-
 from homeassistant.components.climate import ClimateEntity
 from homeassistant.components.climate.const import (
    CURRENT_HVAC_COOL,
@ -20,16 +18,10 @@ from homeassistant.const import ATTR_TEMPERATURE, TEMP_CELSIUS
 from homeassistant.core import HomeAssistant, callback
 from homeassistant.helpers.entity_platform import AddEntitiesCallback

-from .const import (
-    DEFAULT_MAX_TEMP,
-    DEFAULT_MIN_TEMP,
-    DOMAIN,
-    LOGGER,
-    SCHEDULE_OFF,
-    SCHEDULE_ON,
-)
+from .const import DEFAULT_MAX_TEMP, DEFAULT_MIN_TEMP, DOMAIN, SCHEDULE_OFF, SCHEDULE_ON
 from .coordinator import PlugwiseDataUpdateCoordinator
 from .entity import PlugwiseEntity
+from .util import plugwise_command

 HVAC_MODES_HEAT_ONLY = [HVAC_MODE_HEAT, HVAC_MODE_AUTO, HVAC_MODE_OFF]
 HVAC_MODES_HEAT_COOL = [HVAC_MODE_HEAT, HVAC_MODE_COOL, HVAC_MODE_AUTO, HVAC_MODE_OFF]
@ -76,21 +68,16 @@ class PlugwiseClimateEntity(PlugwiseEntity, ClimateEntity):

        self._loc_id = coordinator.data.devices[device_id]["location"]

+    @plugwise_command
    async def async_set_temperature(self, **kwargs: Any) -> None:
        """Set new target temperature."""
-        temperature = kwargs.get(ATTR_TEMPERATURE)
-        if (temperature is not None) and (
-            self._attr_min_temp < temperature < self._attr_max_temp
+        if ((temperature := kwargs.get(ATTR_TEMPERATURE)) is None) or (
+            self._attr_max_temp < temperature < self._attr_min_temp
        ):
-            try:
-                await self.coordinator.api.set_temperature(self._loc_id, temperature)
-                self._attr_target_temperature = temperature
-                self.async_write_ha_state()
-            except PlugwiseException:
-                LOGGER.error("Error while communicating to device")
-        else:
-            LOGGER.error("Invalid temperature requested")
+            raise ValueError("Invalid temperature requested")
+        await self.coordinator.api.set_temperature(self._loc_id, temperature)

+    @plugwise_command
    async def async_set_hvac_mode(self, hvac_mode: str) -> None:
        """Set the hvac mode."""
        state = SCHEDULE_OFF
@ -98,35 +85,22 @@ class PlugwiseClimateEntity(PlugwiseEntity, ClimateEntity):

        if hvac_mode == HVAC_MODE_AUTO:
            state = SCHEDULE_ON
-            try:
-                await self.coordinator.api.set_temperature(
-                    self._loc_id, climate_data.get("schedule_temperature")
-                )
-                self._attr_target_temperature = climate_data.get("schedule_temperature")
-            except PlugwiseException:
-                LOGGER.error("Error while communicating to device")
-
-        try:
-            await self.coordinator.api.set_schedule_state(
-                self._loc_id, climate_data.get("last_used"), state
+            await self.coordinator.api.set_temperature(
+                self._loc_id, climate_data.get("schedule_temperature")
            )
-            self._attr_hvac_mode = hvac_mode
-            self.async_write_ha_state()
-        except PlugwiseException:
-            LOGGER.error("Error while communicating to device")
+            self._attr_target_temperature = climate_data.get("schedule_temperature")

+        await self.coordinator.api.set_schedule_state(
+            self._loc_id, climate_data.get("last_used"), state
+        )
+
+    @plugwise_command
    async def async_set_preset_mode(self, preset_mode: str) -> None:
        """Set the preset mode."""
-        if not (presets := self.coordinator.data.devices[self._dev_id].get("presets")):
+        if not self.coordinator.data.devices[self._dev_id].get("presets"):
            raise ValueError("No presets available")

-        try:
-            await self.coordinator.api.set_preset(self._loc_id, preset_mode)
-            self._attr_preset_mode = preset_mode
-            self._attr_target_temperature = presets.get(preset_mode, "none")[0]
-            self.async_write_ha_state()
-        except PlugwiseException:
-            LOGGER.error("Error while communicating to device")
+        await self.coordinator.api.set_preset(self._loc_id, preset_mode)

    @callback
    def _handle_coordinator_update(self) -> None:
--- a/homeassistant/components/plugwise/switch.py
+++ b/homeassistant/components/plugwise/switch.py
@ -3,8 +3,6 @@ from __future__ import annotations

 from typing import Any

-from plugwise.exceptions import PlugwiseException
-
 from homeassistant.components.switch import SwitchEntity, SwitchEntityDescription
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.core import HomeAssistant, callback
@ -13,6 +11,7 @@ from homeassistant.helpers.entity_platform import AddEntitiesCallback
 from .const import DOMAIN, LOGGER
 from .coordinator import PlugwiseDataUpdateCoordinator
 from .entity import PlugwiseEntity
+from .util import plugwise_command

 SWITCHES: tuple[SwitchEntityDescription, ...] = (
    SwitchEntityDescription(
@ -54,37 +53,25 @@ class PlugwiseSwitchEntity(PlugwiseEntity, SwitchEntity):
        self._attr_unique_id = f"{device_id}-{description.key}"
        self._attr_name = coordinator.data.devices[device_id].get("name")

+    @plugwise_command
    async def async_turn_on(self, **kwargs: Any) -> None:
        """Turn the device on."""
-        try:
-            state_on = await self.coordinator.api.set_switch_state(
-                self._dev_id,
-                self.coordinator.data.devices[self._dev_id].get("members"),
-                self.entity_description.key,
-                "on",
-            )
-        except PlugwiseException:
-            LOGGER.error("Error while communicating to device")
-        else:
-            if state_on:
-                self._attr_is_on = True
-                self.async_write_ha_state()
+        await self.coordinator.api.set_switch_state(
+            self._dev_id,
+            self.coordinator.data.devices[self._dev_id].get("members"),
+            self.entity_description.key,
+            "on",
+        )

+    @plugwise_command
    async def async_turn_off(self, **kwargs: Any) -> None:
        """Turn the device off."""
-        try:
-            state_off = await self.coordinator.api.set_switch_state(
-                self._dev_id,
-                self.coordinator.data.devices[self._dev_id].get("members"),
-                self.entity_description.key,
-                "off",
-            )
-        except PlugwiseException:
-            LOGGER.error("Error while communicating to device")
-        else:
-            if state_off:
-                self._attr_is_on = False
-                self.async_write_ha_state()
+        await self.coordinator.api.set_switch_state(
+            self._dev_id,
+            self.coordinator.data.devices[self._dev_id].get("members"),
+            self.entity_description.key,
+            "off",
+        )

    @callback
    def _handle_coordinator_update(self) -> None:
--- a/homeassistant/components/plugwise/util.py
+++ b/homeassistant/components/plugwise/util.py
@ -0,0 +1,36 @@
+"""Utilities for Plugwise."""
+from collections.abc import Awaitable, Callable, Coroutine
+from typing import Any, TypeVar
+
+from plugwise.exceptions import PlugwiseException
+from typing_extensions import Concatenate, ParamSpec
+
+from homeassistant.exceptions import HomeAssistantError
+
+from .entity import PlugwiseEntity
+
+_P = ParamSpec("_P")
+_R = TypeVar("_R")
+_T = TypeVar("_T", bound=PlugwiseEntity)
+
+
+def plugwise_command(
+    func: Callable[Concatenate[_T, _P], Awaitable[_R]]  # type: ignore[misc]
+) -> Callable[Concatenate[_T, _P], Coroutine[Any, Any, _R]]:  # type: ignore[misc]
+    """Decorate Plugwise calls that send commands/make changes to the device.
+
+    A decorator that wraps the passed in function, catches Plugwise errors,
+    and requests an coordinator update to update status of the devices asap.
+    """
+
+    async def handler(self: _T, *args: _P.args, **kwargs: _P.kwargs) -> _R:
+        try:
+            return await func(self, *args, **kwargs)
+        except PlugwiseException as error:
+            raise HomeAssistantError(
+                f"Error communicating with API: {error}"
+            ) from error
+        finally:
+            await self.coordinator.async_request_refresh()
+
+    return handler
--- a/tests/components/plugwise/test_climate.py
+++ b/tests/components/plugwise/test_climate.py
@ -1,6 +1,7 @@
 """Tests for the Plugwise Climate integration."""

 from plugwise.exceptions import PlugwiseException
+import pytest

 from homeassistant.components.climate.const import (
    HVAC_MODE_AUTO,
@ -8,6 +9,7 @@ from homeassistant.components.climate.const import (
    HVAC_MODE_OFF,
 )
 from homeassistant.config_entries import ConfigEntryState
+from homeassistant.exceptions import HomeAssistantError

 from tests.components.plugwise.common import async_init_integration

@ -56,32 +58,38 @@ async def test_adam_climate_adjust_negative_testing(hass, mock_smile_adam):
    entry = await async_init_integration(hass, mock_smile_adam)
    assert entry.state is ConfigEntryState.LOADED

-    await hass.services.async_call(
-        "climate",
-        "set_temperature",
-        {"entity_id": "climate.zone_lisa_wk", "temperature": 25},
-        blocking=True,
-    )
+    with pytest.raises(HomeAssistantError):
+        await hass.services.async_call(
+            "climate",
+            "set_temperature",
+            {"entity_id": "climate.zone_lisa_wk", "temperature": 25},
+            blocking=True,
+        )
    state = hass.states.get("climate.zone_lisa_wk")
    attrs = state.attributes
    assert attrs["temperature"] == 21.5

-    await hass.services.async_call(
-        "climate",
-        "set_preset_mode",
-        {"entity_id": "climate.zone_thermostat_jessie", "preset_mode": "home"},
-        blocking=True,
-    )
+    with pytest.raises(HomeAssistantError):
+        await hass.services.async_call(
+            "climate",
+            "set_preset_mode",
+            {"entity_id": "climate.zone_thermostat_jessie", "preset_mode": "home"},
+            blocking=True,
+        )
    state = hass.states.get("climate.zone_thermostat_jessie")
    attrs = state.attributes
    assert attrs["preset_mode"] == "asleep"

-    await hass.services.async_call(
-        "climate",
-        "set_hvac_mode",
-        {"entity_id": "climate.zone_thermostat_jessie", "hvac_mode": HVAC_MODE_AUTO},
-        blocking=True,
-    )
+    with pytest.raises(HomeAssistantError):
+        await hass.services.async_call(
+            "climate",
+            "set_hvac_mode",
+            {
+                "entity_id": "climate.zone_thermostat_jessie",
+                "hvac_mode": HVAC_MODE_AUTO,
+            },
+            blocking=True,
+        )
    state = hass.states.get("climate.zone_thermostat_jessie")
    attrs = state.attributes

@ -97,10 +105,11 @@ async def test_adam_climate_entity_climate_changes(hass, mock_smile_adam):
        {"entity_id": "climate.zone_lisa_wk", "temperature": 25},
        blocking=True,
    )
-    state = hass.states.get("climate.zone_lisa_wk")
-    attrs = state.attributes

-    assert attrs["temperature"] == 25.0
+    assert mock_smile_adam.set_temperature.call_count == 1
+    mock_smile_adam.set_temperature.assert_called_with(
+        "c50f167537524366a5af7aa3942feb1e", 25.0
+    )

    await hass.services.async_call(
        "climate",
@ -108,12 +117,11 @@ async def test_adam_climate_entity_climate_changes(hass, mock_smile_adam):
        {"entity_id": "climate.zone_lisa_wk", "preset_mode": "away"},
        blocking=True,
    )
-    state = hass.states.get("climate.zone_lisa_wk")
-    attrs = state.attributes

-    assert attrs["preset_mode"] == "away"
-
-    assert attrs["supported_features"] == 17
+    assert mock_smile_adam.set_preset.call_count == 1
+    mock_smile_adam.set_preset.assert_called_with(
+        "c50f167537524366a5af7aa3942feb1e", "away"
+    )

    await hass.services.async_call(
        "climate",
@ -122,10 +130,10 @@ async def test_adam_climate_entity_climate_changes(hass, mock_smile_adam):
        blocking=True,
    )

-    state = hass.states.get("climate.zone_thermostat_jessie")
-    attrs = state.attributes
-
-    assert attrs["temperature"] == 25.0
+    assert mock_smile_adam.set_temperature.call_count == 2
+    mock_smile_adam.set_temperature.assert_called_with(
+        "82fa13f017d240daa0d0ea1775420f24", 25.0
+    )

    await hass.services.async_call(
        "climate",
@ -133,10 +141,11 @@ async def test_adam_climate_entity_climate_changes(hass, mock_smile_adam):
        {"entity_id": "climate.zone_thermostat_jessie", "preset_mode": "home"},
        blocking=True,
    )
-    state = hass.states.get("climate.zone_thermostat_jessie")
-    attrs = state.attributes

-    assert attrs["preset_mode"] == "home"
+    assert mock_smile_adam.set_preset.call_count == 2
+    mock_smile_adam.set_preset.assert_called_with(
+        "82fa13f017d240daa0d0ea1775420f24", "home"
+    )


 async def test_anna_climate_entity_attributes(hass, mock_smile_anna):
@ -176,10 +185,10 @@ async def test_anna_climate_entity_climate_changes(hass, mock_smile_anna):
        blocking=True,
    )

-    state = hass.states.get("climate.anna")
-    attrs = state.attributes
-
-    assert attrs["temperature"] == 25.0
+    assert mock_smile_anna.set_temperature.call_count == 1
+    mock_smile_anna.set_temperature.assert_called_with(
+        "c784ee9fdab44e1395b8dee7d7a497d5", 25.0
+    )

    await hass.services.async_call(
        "climate",
@ -188,10 +197,10 @@ async def test_anna_climate_entity_climate_changes(hass, mock_smile_anna):
        blocking=True,
    )

-    state = hass.states.get("climate.anna")
-    attrs = state.attributes
-
-    assert attrs["preset_mode"] == "away"
+    assert mock_smile_anna.set_preset.call_count == 1
+    mock_smile_anna.set_preset.assert_called_with(
+        "c784ee9fdab44e1395b8dee7d7a497d5", "away"
+    )

    await hass.services.async_call(
        "climate",
@ -200,7 +209,8 @@ async def test_anna_climate_entity_climate_changes(hass, mock_smile_anna):
        blocking=True,
    )

-    state = hass.states.get("climate.anna")
-    attrs = state.attributes
-
-    assert state.state == "heat_cool"
+    assert mock_smile_anna.set_temperature.call_count == 1
+    assert mock_smile_anna.set_schedule_state.call_count == 1
+    mock_smile_anna.set_schedule_state.assert_called_with(
+        "c784ee9fdab44e1395b8dee7d7a497d5", None, "false"
+    )
--- a/tests/components/plugwise/test_switch.py
+++ b/tests/components/plugwise/test_switch.py
@ -1,10 +1,11 @@
 """Tests for the Plugwise switch integration."""
-
 from plugwise.exceptions import PlugwiseException
+import pytest

 from homeassistant.components.plugwise.const import DOMAIN
 from homeassistant.components.switch import DOMAIN as SWITCH_DOMAIN
 from homeassistant.config_entries import ConfigEntryState
+from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers import entity_registry as er

 from tests.common import MockConfigEntry
@ -29,23 +30,31 @@ async def test_adam_climate_switch_negative_testing(hass, mock_smile_adam):
    entry = await async_init_integration(hass, mock_smile_adam)
    assert entry.state is ConfigEntryState.LOADED

-    await hass.services.async_call(
-        "switch",
-        "turn_off",
-        {"entity_id": "switch.cv_pomp"},
-        blocking=True,
-    )
-    state = hass.states.get("switch.cv_pomp")
-    assert str(state.state) == "on"
+    with pytest.raises(HomeAssistantError):
+        await hass.services.async_call(
+            "switch",
+            "turn_off",
+            {"entity_id": "switch.cv_pomp"},
+            blocking=True,
+        )

-    await hass.services.async_call(
-        "switch",
-        "turn_on",
-        {"entity_id": "switch.fibaro_hc2"},
-        blocking=True,
+    assert mock_smile_adam.set_switch_state.call_count == 1
+    mock_smile_adam.set_switch_state.assert_called_with(
+        "78d1126fc4c743db81b61c20e88342a7", None, "relay", "off"
+    )
+
+    with pytest.raises(HomeAssistantError):
+        await hass.services.async_call(
+            "switch",
+            "turn_on",
+            {"entity_id": "switch.fibaro_hc2"},
+            blocking=True,
+        )
+
+    assert mock_smile_adam.set_switch_state.call_count == 2
+    mock_smile_adam.set_switch_state.assert_called_with(
+        "a28f588dc4a049a483fd03a30361ad3a", None, "relay", "on"
    )
-    state = hass.states.get("switch.fibaro_hc2")
-    assert str(state.state) == "on"


 async def test_adam_climate_switch_changes(hass, mock_smile_adam):
@ -59,8 +68,11 @@ async def test_adam_climate_switch_changes(hass, mock_smile_adam):
        {"entity_id": "switch.cv_pomp"},
        blocking=True,
    )
-    state = hass.states.get("switch.cv_pomp")
-    assert str(state.state) == "off"
+
+    assert mock_smile_adam.set_switch_state.call_count == 1
+    mock_smile_adam.set_switch_state.assert_called_with(
+        "78d1126fc4c743db81b61c20e88342a7", None, "relay", "off"
+    )

    await hass.services.async_call(
        "switch",
@ -68,17 +80,23 @@ async def test_adam_climate_switch_changes(hass, mock_smile_adam):
        {"entity_id": "switch.fibaro_hc2"},
        blocking=True,
    )
-    state = hass.states.get("switch.fibaro_hc2")
-    assert str(state.state) == "off"
+
+    assert mock_smile_adam.set_switch_state.call_count == 2
+    mock_smile_adam.set_switch_state.assert_called_with(
+        "a28f588dc4a049a483fd03a30361ad3a", None, "relay", "off"
+    )

    await hass.services.async_call(
        "switch",
-        "toggle",
+        "turn_on",
        {"entity_id": "switch.fibaro_hc2"},
        blocking=True,
    )
-    state = hass.states.get("switch.fibaro_hc2")
-    assert str(state.state) == "on"
+
+    assert mock_smile_adam.set_switch_state.call_count == 3
+    mock_smile_adam.set_switch_state.assert_called_with(
+        "a28f588dc4a049a483fd03a30361ad3a", None, "relay", "on"
+    )


 async def test_stretch_switch_entities(hass, mock_stretch):
@ -104,9 +122,10 @@ async def test_stretch_switch_changes(hass, mock_stretch):
        {"entity_id": "switch.koelkast_92c4a"},
        blocking=True,
    )
-
-    state = hass.states.get("switch.koelkast_92c4a")
-    assert str(state.state) == "off"
+    assert mock_stretch.set_switch_state.call_count == 1
+    mock_stretch.set_switch_state.assert_called_with(
+        "e1c884e7dede431dadee09506ec4f859", None, "relay", "off"
+    )

    await hass.services.async_call(
        "switch",
@ -114,17 +133,21 @@ async def test_stretch_switch_changes(hass, mock_stretch):
        {"entity_id": "switch.droger_52559"},
        blocking=True,
    )
-    state = hass.states.get("switch.droger_52559")
-    assert str(state.state) == "off"
+    assert mock_stretch.set_switch_state.call_count == 2
+    mock_stretch.set_switch_state.assert_called_with(
+        "cfe95cf3de1948c0b8955125bf754614", None, "relay", "off"
+    )

    await hass.services.async_call(
        "switch",
-        "toggle",
+        "turn_on",
        {"entity_id": "switch.droger_52559"},
        blocking=True,
    )
-    state = hass.states.get("switch.droger_52559")
-    assert str(state.state) == "on"
+    assert mock_stretch.set_switch_state.call_count == 3
+    mock_stretch.set_switch_state.assert_called_with(
+        "cfe95cf3de1948c0b8955125bf754614", None, "relay", "on"
+    )


 async def test_unique_id_migration_plug_relay(hass, mock_smile_adam):