Migrate Aurora_ABB_Powerone to DataUpdateCoordinator (#72363)

* Refactor to DataUpdateCoordinator

* Fix tests for sunset/sunrise

* Correct time offsets in tests

* Fix time intervals (attempt 2)

* Merge dev

* Fix tests after rebase

* Fix isort

* Address review comments: const and increase cov

* Fix merge problems

* Refactor, removing unnecessary file

* Perform blocking serial IO in the executor

* Replace deprecated  async_setup_platforms

* Update based on review comments

* Fix tests

* Update based on review comments.

* Update homeassistant/components/aurora_abb_powerone/sensor.py

Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>

* Use freezer for time deltas.

* Address review comments

---------

Co-authored-by: Dave T <davet2001@users.noreply.github.com>
Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>

homeassistant/components/aurora_abb_powerone/__init__.py

@@ -12,13 +12,14 @@
 
 import logging
 
-from aurorapy.client import AuroraSerialClient
+from aurorapy.client import AuroraError, AuroraSerialClient, AuroraTimeoutError
 
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import CONF_ADDRESS, CONF_PORT, Platform
 from homeassistant.core import HomeAssistant
+from homeassistant.helpers.update_coordinator import DataUpdateCoordinator
 
-from .const import DOMAIN
+from .const import DOMAIN, SCAN_INTERVAL
 
 PLATFORMS = [Platform.SENSOR]
 
@@ -30,8 +31,10 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
 
     comport = entry.data[CONF_PORT]
     address = entry.data[CONF_ADDRESS]
-    ser_client = AuroraSerialClient(address, comport, parity="N", timeout=1)
+    coordinator = AuroraAbbDataUpdateCoordinator(hass, comport, address)
-    hass.data.setdefault(DOMAIN, {})[entry.entry_id] = ser_client
+    await coordinator.async_config_entry_first_refresh()
+
+    hass.data.setdefault(DOMAIN, {})[entry.entry_id] = coordinator
     await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)
 
     return True
@@ -47,3 +50,58 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
         hass.data[DOMAIN].pop(entry.entry_id)
 
     return unload_ok
+
+
+class AuroraAbbDataUpdateCoordinator(DataUpdateCoordinator[dict[str, float]]):
+    """Class to manage fetching AuroraAbbPowerone data."""
+
+    def __init__(self, hass: HomeAssistant, comport: str, address: int) -> None:
+        """Initialize the data update coordinator."""
+        self.available_prev = False
+        self.available = False
+        self.client = AuroraSerialClient(address, comport, parity="N", timeout=1)
+        super().__init__(hass, _LOGGER, name=DOMAIN, update_interval=SCAN_INTERVAL)
+
+    def _update_data(self) -> dict[str, float]:
+        """Fetch new state data for the sensor.
+
+        This is the only function that should fetch new data for Home Assistant.
+        """
+        data: dict[str, float] = {}
+        self.available_prev = self.available
+        try:
+            self.client.connect()
+
+            # read ADC channel 3 (grid power output)
+            power_watts = self.client.measure(3, True)
+            temperature_c = self.client.measure(21)
+            energy_wh = self.client.cumulated_energy(5)
+        except AuroraTimeoutError:
+            self.available = False
+            _LOGGER.debug("No response from inverter (could be dark)")
+        except AuroraError as error:
+            self.available = False
+            raise error
+        else:
+            data["instantaneouspower"] = round(power_watts, 1)
+            data["temp"] = round(temperature_c, 1)
+            data["totalenergy"] = round(energy_wh / 1000, 2)
+            self.available = True
+
+        finally:
+            if self.available != self.available_prev:
+                if self.available:
+                    _LOGGER.info("Communication with %s back online", self.name)
+                else:
+                    _LOGGER.warning(
+                        "Communication with %s lost",
+                        self.name,
+                    )
+            if self.client.serline.isOpen():
+                self.client.close()
+
+        return data
+
+    async def _async_update_data(self) -> dict[str, float]:
+        """Update inverter data in the executor."""
+        return await self.hass.async_add_executor_job(self._update_data)

homeassistant/components/aurora_abb_powerone/aurora_device.py

@@ -1,57 +0,0 @@
-"""Top level class for AuroraABBPowerOneSolarPV inverters and sensors."""
-from __future__ import annotations
-
-from collections.abc import Mapping
-import logging
-from typing import Any
-
-from aurorapy.client import AuroraSerialClient
-
-from homeassistant.helpers.device_registry import DeviceInfo
-from homeassistant.helpers.entity import Entity
-
-from .const import (
-    ATTR_DEVICE_NAME,
-    ATTR_FIRMWARE,
-    ATTR_MODEL,
-    ATTR_SERIAL_NUMBER,
-    DEFAULT_DEVICE_NAME,
-    DOMAIN,
-    MANUFACTURER,
-)
-
-_LOGGER = logging.getLogger(__name__)
-
-
-class AuroraEntity(Entity):
-    """Representation of an Aurora ABB PowerOne device."""
-
-    def __init__(self, client: AuroraSerialClient, data: Mapping[str, Any]) -> None:
-        """Initialise the basic device."""
-        self._data = data
-        self.type = "device"
-        self.client = client
-        self._available = True
-
-    @property
-    def unique_id(self) -> str | None:
-        """Return the unique id for this device."""
-        if (serial := self._data.get(ATTR_SERIAL_NUMBER)) is None:
-            return None
-        return f"{serial}_{self.entity_description.key}"
-
-    @property
-    def available(self) -> bool:
-        """Return True if entity is available."""
-        return self._available
-
-    @property
-    def device_info(self) -> DeviceInfo:
-        """Return device specific attributes."""
-        return DeviceInfo(
-            identifiers={(DOMAIN, self._data[ATTR_SERIAL_NUMBER])},
-            manufacturer=MANUFACTURER,
-            model=self._data[ATTR_MODEL],
-            name=self._data.get(ATTR_DEVICE_NAME, DEFAULT_DEVICE_NAME),
-            sw_version=self._data[ATTR_FIRMWARE],
-        )

homeassistant/components/aurora_abb_powerone/const.py

@@ -1,5 +1,7 @@
 """Constants for the Aurora ABB PowerOne integration."""
 
+from datetime import timedelta
+
 DOMAIN = "aurora_abb_powerone"
 
 # Min max addresses and default according to here:
@@ -8,6 +10,7 @@ DOMAIN = "aurora_abb_powerone"
 MIN_ADDRESS = 2
 MAX_ADDRESS = 63
 DEFAULT_ADDRESS = 2
+SCAN_INTERVAL = timedelta(seconds=30)
 
 DEFAULT_INTEGRATION_TITLE = "PhotoVoltaic Inverters"
 DEFAULT_DEVICE_NAME = "Solar Inverter"

homeassistant/components/aurora_abb_powerone/sensor.py

@@ -5,8 +5,6 @@ from collections.abc import Mapping
 import logging
 from typing import Any
 
-from aurorapy.client import AuroraError, AuroraSerialClient, AuroraTimeoutError
-
 from homeassistant.components.sensor import (
     SensorDeviceClass,
     SensorEntity,
@@ -21,10 +19,21 @@ from homeassistant.const import (
     UnitOfTemperature,
 )
 from homeassistant.core import HomeAssistant
+from homeassistant.helpers.device_registry import DeviceInfo
 from homeassistant.helpers.entity_platform import AddEntitiesCallback
+from homeassistant.helpers.typing import StateType
+from homeassistant.helpers.update_coordinator import CoordinatorEntity
 
-from .aurora_device import AuroraEntity
+from . import AuroraAbbDataUpdateCoordinator
-from .const import DOMAIN
+from .const import (
+    ATTR_DEVICE_NAME,
+    ATTR_FIRMWARE,
+    ATTR_MODEL,
+    ATTR_SERIAL_NUMBER,
+    DEFAULT_DEVICE_NAME,
+    DOMAIN,
+    MANUFACTURER,
+)
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -61,70 +70,40 @@ async def async_setup_entry(
     """Set up aurora_abb_powerone sensor based on a config entry."""
     entities = []
 
-    client = hass.data[DOMAIN][config_entry.entry_id]
+    coordinator = hass.data[DOMAIN][config_entry.entry_id]
     data = config_entry.data
 
     for sens in SENSOR_TYPES:
-        entities.append(AuroraSensor(client, data, sens))
+        entities.append(AuroraSensor(coordinator, data, sens))
 
     _LOGGER.debug("async_setup_entry adding %d entities", len(entities))
     async_add_entities(entities, True)
 
 
-class AuroraSensor(AuroraEntity, SensorEntity):
+class AuroraSensor(CoordinatorEntity[AuroraAbbDataUpdateCoordinator], SensorEntity):
-    """Representation of a Sensor on a Aurora ABB PowerOne Solar inverter."""
+    """Representation of a Sensor on an Aurora ABB PowerOne Solar inverter."""
 
     _attr_has_entity_name = True
 
     def __init__(
         self,
-        client: AuroraSerialClient,
+        coordinator: AuroraAbbDataUpdateCoordinator,
         data: Mapping[str, Any],
         entity_description: SensorEntityDescription,
     ) -> None:
         """Initialize the sensor."""
-        super().__init__(client, data)
+        super().__init__(coordinator)
         self.entity_description = entity_description
-        self.available_prev = True
+        self._attr_unique_id = f"{data[ATTR_SERIAL_NUMBER]}_{entity_description.key}"
+        self._attr_device_info = DeviceInfo(
+            identifiers={(DOMAIN, data[ATTR_SERIAL_NUMBER])},
+            manufacturer=MANUFACTURER,
+            model=data[ATTR_MODEL],
+            name=data.get(ATTR_DEVICE_NAME, DEFAULT_DEVICE_NAME),
+            sw_version=data[ATTR_FIRMWARE],
+        )
 
-    def update(self) -> None:
+    @property
-        """Fetch new state data for the sensor.
+    def native_value(self) -> StateType:
-
+        """Get the value of the sensor from previously collected data."""
-        This is the only method that should fetch new data for Home Assistant.
+        return self.coordinator.data.get(self.entity_description.key)
-        """
-        try:
-            self.available_prev = self._attr_available
-            self.client.connect()
-            if self.entity_description.key == "instantaneouspower":
-                # read ADC channel 3 (grid power output)
-                power_watts = self.client.measure(3, True)
-                self._attr_native_value = round(power_watts, 1)
-            elif self.entity_description.key == "temp":
-                temperature_c = self.client.measure(21)
-                self._attr_native_value = round(temperature_c, 1)
-            elif self.entity_description.key == "totalenergy":
-                energy_wh = self.client.cumulated_energy(5)
-                self._attr_native_value = round(energy_wh / 1000, 2)
-            self._attr_available = True
-
-        except AuroraTimeoutError:
-            self._attr_state = None
-            self._attr_native_value = None
-            self._attr_available = False
-            _LOGGER.debug("No response from inverter (could be dark)")
-        except AuroraError as error:
-            self._attr_state = None
-            self._attr_native_value = None
-            self._attr_available = False
-            raise error
-        finally:
-            if self._attr_available != self.available_prev:
-                if self._attr_available:
-                    _LOGGER.info("Communication with %s back online", self.name)
-                else:
-                    _LOGGER.warning(
-                        "Communication with %s lost",
-                        self.name,
-                    )
-            if self.client.serline.isOpen():
-                self.client.close()

tests/components/aurora_abb_powerone/test_config_flow.py

@@ -1,5 +1,4 @@
 """Test the Aurora ABB PowerOne Solar PV config flow."""
-from logging import INFO
 from unittest.mock import patch
 
 from aurorapy.client import AuroraError, AuroraTimeoutError
@@ -49,9 +48,6 @@ async def test_form(hass: HomeAssistant) -> None:
     ), patch(
         "aurorapy.client.AuroraSerialClient.firmware",
         return_value="1.234",
-    ), patch(
-        "homeassistant.components.aurora_abb_powerone.config_flow._LOGGER.getEffectiveLevel",
-        return_value=INFO,
     ) as mock_setup, patch(
         "homeassistant.components.aurora_abb_powerone.async_setup_entry",
         return_value=True,

tests/components/aurora_abb_powerone/test_init.py

@@ -18,9 +18,6 @@ async def test_unload_entry(hass: HomeAssistant) -> None:
     """Test unloading the aurora_abb_powerone entry."""
 
     with patch("aurorapy.client.AuroraSerialClient.connect", return_value=None), patch(
-        "homeassistant.components.aurora_abb_powerone.sensor.AuroraSensor.update",
-        return_value=None,
-    ), patch(
         "aurorapy.client.AuroraSerialClient.serial_number",
         return_value="9876543",
     ), patch(

tests/components/aurora_abb_powerone/test_sensor.py

@@ -1,8 +1,8 @@
 """Test the Aurora ABB PowerOne Solar PV sensors."""
-from datetime import timedelta
 from unittest.mock import patch
 
 from aurorapy.client import AuroraError, AuroraTimeoutError
+from freezegun.api import FrozenDateTimeFactory
 
 from homeassistant.components.aurora_abb_powerone.const import (
     ATTR_DEVICE_NAME,
@@ -11,10 +11,10 @@ from homeassistant.components.aurora_abb_powerone.const import (
     ATTR_SERIAL_NUMBER,
     DEFAULT_INTEGRATION_TITLE,
     DOMAIN,
+    SCAN_INTERVAL,
 )
 from homeassistant.const import CONF_ADDRESS, CONF_PORT
 from homeassistant.core import HomeAssistant
-import homeassistant.util.dt as dt_util
 
 from tests.common import MockConfigEntry, async_fire_time_changed
 
@@ -95,14 +95,16 @@ async def test_sensors(hass: HomeAssistant) -> None:
         assert energy.state == "12.35"
 
 
-async def test_sensor_dark(hass: HomeAssistant) -> None:
+async def test_sensor_dark(hass: HomeAssistant, freezer: FrozenDateTimeFactory) -> None:
     """Test that darkness (no comms) is handled correctly."""
     mock_entry = _mock_config_entry()
 
-    utcnow = dt_util.utcnow()
     # sun is up
     with patch("aurorapy.client.AuroraSerialClient.connect", return_value=None), patch(
         "aurorapy.client.AuroraSerialClient.measure", side_effect=_simulated_returns
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.cumulated_energy",
+        side_effect=_simulated_returns,
     ), patch(
         "aurorapy.client.AuroraSerialClient.serial_number",
         return_value="9876543",
@@ -128,16 +130,24 @@ async def test_sensor_dark(hass: HomeAssistant) -> None:
     with patch("aurorapy.client.AuroraSerialClient.connect", return_value=None), patch(
         "aurorapy.client.AuroraSerialClient.measure",
         side_effect=AuroraTimeoutError("No response after 10 seconds"),
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.cumulated_energy",
+        side_effect=AuroraTimeoutError("No response after 3 tries"),
     ):
-        async_fire_time_changed(hass, utcnow + timedelta(seconds=60))
+        freezer.tick(SCAN_INTERVAL * 2)
+        async_fire_time_changed(hass)
         await hass.async_block_till_done()
-        power = hass.states.get("sensor.mydevicename_power_output")
+        power = hass.states.get("sensor.mydevicename_total_energy")
         assert power.state == "unknown"
     # sun rose again
     with patch("aurorapy.client.AuroraSerialClient.connect", return_value=None), patch(
         "aurorapy.client.AuroraSerialClient.measure", side_effect=_simulated_returns
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.cumulated_energy",
+        side_effect=_simulated_returns,
     ):
-        async_fire_time_changed(hass, utcnow + timedelta(seconds=60))
+        freezer.tick(SCAN_INTERVAL * 4)
+        async_fire_time_changed(hass)
         await hass.async_block_till_done()
         power = hass.states.get("sensor.mydevicename_power_output")
         assert power is not None
@@ -146,8 +156,12 @@ async def test_sensor_dark(hass: HomeAssistant) -> None:
     with patch("aurorapy.client.AuroraSerialClient.connect", return_value=None), patch(
         "aurorapy.client.AuroraSerialClient.measure",
         side_effect=AuroraTimeoutError("No response after 10 seconds"),
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.cumulated_energy",
+        side_effect=AuroraError("No response after 10 seconds"),
     ):
-        async_fire_time_changed(hass, utcnow + timedelta(seconds=60))
+        freezer.tick(SCAN_INTERVAL * 6)
+        async_fire_time_changed(hass)
         await hass.async_block_till_done()
         power = hass.states.get("sensor.mydevicename_power_output")
         assert power.state == "unknown"  # should this be 'available'?
@@ -160,7 +174,7 @@ async def test_sensor_unknown_error(hass: HomeAssistant) -> None:
     with patch("aurorapy.client.AuroraSerialClient.connect", return_value=None), patch(
         "aurorapy.client.AuroraSerialClient.measure",
         side_effect=AuroraError("another error"),
-    ):
+    ), patch("serial.Serial.isOpen", return_value=True):
         mock_entry.add_to_hass(hass)
         await hass.config_entries.async_setup(mock_entry.entry_id)
         await hass.async_block_till_done()