Improve Advantage Air integration to Platinum quality (#41996)

* 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 <chris@talkingtontech.com>

* Fix ENTRY_STATE_NOT_LOADED

* Update test docstring

* Remove obsolete __init__'s

Co-authored-by: Chris Talkington <chris@talkingtontech.com>

* Remove unused __init__ from cover

* Code Quality 🏆 Platinum

* Re-push manifest.json

Co-authored-by: Chris Talkington <chris@talkingtontech.com>

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 (
+from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
-    CoordinatorEntity,
-    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):
+async def async_setup_entry(hass, entry):
-    """Set up AdvantageAir Config."""
+    """Set up Advantage Air config."""
-    ip_address = config_entry.data[CONF_IP_ADDRESS]
+    ip_address = entry.data[CONF_IP_ADDRESS]
-    port = config_entry.data[CONF_PORT]
+    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):
+async def async_unload_entry(hass, entry):
-    """Parent class for Advantage Air Entities."""
+    """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):
+    if unload_ok:
-        """Initialize common aspects of an Advantage Air sensor."""
+        hass.data[DOMAIN].pop(entry.entry_id)
-        super().__init__(instance["coordinator"])
-        self.async_change = instance["async_change"]
-        self.ac_key = ac_key
-        self.zone_key = zone_key
 
-    @property
+    return unload_ok
-    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"],
-        }

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 = []
-        entities = []
+    for ac_key, ac_device in instance["coordinator"].data["aircons"].items():
-        for ac_key, ac_device in instance["coordinator"].data["aircons"].items():
+        entities.append(AdvantageAirZoneFilter(instance, ac_key))
-            entities.append(AdvantageAirZoneFilter(instance, ac_key))
+        for zone_key, zone in ac_device["zones"].items():
-            for zone_key, zone in ac_device["zones"].items():
+            # Only add motion sensor when motion is enabled
-                # Only add motion sensor when motion is enabled
+            if zone["motionConfig"] >= 2:
-                if zone["motionConfig"] == 2:
+                entities.append(AdvantageAirZoneMotion(instance, ac_key, zone_key))
-                    entities.append(AdvantageAirZoneMotion(instance, ac_key, zone_key))
+    async_add_entities(entities)
-        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"]}

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():

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():

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"],
+        }

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": [
+  "codeowners": ["@Bre77"],
-    "@Bre77"
+  "requirements": ["advantage_air==0.2.1"],
-  ],
+  "quality_scale": "platinum"
-  "requirements": [
+}
-    "advantage_air==0.2.1"
-  ]
-}

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(
+        await self.async_change({self.ac_key: {"info": {self._time_key: value}}})
-            {self.ac_key: {"info": {f"countDownTo{self.time_period}": value}}}
-        )
 
 
 class AdvantageAirZoneVent(AdvantageAirEntity):

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):

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."""