Fix initial HomeWizard integration issues (#63921)

* Use helper for setup and unload

* Set mutable object in __init__

* Move logic out of try..except

* Print stack trace on error in config flow

* Don't catch broad except in update_coordinator, already handled

* Fix typo

* Combine available with super.available()

* Fix issues with config_flow test

* Test coordiantor indirectly

* Test coordinator by moving time forward

* set utcnow beforehand and increase timedelta

* Roll back f658749

* Rolled back the rollback and fixed timing

* Move setting time after adding to hass

homeassistant/components/homewizard/__init__.py

@@ -1,5 +1,4 @@
 """The Homewizard integration."""
-import asyncio
 import logging
 
 from aiohwenergy import DisabledError
@@ -41,10 +40,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
         COORDINATOR: coordinator,
     }
 
-    for component in PLATFORMS:
+    hass.config_entries.async_setup_platforms(entry, PLATFORMS)
-        hass.async_create_task(
-            hass.config_entries.async_forward_entry_setup(entry, component)
-        )
 
     return True
 
@@ -53,14 +49,7 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     """Unload a config entry."""
     _LOGGER.debug("__init__ async_unload_entry")
 
-    unload_ok = all(
+    unload_ok = await hass.config_entries.async_unload_platforms(entry, PLATFORMS)
-        await asyncio.gather(
-            *(
-                hass.config_entries.async_forward_entry_unload(entry, component)
-                for component in PLATFORMS
-            )
-        )
-    )
 
     if unload_ok:
         config_data = hass.data[DOMAIN].pop(entry.entry_id)

homeassistant/components/homewizard/config_flow.py

@@ -23,7 +23,10 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
     """Handle a config flow for P1 meter."""
 
     VERSION = 1
-    config: dict[str, str | int] = {}
+
+    def __init__(self) -> None:
+        """Initialize the HomeWizard config flow."""
+        self.config: dict[str, str | int] = {}
 
     async def async_step_user(
         self, user_input: dict[str, Any] | None = None
@@ -143,19 +146,16 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
         # This is to test the connection and to get info for unique_id
         energy_api = aiohwenergy.HomeWizardEnergy(ip_address)
 
-        initialized = False
         try:
             with async_timeout.timeout(10):
                 await energy_api.initialize()
-                if energy_api.device is not None:
-                    initialized = True
 
         except aiohwenergy.DisabledError as ex:
             _LOGGER.error("API disabled, API must be enabled in the app")
             raise AbortFlow("api_not_enabled") from ex
 
         except Exception as ex:
-            _LOGGER.error(
+            _LOGGER.exception(
                 "Error connecting with Energy Device at %s",
                 ip_address,
             )
@@ -164,7 +164,7 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
         finally:
             await energy_api.close()
 
-        if not initialized:
+        if energy_api.device is None:
             _LOGGER.error("Initialization failed")
             raise AbortFlow("unknown_error")
 

homeassistant/components/homewizard/coordinator.py

@@ -50,11 +50,6 @@ class HWEnergyDeviceUpdateCoordinator(
                     "API disabled, API must be enabled in the app"
                 ) from ex
 
-            except Exception as ex:
-                raise UpdateFailed(
-                    f"Error connecting with Energy Device at {self.api.host}"
-                ) from ex
-
             data: DeviceResponseEntry = {
                 "device": self.api.device,
                 "data": {},

homeassistant/components/homewizard/sensor.py

@@ -166,7 +166,7 @@ class HWEnergySensor(CoordinatorEntity[DeviceResponseEntry], SensorEntity):
         self._attr_unique_id = f"{entry.unique_id}_{description.key}"
 
         # Special case for export, not everyone has solarpanels
-        # The change that 'export' is non-zero when you have solar panels is nil
+        # The chance that 'export' is non-zero when you have solar panels is nil
         if self.data_type in [
             "total_power_export_t1_kwh",
             "total_power_export_t2_kwh",
@@ -198,4 +198,4 @@ class HWEnergySensor(CoordinatorEntity[DeviceResponseEntry], SensorEntity):
     @property
     def available(self) -> bool:
         """Return availability of meter."""
-        return self.data_type in self.data["data"]
+        return super().available and self.data_type in self.data["data"]

tests/components/homewizard/test_config_flow.py

@@ -27,32 +27,25 @@ async def test_manual_flow_works(hass, aioclient_mock):
     assert result["type"] == "form"
     assert result["step_id"] == "user"
 
-    with patch(
+    with patch("aiohwenergy.HomeWizardEnergy", return_value=device,), patch(
-        "aiohwenergy.HomeWizardEnergy",
+        "homeassistant.components.homewizard.async_setup_entry",
-        return_value=device,
+        return_value=True,
-    ):
+    ) as mock_setup_entry:
         result = await hass.config_entries.flow.async_configure(
             result["flow_id"], {CONF_IP_ADDRESS: "2.2.2.2"}
         )
 
     assert result["type"] == "create_entry"
-
     assert result["title"] == f"{device.device.product_name} (aabbccddeeff)"
     assert result["data"][CONF_IP_ADDRESS] == "2.2.2.2"
 
-    with patch(
+    assert len(hass.config_entries.async_entries(DOMAIN)) == 1
-        "aiohwenergy.HomeWizardEnergy",
-        return_value=device,
-    ):
-        entries = hass.config_entries.async_entries(DOMAIN)
 
-    assert len(entries) == 1
+    assert len(device.initialize.mock_calls) == 1
-    entry = entries[0]
-    assert entry.unique_id == f"{device.device.product_type}_{device.device.serial}"
-
-    assert len(device.initialize.mock_calls) == 2
     assert len(device.close.mock_calls) == 1
 
+    assert len(mock_setup_entry.mock_calls) == 1
+
 
 async def test_discovery_flow_works(hass, aioclient_mock):
     """Test discovery setup flow works."""
@@ -72,10 +65,7 @@ async def test_discovery_flow_works(hass, aioclient_mock):
         },
     )
 
-    with patch("aiohwenergy.HomeWizardEnergy", return_value=get_mock_device()), patch(
+    with patch("aiohwenergy.HomeWizardEnergy", return_value=get_mock_device()):
-        "homeassistant.components.homewizard.async_setup_entry",
-        return_value=True,
-    ):
         flow = await hass.config_entries.flow.async_init(
             DOMAIN,
             context={"source": config_entries.SOURCE_ZEROCONF},
@@ -116,15 +106,11 @@ async def test_discovery_disabled_api(hass, aioclient_mock):
         },
     )
 
-    with patch("aiohwenergy.HomeWizardEnergy", return_value=get_mock_device()), patch(
+    result = await hass.config_entries.flow.async_init(
-        "homeassistant.components.homewizard.async_setup_entry",
+        DOMAIN,
-        return_value=True,
+        context={"source": config_entries.SOURCE_ZEROCONF},
-    ):
+        data=service_info,
-        result = await hass.config_entries.flow.async_init(
+    )
-            DOMAIN,
-            context={"source": config_entries.SOURCE_ZEROCONF},
-            data=service_info,
-        )
 
     assert result["type"] == RESULT_TYPE_ABORT
     assert result["reason"] == "api_not_enabled"
@@ -148,15 +134,11 @@ async def test_discovery_missing_data_in_service_info(hass, aioclient_mock):
         },
     )
 
-    with patch("aiohwenergy.HomeWizardEnergy", return_value=get_mock_device()), patch(
+    result = await hass.config_entries.flow.async_init(
-        "homeassistant.components.homewizard.async_setup_entry",
+        DOMAIN,
-        return_value=True,
+        context={"source": config_entries.SOURCE_ZEROCONF},
-    ):
+        data=service_info,
-        result = await hass.config_entries.flow.async_init(
+    )
-            DOMAIN,
-            context={"source": config_entries.SOURCE_ZEROCONF},
-            data=service_info,
-        )
 
     assert result["type"] == RESULT_TYPE_ABORT
     assert result["reason"] == "invalid_discovery_parameters"
@@ -180,15 +162,11 @@ async def test_discovery_invalid_api(hass, aioclient_mock):
         },
     )
 
-    with patch("aiohwenergy.HomeWizardEnergy", return_value=get_mock_device()), patch(
+    result = await hass.config_entries.flow.async_init(
-        "homeassistant.components.homewizard.async_setup_entry",
+        DOMAIN,
-        return_value=True,
+        context={"source": config_entries.SOURCE_ZEROCONF},
-    ):
+        data=service_info,
-        result = await hass.config_entries.flow.async_init(
+    )
-            DOMAIN,
-            context={"source": config_entries.SOURCE_ZEROCONF},
-            data=service_info,
-        )
 
     assert result["type"] == RESULT_TYPE_ABORT
     assert result["reason"] == "unsupported_api_version"
@@ -197,11 +175,11 @@ async def test_discovery_invalid_api(hass, aioclient_mock):
 async def test_check_disabled_api(hass, aioclient_mock):
     """Test check detecting disabled api."""
 
-    def MockInitialize():
+    def mock_initialize():
         raise DisabledError
 
     device = get_mock_device()
-    device.initialize.side_effect = MockInitialize
+    device.initialize.side_effect = mock_initialize
 
     result = await hass.config_entries.flow.async_init(
         DOMAIN, context={"source": config_entries.SOURCE_USER}
@@ -225,11 +203,11 @@ async def test_check_disabled_api(hass, aioclient_mock):
 async def test_check_error_handling_api(hass, aioclient_mock):
     """Test check detecting error with api."""
 
-    def MockInitialize():
+    def mock_initialize():
         raise Exception()
 
     device = get_mock_device()
-    device.initialize.side_effect = MockInitialize
+    device.initialize.side_effect = mock_initialize
 
     result = await hass.config_entries.flow.async_init(
         DOMAIN, context={"source": config_entries.SOURCE_USER}

tests/components/homewizard/test_coordinator.py

@@ -1,131 +0,0 @@
-"""Test the update coordinator for HomeWizard."""
-
-from datetime import timedelta
-import json
-from unittest.mock import AsyncMock, patch
-
-from aiohwenergy import errors
-from pytest import raises
-
-from homeassistant.components.homewizard.const import CONF_DATA, CONF_DEVICE
-from homeassistant.components.homewizard.coordinator import (
-    HWEnergyDeviceUpdateCoordinator as Coordinator,
-)
-from homeassistant.helpers.update_coordinator import UpdateFailed
-
-from .generator import get_mock_device
-
-
-async def test_coordinator_sets_update_interval(aioclient_mock, hass):
-    """Test coordinator calculates correct update interval."""
-
-    # P1 meter
-    meter = get_mock_device(product_type="p1_meter")
-
-    coordinator = Coordinator(hass, meter)
-    assert coordinator.update_interval == timedelta(seconds=5)
-
-
-def mock_request_response(
-    status: int, data: str, content_type: str = "application/json"
-):
-    """Return the default mocked config entry data."""
-
-    mock_response = AsyncMock()
-    mock_response.status = status
-    mock_response.content_type = content_type
-
-    async def return_json():
-        return json.loads(data)
-
-    async def return_text(format: str):
-        return data
-
-    mock_response.json = return_json
-    mock_response.text = return_text
-
-    return mock_response
-
-
-async def test_coordinator_fetches_data(aioclient_mock, hass):
-    """Test coordinator fetches data."""
-
-    # P1 meter and (very advanced kWh meter)
-    meter = get_mock_device(product_type="p1_meter")
-    meter.data.smr_version = 50
-    meter.data.available_datapoints = [
-        "active_power_l1_w",
-        "active_power_l2_w",
-        "active_power_l3_w",
-        "active_power_w",
-        "meter_model",
-        "smr_version",
-        "total_power_export_t1_kwh",
-        "total_power_export_t2_kwh",
-        "total_power_import_t1_kwh",
-        "total_power_import_t2_kwh",
-        "total_gas_m3",
-        "wifi_ssid",
-        "wifi_strength",
-    ]
-
-    coordinator = Coordinator(hass, "1.2.3.4")
-    coordinator.api = meter
-    data = await coordinator._async_update_data()
-
-    print(data[CONF_DEVICE])
-    print(meter.device.product_type)
-    assert data[CONF_DEVICE] == meter.device
-    assert coordinator.api.host == "1.2.3.4"
-    assert coordinator.api == meter
-
-    assert (
-        len(coordinator.api.initialize.mock_calls) == 0
-    )  # Already initialized by 'coordinator.api = meter'
-    assert len(coordinator.api.update.mock_calls) == 2  # Init and update
-    assert len(coordinator.api.close.mock_calls) == 0
-
-    for datapoint in meter.data.available_datapoints:
-        assert datapoint in data[CONF_DATA]
-
-
-async def test_coordinator_failed_to_update(aioclient_mock, hass):
-    """Test coordinator handles failed update correctly."""
-
-    # Update failed by internal error
-    meter = get_mock_device(product_type="p1_meter")
-
-    async def _failed_update() -> bool:
-        return False
-
-    meter.update = _failed_update
-
-    with patch(
-        "aiohwenergy.HomeWizardEnergy",
-        return_value=meter,
-    ):
-        coordinator = Coordinator(hass, "1.2.3.4")
-
-    with raises(UpdateFailed):
-        await coordinator._async_update_data()
-
-
-async def test_coordinator_detected_disabled_api(aioclient_mock, hass):
-    """Test coordinator handles disabled api correctly."""
-
-    # Update failed by internal error
-    meter = get_mock_device(product_type="p1_meter")
-
-    async def _failed_update() -> bool:
-        raise errors.DisabledError()
-
-    meter.update = _failed_update
-
-    with patch(
-        "aiohwenergy.HomeWizardEnergy",
-        return_value=meter,
-    ):
-        coordinator = Coordinator(hass, "1.2.3.4")
-
-    with raises(UpdateFailed):
-        await coordinator._async_update_data()

tests/components/homewizard/test_sensor.py

@@ -1,6 +1,9 @@
 """Test the update coordinator for HomeWizard."""
 
-from unittest.mock import patch
+from datetime import timedelta
+from unittest.mock import AsyncMock, patch
+
+from aiohwenergy.errors import DisabledError
 
 from homeassistant.components.sensor import (
     ATTR_STATE_CLASS,
@@ -20,9 +23,12 @@ from homeassistant.const import (
     VOLUME_CUBIC_METERS,
 )
 from homeassistant.helpers import entity_registry as er
+import homeassistant.util.dt as dt_util
 
 from .generator import get_mock_device
 
+from tests.common import async_fire_time_changed
+
 
 async def test_sensor_entity_smr_version(
     hass, mock_config_entry_data, mock_config_entry
@@ -637,3 +643,105 @@ async def test_sensor_entity_export_disabled_when_unused(
     )
     assert entry
     assert entry.disabled
+
+
+async def test_sensors_unreachable(hass, mock_config_entry_data, mock_config_entry):
+    """Test sensor handles api unreachable."""
+
+    api = get_mock_device()
+    api.data.available_datapoints = [
+        "total_power_import_t1_kwh",
+    ]
+    api.data.total_power_import_t1_kwh = 1234.123
+
+    with patch(
+        "aiohwenergy.HomeWizardEnergy",
+        return_value=api,
+    ):
+        api.update = AsyncMock(return_value=True)
+
+        entry = mock_config_entry
+        entry.data = mock_config_entry_data
+        entry.add_to_hass(hass)
+        utcnow = dt_util.utcnow()
+
+        await hass.config_entries.async_setup(entry.entry_id)
+        await hass.async_block_till_done()
+
+        assert (
+            hass.states.get(
+                "sensor.product_name_aabbccddeeff_total_power_import_t1"
+            ).state
+            == "1234.123"
+        )
+
+        api.update = AsyncMock(return_value=False)
+        async_fire_time_changed(hass, utcnow + timedelta(seconds=5))
+        await hass.async_block_till_done()
+        assert (
+            hass.states.get(
+                "sensor.product_name_aabbccddeeff_total_power_import_t1"
+            ).state
+            == "unavailable"
+        )
+
+        api.update = AsyncMock(return_value=True)
+        async_fire_time_changed(hass, utcnow + timedelta(seconds=10))
+        await hass.async_block_till_done()
+        assert (
+            hass.states.get(
+                "sensor.product_name_aabbccddeeff_total_power_import_t1"
+            ).state
+            == "1234.123"
+        )
+
+
+async def test_api_disabled(hass, mock_config_entry_data, mock_config_entry):
+    """Test sensor handles api unreachable."""
+
+    api = get_mock_device()
+    api.data.available_datapoints = [
+        "total_power_import_t1_kwh",
+    ]
+    api.data.total_power_import_t1_kwh = 1234.123
+
+    with patch(
+        "aiohwenergy.HomeWizardEnergy",
+        return_value=api,
+    ):
+        api.update = AsyncMock(return_value=True)
+
+        entry = mock_config_entry
+        entry.data = mock_config_entry_data
+        entry.add_to_hass(hass)
+
+        await hass.config_entries.async_setup(entry.entry_id)
+        await hass.async_block_till_done()
+        utcnow = dt_util.utcnow()
+
+        assert (
+            hass.states.get(
+                "sensor.product_name_aabbccddeeff_total_power_import_t1"
+            ).state
+            == "1234.123"
+        )
+
+        api.update = AsyncMock(side_effect=DisabledError)
+        async_fire_time_changed(hass, utcnow + timedelta(seconds=5))
+        await hass.async_block_till_done()
+        assert (
+            hass.states.get(
+                "sensor.product_name_aabbccddeeff_total_power_import_t1"
+            ).state
+            == "unavailable"
+        )
+
+        api.update = AsyncMock(return_value=True)
+        async_fire_time_changed(hass, utcnow + timedelta(seconds=10))
+        await hass.async_block_till_done()
+        assert (
+            hass.states.get(
+                "sensor.product_name_aabbccddeeff_total_power_import_t1"
+            ).state
+            == "1234.123"
+        )