Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugwise Quality improvements #132175

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions homeassistant/components/plugwise/climate.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
)
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
Expand Down Expand Up @@ -231,7 +231,10 @@ async def async_set_temperature(self, **kwargs: Any) -> None:
if temperature is None or not (
self._attr_min_temp <= temperature <= self._attr_max_temp
):
raise ValueError("Invalid temperature change requested")
raise ServiceValidationError(
translation_domain=DOMAIN,
translation_key="invalid_temperature_change_requested",
)

if mode := kwargs.get(ATTR_HVAC_MODE):
await self.async_set_hvac_mode(mode)
Expand All @@ -242,7 +245,15 @@ async def async_set_temperature(self, **kwargs: Any) -> None:
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
Expand Down
4 changes: 1 addition & 3 deletions homeassistant/components/plugwise/coordinator.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ async def _connect(self) -> None:

async def _async_update_data(self) -> PlugwiseData:
"""Fetch data from Plugwise."""
data = PlugwiseData(devices={}, gateway={})
try:
if not self._connected:
await self._connect()
Expand All @@ -85,9 +84,8 @@ async def _async_update_data(self) -> 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:
Expand Down
34 changes: 13 additions & 21 deletions homeassistant/components/plugwise/quality_scale.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
8 changes: 8 additions & 0 deletions homeassistant/components/plugwise/strings.json
Original file line number Diff line number Diff line change
Expand Up @@ -281,5 +281,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}."
}
}
}
2 changes: 1 addition & 1 deletion tests/components/plugwise/test_climate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion tests/components/plugwise/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
Loading