From 582074d1dbfde07b9bc07a923712ce9f33f690b8 Mon Sep 17 00:00:00 2001 From: Brett Date: Wed, 21 Oct 2020 04:30:09 +1000 Subject: [PATCH] Improve Advantage Air integration to Platinum quality (#41996) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Rename to ADVANTAGE_AIR_DOMAIN * Remove attributes from binary sensor platform * Handle other motionConfig values * Restructure * Unloading * Fix unloading * PARALLEL_UPDATES * Remove pointless check * Rollup to sensor * Rollup to switch platform * @ctalkington suggestion of added __init__ back * Fix unload test Co-authored-by: Chris Talkington * Fix ENTRY_STATE_NOT_LOADED * Update test docstring * Remove obsolete __init__'s Co-authored-by: Chris Talkington * Remove unused __init__ from cover * Code Quality 🏆 Platinum * Re-push manifest.json Co-authored-by: Chris Talkington --- .../components/advantage_air/__init__.py | 62 +++++++------------ .../components/advantage_air/binary_sensor.py | 30 ++++----- .../components/advantage_air/climate.py | 8 ++- .../components/advantage_air/cover.py | 12 +++- .../components/advantage_air/entity.py | 35 +++++++++++ .../components/advantage_air/manifest.json | 11 ++-- .../components/advantage_air/sensor.py | 21 ++++--- .../components/advantage_air/switch.py | 2 +- tests/components/advantage_air/test_init.py | 12 +++- 9 files changed, 110 insertions(+), 83 deletions(-) create mode 100644 homeassistant/components/advantage_air/entity.py diff --git a/homeassistant/components/advantage_air/__init__.py b/homeassistant/components/advantage_air/__init__.py index b66306d2c69..7b270f1f335 100644 --- a/homeassistant/components/advantage_air/__init__.py +++ b/homeassistant/components/advantage_air/__init__.py @@ -1,5 +1,6 @@ """Advantage Air climate integration.""" +import asyncio from datetime import timedelta import logging @@ -8,11 +9,7 @@ from advantage_air import ApiError, advantage_air from homeassistant.const import CONF_IP_ADDRESS, CONF_PORT from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.helpers.aiohttp_client import async_get_clientsession -from homeassistant.helpers.update_coordinator import ( - CoordinatorEntity, - DataUpdateCoordinator, - UpdateFailed, -) +from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed from .const import ADVANTAGE_AIR_RETRY, DOMAIN @@ -23,15 +20,15 @@ _LOGGER = logging.getLogger(__name__) async def async_setup(hass, config): - """Set up AdvantageAir.""" + """Set up Advantage Air integration.""" hass.data[DOMAIN] = {} return True -async def async_setup_entry(hass, config_entry): - """Set up AdvantageAir Config.""" - ip_address = config_entry.data[CONF_IP_ADDRESS] - port = config_entry.data[CONF_PORT] +async def async_setup_entry(hass, entry): + """Set up Advantage Air config.""" + ip_address = entry.data[CONF_IP_ADDRESS] + port = entry.data[CONF_PORT] api = advantage_air( ip_address, port=port, @@ -65,46 +62,31 @@ async def async_setup_entry(hass, config_entry): if not coordinator.data: raise ConfigEntryNotReady - hass.data[DOMAIN][config_entry.entry_id] = { + hass.data[DOMAIN][entry.entry_id] = { "coordinator": coordinator, "async_change": async_change, } for platform in ADVANTAGE_AIR_PLATFORMS: hass.async_create_task( - hass.config_entries.async_forward_entry_setup(config_entry, platform) + hass.config_entries.async_forward_entry_setup(entry, platform) ) return True -class AdvantageAirEntity(CoordinatorEntity): - """Parent class for Advantage Air Entities.""" +async def async_unload_entry(hass, entry): + """Unload Advantage Air Config.""" + unload_ok = all( + await asyncio.gather( + *[ + hass.config_entries.async_forward_entry_unload(entry, component) + for component in ADVANTAGE_AIR_PLATFORMS + ] + ) + ) - def __init__(self, instance, ac_key, zone_key=None): - """Initialize common aspects of an Advantage Air sensor.""" - super().__init__(instance["coordinator"]) - self.async_change = instance["async_change"] - self.ac_key = ac_key - self.zone_key = zone_key + if unload_ok: + hass.data[DOMAIN].pop(entry.entry_id) - @property - def _ac(self): - return self.coordinator.data["aircons"][self.ac_key]["info"] - - @property - def _zone(self): - if not self.zone_key: - return None - return self.coordinator.data["aircons"][self.ac_key]["zones"][self.zone_key] - - @property - def device_info(self): - """Return parent device information.""" - return { - "identifiers": {(DOMAIN, self.coordinator.data["system"]["rid"])}, - "name": self.coordinator.data["system"]["name"], - "manufacturer": "Advantage Air", - "model": self.coordinator.data["system"]["sysType"], - "sw_version": self.coordinator.data["system"]["myAppRev"], - } + return unload_ok diff --git a/homeassistant/components/advantage_air/binary_sensor.py b/homeassistant/components/advantage_air/binary_sensor.py index 25a1bb8a805..50a7ef83895 100644 --- a/homeassistant/components/advantage_air/binary_sensor.py +++ b/homeassistant/components/advantage_air/binary_sensor.py @@ -1,6 +1,5 @@ """Binary Sensor platform for Advantage Air integration.""" -from homeassistant.components.advantage_air import AdvantageAirEntity from homeassistant.components.binary_sensor import ( DEVICE_CLASS_MOTION, DEVICE_CLASS_PROBLEM, @@ -8,6 +7,9 @@ from homeassistant.components.binary_sensor import ( ) from .const import DOMAIN as ADVANTAGE_AIR_DOMAIN +from .entity import AdvantageAirEntity + +PARALLEL_UPDATES = 0 async def async_setup_entry(hass, config_entry, async_add_entities): @@ -15,19 +17,18 @@ async def async_setup_entry(hass, config_entry, async_add_entities): instance = hass.data[ADVANTAGE_AIR_DOMAIN][config_entry.entry_id] - if "aircons" in instance["coordinator"].data: - entities = [] - for ac_key, ac_device in instance["coordinator"].data["aircons"].items(): - entities.append(AdvantageAirZoneFilter(instance, ac_key)) - for zone_key, zone in ac_device["zones"].items(): - # Only add motion sensor when motion is enabled - if zone["motionConfig"] == 2: - entities.append(AdvantageAirZoneMotion(instance, ac_key, zone_key)) - async_add_entities(entities) + entities = [] + for ac_key, ac_device in instance["coordinator"].data["aircons"].items(): + entities.append(AdvantageAirZoneFilter(instance, ac_key)) + for zone_key, zone in ac_device["zones"].items(): + # Only add motion sensor when motion is enabled + if zone["motionConfig"] >= 2: + entities.append(AdvantageAirZoneMotion(instance, ac_key, zone_key)) + async_add_entities(entities) class AdvantageAirZoneFilter(AdvantageAirEntity, BinarySensorEntity): - """AdvantageAir Filter.""" + """Advantage Air Filter.""" @property def name(self): @@ -51,7 +52,7 @@ class AdvantageAirZoneFilter(AdvantageAirEntity, BinarySensorEntity): class AdvantageAirZoneMotion(AdvantageAirEntity, BinarySensorEntity): - """AdvantageAir Zone Motion.""" + """Advantage Air Zone Motion.""" @property def name(self): @@ -72,8 +73,3 @@ class AdvantageAirZoneMotion(AdvantageAirEntity, BinarySensorEntity): def is_on(self): """Return if motion is detect.""" return self._zone["motion"] - - @property - def device_state_attributes(self): - """Return additional motion configuration.""" - return {"motionConfig": self._zone["motionConfig"]} diff --git a/homeassistant/components/advantage_air/climate.py b/homeassistant/components/advantage_air/climate.py index c829824c94f..d3c4e897819 100644 --- a/homeassistant/components/advantage_air/climate.py +++ b/homeassistant/components/advantage_air/climate.py @@ -1,6 +1,5 @@ """Climate platform for Advantage Air integration.""" -from homeassistant.components.advantage_air import AdvantageAirEntity from homeassistant.components.climate import ClimateEntity from homeassistant.components.climate.const import ( FAN_AUTO, @@ -22,8 +21,9 @@ from .const import ( ADVANTAGE_AIR_STATE_OFF, ADVANTAGE_AIR_STATE_ON, ADVANTAGE_AIR_STATE_OPEN, - DOMAIN, + DOMAIN as ADVANTAGE_AIR_DOMAIN, ) +from .entity import AdvantageAirEntity ADVANTAGE_AIR_HVAC_MODES = { "heat": HVAC_MODE_HEAT, @@ -51,11 +51,13 @@ AC_HVAC_MODES = [ ] ZONE_HVAC_MODES = [HVAC_MODE_OFF, HVAC_MODE_FAN_ONLY] +PARALLEL_UPDATES = 0 + async def async_setup_entry(hass, config_entry, async_add_entities): """Set up AdvantageAir climate platform.""" - instance = hass.data[DOMAIN][config_entry.entry_id] + instance = hass.data[ADVANTAGE_AIR_DOMAIN][config_entry.entry_id] entities = [] for ac_key, ac_device in instance["coordinator"].data["aircons"].items(): diff --git a/homeassistant/components/advantage_air/cover.py b/homeassistant/components/advantage_air/cover.py index a238a9cdca8..69d66849cd6 100644 --- a/homeassistant/components/advantage_air/cover.py +++ b/homeassistant/components/advantage_air/cover.py @@ -1,6 +1,5 @@ """Cover platform for Advantage Air integration.""" -from homeassistant.components.advantage_air import AdvantageAirEntity from homeassistant.components.cover import ( ATTR_POSITION, DEVICE_CLASS_DAMPER, @@ -10,13 +9,20 @@ from homeassistant.components.cover import ( CoverEntity, ) -from .const import ADVANTAGE_AIR_STATE_CLOSE, ADVANTAGE_AIR_STATE_OPEN, DOMAIN +from .const import ( + ADVANTAGE_AIR_STATE_CLOSE, + ADVANTAGE_AIR_STATE_OPEN, + DOMAIN as ADVANTAGE_AIR_DOMAIN, +) +from .entity import AdvantageAirEntity + +PARALLEL_UPDATES = 0 async def async_setup_entry(hass, config_entry, async_add_entities): """Set up AdvantageAir cover platform.""" - instance = hass.data[DOMAIN][config_entry.entry_id] + instance = hass.data[ADVANTAGE_AIR_DOMAIN][config_entry.entry_id] entities = [] for ac_key, ac_device in instance["coordinator"].data["aircons"].items(): diff --git a/homeassistant/components/advantage_air/entity.py b/homeassistant/components/advantage_air/entity.py new file mode 100644 index 00000000000..ea20368c10f --- /dev/null +++ b/homeassistant/components/advantage_air/entity.py @@ -0,0 +1,35 @@ +"""Advantage Air parent entity class.""" + +from homeassistant.helpers.update_coordinator import CoordinatorEntity + +from .const import DOMAIN + + +class AdvantageAirEntity(CoordinatorEntity): + """Parent class for Advantage Air Entities.""" + + def __init__(self, instance, ac_key, zone_key=None): + """Initialize common aspects of an Advantage Air sensor.""" + super().__init__(instance["coordinator"]) + self.async_change = instance["async_change"] + self.ac_key = ac_key + self.zone_key = zone_key + + @property + def _ac(self): + return self.coordinator.data["aircons"][self.ac_key]["info"] + + @property + def _zone(self): + return self.coordinator.data["aircons"][self.ac_key]["zones"][self.zone_key] + + @property + def device_info(self): + """Return parent device information.""" + return { + "identifiers": {(DOMAIN, self.coordinator.data["system"]["rid"])}, + "name": self.coordinator.data["system"]["name"], + "manufacturer": "Advantage Air", + "model": self.coordinator.data["system"]["sysType"], + "sw_version": self.coordinator.data["system"]["myAppRev"], + } diff --git a/homeassistant/components/advantage_air/manifest.json b/homeassistant/components/advantage_air/manifest.json index eab487cd7da..87655d61be4 100644 --- a/homeassistant/components/advantage_air/manifest.json +++ b/homeassistant/components/advantage_air/manifest.json @@ -3,10 +3,7 @@ "name": "Advantage Air", "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/advantage_air", - "codeowners": [ - "@Bre77" - ], - "requirements": [ - "advantage_air==0.2.1" - ] -} \ No newline at end of file + "codeowners": ["@Bre77"], + "requirements": ["advantage_air==0.2.1"], + "quality_scale": "platinum" +} diff --git a/homeassistant/components/advantage_air/sensor.py b/homeassistant/components/advantage_air/sensor.py index e124f7b2a08..d25ce4888fc 100644 --- a/homeassistant/components/advantage_air/sensor.py +++ b/homeassistant/components/advantage_air/sensor.py @@ -1,16 +1,18 @@ """Sensor platform for Advantage Air integration.""" import voluptuous as vol -from homeassistant.components.advantage_air import AdvantageAirEntity from homeassistant.const import PERCENTAGE from homeassistant.helpers import config_validation as cv, entity_platform from .const import ADVANTAGE_AIR_STATE_OPEN, DOMAIN as ADVANTAGE_AIR_DOMAIN +from .entity import AdvantageAirEntity ADVANTAGE_AIR_SET_COUNTDOWN_VALUE = "minutes" ADVANTAGE_AIR_SET_COUNTDOWN_UNIT = "min" ADVANTAGE_AIR_SERVICE_SET_TIME_TO = "set_time_to" +PARALLEL_UPDATES = 0 + async def async_setup_entry(hass, config_entry, async_add_entities): """Set up AdvantageAir sensor platform.""" @@ -41,25 +43,26 @@ async def async_setup_entry(hass, config_entry, async_add_entities): class AdvantageAirTimeTo(AdvantageAirEntity): """Representation of Advantage Air timer control.""" - def __init__(self, instance, ac_key, time_period): + def __init__(self, instance, ac_key, action): """Initialize the Advantage Air timer control.""" super().__init__(instance, ac_key) - self.time_period = time_period + self.action = action + self._time_key = f"countDownTo{self.action}" @property def name(self): """Return the name.""" - return f'{self._ac["name"]} Time To {self.time_period}' + return f'{self._ac["name"]} Time To {self.action}' @property def unique_id(self): """Return a unique id.""" - return f'{self.coordinator.data["system"]["rid"]}-{self.ac_key}-timeto{self.time_period}' + return f'{self.coordinator.data["system"]["rid"]}-{self.ac_key}-timeto{self.action}' @property def state(self): """Return the current value.""" - return self._ac[f"countDownTo{self.time_period}"] + return self._ac[self._time_key] @property def unit_of_measurement(self): @@ -69,16 +72,14 @@ class AdvantageAirTimeTo(AdvantageAirEntity): @property def icon(self): """Return a representative icon of the timer.""" - if self._ac[f"countDownTo{self.time_period}"] > 0: + if self._ac[self._time_key] > 0: return "mdi:timer-outline" return "mdi:timer-off-outline" async def set_time_to(self, **kwargs): """Set the timer value.""" value = min(720, max(0, int(kwargs[ADVANTAGE_AIR_SET_COUNTDOWN_VALUE]))) - await self.async_change( - {self.ac_key: {"info": {f"countDownTo{self.time_period}": value}}} - ) + await self.async_change({self.ac_key: {"info": {self._time_key: value}}}) class AdvantageAirZoneVent(AdvantageAirEntity): diff --git a/homeassistant/components/advantage_air/switch.py b/homeassistant/components/advantage_air/switch.py index 0416613bf8c..6c687c1427e 100644 --- a/homeassistant/components/advantage_air/switch.py +++ b/homeassistant/components/advantage_air/switch.py @@ -1,6 +1,5 @@ """Switch platform for Advantage Air integration.""" -from homeassistant.components.advantage_air import AdvantageAirEntity from homeassistant.helpers.entity import ToggleEntity from .const import ( @@ -8,6 +7,7 @@ from .const import ( ADVANTAGE_AIR_STATE_ON, DOMAIN as ADVANTAGE_AIR_DOMAIN, ) +from .entity import AdvantageAirEntity async def async_setup_entry(hass, config_entry, async_add_entities): diff --git a/tests/components/advantage_air/test_init.py b/tests/components/advantage_air/test_init.py index 22be288e452..1567b9ee8ad 100644 --- a/tests/components/advantage_air/test_init.py +++ b/tests/components/advantage_air/test_init.py @@ -1,6 +1,10 @@ """Test the Advantage Air Initialization.""" -from homeassistant.config_entries import ENTRY_STATE_LOADED, ENTRY_STATE_SETUP_RETRY +from homeassistant.config_entries import ( + ENTRY_STATE_LOADED, + ENTRY_STATE_NOT_LOADED, + ENTRY_STATE_SETUP_RETRY, +) from tests.components.advantage_air import ( TEST_SYSTEM_DATA, @@ -10,7 +14,7 @@ from tests.components.advantage_air import ( async def test_async_setup_entry(hass, aioclient_mock): - """Test a successful setup entry.""" + """Test a successful setup entry and unload.""" aioclient_mock.get( TEST_SYSTEM_URL, @@ -20,6 +24,10 @@ async def test_async_setup_entry(hass, aioclient_mock): entry = await add_mock_config(hass) assert entry.state == ENTRY_STATE_LOADED + assert await hass.config_entries.async_unload(entry.entry_id) + await hass.async_block_till_done() + assert entry.state == ENTRY_STATE_NOT_LOADED + async def test_async_setup_entry_failure(hass, aioclient_mock): """Test a unsuccessful setup entry."""