From 421e2411d3d29c7550edf4b6e4f5365d142a5215 Mon Sep 17 00:00:00 2001 From: Tom Date: Sun, 8 Dec 2024 22:58:17 +0100 Subject: [PATCH] Plugwise Quality improvements (#132175) --- homeassistant/components/plugwise/climate.py | 18 +++++----- .../components/plugwise/coordinator.py | 4 +-- .../components/plugwise/quality_scale.yaml | 34 +++++++------------ .../components/plugwise/strings.json | 8 +++++ tests/components/plugwise/test_climate.py | 2 +- tests/components/plugwise/test_init.py | 1 - 6 files changed, 33 insertions(+), 34 deletions(-) diff --git a/homeassistant/components/plugwise/climate.py b/homeassistant/components/plugwise/climate.py index b27fd1d4f0e..4090405650a 100644 --- a/homeassistant/components/plugwise/climate.py +++ b/homeassistant/components/plugwise/climate.py @@ -15,7 +15,7 @@ from homeassistant.components.climate import ( ) from homeassistant.const import ATTR_TEMPERATURE, UnitOfTemperature from homeassistant.core import HomeAssistant, callback -from homeassistant.exceptions import HomeAssistantError +from homeassistant.exceptions import ServiceValidationError from homeassistant.helpers.entity_platform import AddEntitiesCallback from . import PlugwiseConfigEntry @@ -226,12 +226,6 @@ class PlugwiseClimateEntity(PlugwiseEntity, ClimateEntity): if ATTR_TARGET_TEMP_LOW in kwargs: data["setpoint_low"] = kwargs.get(ATTR_TARGET_TEMP_LOW) - for temperature in data.values(): - if temperature is None or not ( - self._attr_min_temp <= temperature <= self._attr_max_temp - ): - raise ValueError("Invalid temperature change requested") - if mode := kwargs.get(ATTR_HVAC_MODE): await self.async_set_hvac_mode(mode) @@ -241,7 +235,15 @@ class PlugwiseClimateEntity(PlugwiseEntity, ClimateEntity): async def async_set_hvac_mode(self, hvac_mode: HVACMode) -> None: """Set the hvac mode.""" if hvac_mode not in self.hvac_modes: - raise HomeAssistantError("Unsupported hvac_mode") + hvac_modes = ", ".join(self.hvac_modes) + raise ServiceValidationError( + translation_domain=DOMAIN, + translation_key="unsupported_hvac_mode_requested", + translation_placeholders={ + "hvac_mode": hvac_mode, + "hvac_modes": hvac_modes, + }, + ) if hvac_mode == self.hvac_mode: return diff --git a/homeassistant/components/plugwise/coordinator.py b/homeassistant/components/plugwise/coordinator.py index 6ce6855e7d6..bf9e7d31cc0 100644 --- a/homeassistant/components/plugwise/coordinator.py +++ b/homeassistant/components/plugwise/coordinator.py @@ -68,7 +68,6 @@ class PlugwiseDataUpdateCoordinator(DataUpdateCoordinator[PlugwiseData]): async def _async_update_data(self) -> PlugwiseData: """Fetch data from Plugwise.""" - data = PlugwiseData(devices={}, gateway={}) try: if not self._connected: await self._connect() @@ -85,9 +84,8 @@ class PlugwiseDataUpdateCoordinator(DataUpdateCoordinator[PlugwiseData]): raise UpdateFailed("Data incomplete or missing") from err except UnsupportedDeviceError as err: raise ConfigEntryError("Device with unsupported firmware") from err - else: - self._async_add_remove_devices(data, self.config_entry) + self._async_add_remove_devices(data, self.config_entry) return data def _async_add_remove_devices(self, data: PlugwiseData, entry: ConfigEntry) -> None: diff --git a/homeassistant/components/plugwise/quality_scale.yaml b/homeassistant/components/plugwise/quality_scale.yaml index 58a20046c5b..b2801319e91 100644 --- a/homeassistant/components/plugwise/quality_scale.yaml +++ b/homeassistant/components/plugwise/quality_scale.yaml @@ -8,26 +8,22 @@ rules: config-flow-test-coverage: status: todo comment: Cover test_form and zeroconf - runtime-data: - status: todo - comment: Clean up test_init for testing internals + runtime-data: done test-before-setup: done - appropriate-polling: - status: todo - comment: Clean up coordinator (L71) check for mypy happiness + appropriate-polling: done entity-unique-id: done has-entity-name: done entity-event-setup: done dependency-transparency: done action-setup: - status: todo - comment: Check if we have these, otherwise exempt + status: exempt + comment: Plugwise integration has no custom actions common-modules: status: todo comment: Verify entity for async_added_to_hass usage (discard?) docs-high-level-description: status: todo - comment: Rewrite top section + comment: Rewrite top section, docs PR prepared docs-installation-instructions: status: todo comment: Docs PR 36087 @@ -38,9 +34,7 @@ rules: config-entry-unloading: done log-when-unavailable: done entity-unavailable: done - action-exceptions: - status: todo - comment: Climate exception on ValueError should be ServiceValidationError + action-exceptions: done reauthentication-flow: status: exempt comment: The hubs have a hardcoded `Smile ID` printed on the sticker used as password, it can not be changed @@ -53,7 +47,7 @@ rules: integration-owner: done docs-installation-parameters: status: todo - comment: Docs PR 36087 (partial) + todo rewrite generically + comment: Docs PR 36087 (partial) + todo rewrite generically (PR prepared) docs-configuration-parameters: status: exempt comment: Plugwise has no options flow @@ -68,34 +62,32 @@ rules: diagnostics: done exception-translations: status: todo - comment: Add coordinator, util and climate exceptions + comment: Add coordinator, util exceptions (climate done in core 132175) icon-translations: done reconfiguration-flow: status: todo comment: This integration does not have any reconfiguration steps (yet) investigate how/why - dynamic-devices: - status: todo - comment: Add missing logic to button for unloading and creation + dynamic-devices: done discovery-update-info: done repair-issues: status: exempt comment: This integration does not have repairs docs-use-cases: status: todo - comment: Check for completeness + comment: Check for completeness, PR prepared docs-supported-devices: status: todo - comment: The list is there but could be improved for readability + comment: The list is there but could be improved for readability, PR prepared docs-supported-functions: status: todo comment: Check for completeness docs-data-update: done docs-known-limitations: status: todo - comment: Partial in 36087 but could be more elaborat + comment: Partial in 36087 but could be more elaborate docs-troubleshooting: status: todo - comment: Check for completeness + comment: Check for completeness, PR prepared docs-examples: status: todo comment: Check for completeness diff --git a/homeassistant/components/plugwise/strings.json b/homeassistant/components/plugwise/strings.json index 20029298c4e..badd522e78b 100644 --- a/homeassistant/components/plugwise/strings.json +++ b/homeassistant/components/plugwise/strings.json @@ -284,5 +284,13 @@ "name": "Relay" } } + }, + "exceptions": { + "invalid_temperature_change_requested": { + "message": "Invalid temperature change requested." + }, + "unsupported_hvac_mode_requested": { + "message": "Unsupported mode {hvac_mode} requested, valid modes are: {hvac_modes}." + } } } diff --git a/tests/components/plugwise/test_climate.py b/tests/components/plugwise/test_climate.py index c0c1c00c68d..39dcec92195 100644 --- a/tests/components/plugwise/test_climate.py +++ b/tests/components/plugwise/test_climate.py @@ -233,7 +233,7 @@ async def test_adam_climate_entity_climate_changes( "c50f167537524366a5af7aa3942feb1e", "off" ) - with pytest.raises(HomeAssistantError): + with pytest.raises(ServiceValidationError): await hass.services.async_call( CLIMATE_DOMAIN, SERVICE_SET_HVAC_MODE, diff --git a/tests/components/plugwise/test_init.py b/tests/components/plugwise/test_init.py index 3b9881c9e3d..99ff79263b6 100644 --- a/tests/components/plugwise/test_init.py +++ b/tests/components/plugwise/test_init.py @@ -78,7 +78,6 @@ async def test_load_unload_config_entry( await hass.config_entries.async_unload(mock_config_entry.entry_id) await hass.async_block_till_done() - assert not hass.data.get(DOMAIN) assert mock_config_entry.state is ConfigEntryState.NOT_LOADED