diff --git a/homeassistant/components/opower/manifest.json b/homeassistant/components/opower/manifest.json index a6409dfab79..5cb652f629a 100644 --- a/homeassistant/components/opower/manifest.json +++ b/homeassistant/components/opower/manifest.json @@ -8,6 +8,6 @@ "integration_type": "service", "iot_class": "cloud_polling", "loggers": ["opower"], - "quality_scale": "silver", + "quality_scale": "platinum", "requirements": ["opower==0.17.0"] } diff --git a/homeassistant/components/opower/quality_scale.yaml b/homeassistant/components/opower/quality_scale.yaml index 112999a2ef2..c51fa99c8ff 100644 --- a/homeassistant/components/opower/quality_scale.yaml +++ b/homeassistant/components/opower/quality_scale.yaml @@ -58,7 +58,7 @@ rules: docs-supported-functions: done docs-troubleshooting: done docs-use-cases: done - dynamic-devices: todo + dynamic-devices: done entity-category: done entity-device-class: done entity-disabled-by-default: done @@ -71,7 +71,7 @@ rules: status: exempt comment: The integration has no user-configurable options that are not authentication-related. repair-issues: done - stale-devices: todo + stale-devices: done # Platinum async-dependency: done diff --git a/homeassistant/components/opower/sensor.py b/homeassistant/components/opower/sensor.py index 72dccb1eebf..08341146a48 100644 --- a/homeassistant/components/opower/sensor.py +++ b/homeassistant/components/opower/sensor.py @@ -15,7 +15,8 @@ from homeassistant.components.sensor import ( SensorStateClass, ) from homeassistant.const import EntityCategory, UnitOfEnergy, UnitOfVolume -from homeassistant.core import HomeAssistant +from homeassistant.core import HomeAssistant, callback +from homeassistant.helpers import device_registry as dr, entity_registry as er from homeassistant.helpers.device_registry import DeviceEntryType, DeviceInfo from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback from homeassistant.helpers.typing import StateType @@ -207,48 +208,102 @@ async def async_setup_entry( async_add_entities: AddConfigEntryEntitiesCallback, ) -> None: """Set up the Opower sensor.""" - coordinator = entry.runtime_data - entities: list[OpowerSensor] = [] - opower_data_list = coordinator.data.values() - for opower_data in opower_data_list: - account = opower_data.account - forecast = opower_data.forecast - device_id = ( - f"{coordinator.api.utility.subdomain()}_{account.utility_account_id}" - ) - device = DeviceInfo( - identifiers={(DOMAIN, device_id)}, - name=f"{account.meter_type.name} account {account.utility_account_id}", - manufacturer="Opower", - model=coordinator.api.utility.name(), - entry_type=DeviceEntryType.SERVICE, - ) - sensors: tuple[OpowerEntityDescription, ...] = COMMON_SENSORS - if ( - account.meter_type == MeterType.ELEC - and forecast is not None - and forecast.unit_of_measure == UnitOfMeasure.KWH - ): - sensors += ELEC_SENSORS - elif ( - account.meter_type == MeterType.GAS - and forecast is not None - and forecast.unit_of_measure in [UnitOfMeasure.THERM, UnitOfMeasure.CCF] - ): - sensors += GAS_SENSORS - entities.extend( - OpowerSensor( - coordinator, - sensor, - account.utility_account_id, - device, - device_id, - ) - for sensor in sensors - ) + created_sensors: set[tuple[str, str]] = set() - async_add_entities(entities) + @callback + def _update_entities() -> None: + """Update entities.""" + new_entities: list[OpowerSensor] = [] + current_account_device_ids: set[str] = set() + current_account_ids: set[str] = set() + + for opower_data in coordinator.data.values(): + account = opower_data.account + forecast = opower_data.forecast + device_id = ( + f"{coordinator.api.utility.subdomain()}_{account.utility_account_id}" + ) + current_account_device_ids.add(device_id) + current_account_ids.add(account.utility_account_id) + device = DeviceInfo( + identifiers={(DOMAIN, device_id)}, + name=f"{account.meter_type.name} account {account.utility_account_id}", + manufacturer="Opower", + model=coordinator.api.utility.name(), + entry_type=DeviceEntryType.SERVICE, + ) + sensors: tuple[OpowerEntityDescription, ...] = COMMON_SENSORS + if ( + account.meter_type == MeterType.ELEC + and forecast is not None + and forecast.unit_of_measure == UnitOfMeasure.KWH + ): + sensors += ELEC_SENSORS + elif ( + account.meter_type == MeterType.GAS + and forecast is not None + and forecast.unit_of_measure in [UnitOfMeasure.THERM, UnitOfMeasure.CCF] + ): + sensors += GAS_SENSORS + for sensor in sensors: + sensor_key = (account.utility_account_id, sensor.key) + if sensor_key in created_sensors: + continue + created_sensors.add(sensor_key) + new_entities.append( + OpowerSensor( + coordinator, + sensor, + account.utility_account_id, + device, + device_id, + ) + ) + + if new_entities: + async_add_entities(new_entities) + + # Remove any registered devices not in the current coordinator data + device_registry = dr.async_get(hass) + entity_registry = er.async_get(hass) + for device_entry in dr.async_entries_for_config_entry( + device_registry, entry.entry_id + ): + device_domain_ids = { + identifier[1] + for identifier in device_entry.identifiers + if identifier[0] == DOMAIN + } + if not device_domain_ids: + # This device has no Opower identifiers; it may be a merged/shared + # device owned by another integration. Do not alter it here. + continue + if not device_domain_ids.isdisjoint(current_account_device_ids): + continue # device is still active + # Device is stale — remove its entities then detach it + for entity_entry in er.async_entries_for_device( + entity_registry, device_entry.id, include_disabled_entities=True + ): + if entity_entry.config_entry_id != entry.entry_id: + continue + entity_registry.async_remove(entity_entry.entity_id) + device_registry.async_update_device( + device_entry.id, remove_config_entry_id=entry.entry_id + ) + + # Prune sensor tracking for accounts that are no longer present + if created_sensors: + stale_sensor_keys = { + sensor_key + for sensor_key in created_sensors + if sensor_key[0] not in current_account_ids + } + if stale_sensor_keys: + created_sensors.difference_update(stale_sensor_keys) + + _update_entities() + entry.async_on_unload(coordinator.async_add_listener(_update_entities)) class OpowerSensor(CoordinatorEntity[OpowerCoordinator], SensorEntity): @@ -272,6 +327,11 @@ class OpowerSensor(CoordinatorEntity[OpowerCoordinator], SensorEntity): self._attr_device_info = device self.utility_account_id = utility_account_id + @property + def available(self) -> bool: + """Return if entity is available.""" + return super().available and self.utility_account_id in self.coordinator.data + @property def native_value(self) -> StateType | date | datetime: """Return the state.""" diff --git a/tests/components/opower/test_sensor.py b/tests/components/opower/test_sensor.py index 25df9cbcdb6..147ca6e55bb 100644 --- a/tests/components/opower/test_sensor.py +++ b/tests/components/opower/test_sensor.py @@ -6,10 +6,11 @@ from unittest.mock import AsyncMock, patch from opower import CostRead import pytest +from homeassistant.components.opower.const import DOMAIN from homeassistant.components.recorder import Recorder from homeassistant.const import ATTR_UNIT_OF_MEASUREMENT, UnitOfEnergy, UnitOfVolume from homeassistant.core import HomeAssistant -from homeassistant.helpers import entity_registry as er +from homeassistant.helpers import device_registry as dr, entity_registry as er from homeassistant.util import dt as dt_util from tests.common import MockConfigEntry @@ -114,3 +115,111 @@ async def test_sensors( state = hass.states.get("sensor.gas_account_222222_last_updated") assert state assert state.state == "2023-01-02T08:00:00+00:00" + + +async def test_dynamic_and_stale_devices( + recorder_mock: Recorder, + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_opower_api: AsyncMock, + device_registry: dr.DeviceRegistry, + entity_registry: er.EntityRegistry, +) -> None: + """Test the dynamic addition and removal of Opower devices.""" + original_accounts = mock_opower_api.async_get_accounts.return_value + original_forecasts = mock_opower_api.async_get_forecast.return_value + + await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + devices = dr.async_entries_for_config_entry( + device_registry, mock_config_entry.entry_id + ) + entities = er.async_entries_for_config_entry( + entity_registry, mock_config_entry.entry_id + ) + initial_device_ids = {device.id for device in devices} + initial_entity_ids = {entity.entity_id for entity in entities} + # Ensure we actually created some devices and entities for this entry + assert initial_device_ids + assert initial_entity_ids + + # Remove the second account and update data + mock_opower_api.async_get_accounts.return_value = [original_accounts[0]] + mock_opower_api.async_get_forecast.return_value = [original_forecasts[0]] + + coordinator = mock_config_entry.runtime_data + await coordinator.async_refresh() + await hass.async_block_till_done() + + devices = dr.async_entries_for_config_entry( + device_registry, mock_config_entry.entry_id + ) + entities = er.async_entries_for_config_entry( + entity_registry, mock_config_entry.entry_id + ) + device_ids_after_removal = {device.id for device in devices} + entity_ids_after_removal = {entity.entity_id for entity in entities} + # After removing one account, we should have removed some devices/entities + # but not added any new ones. + assert device_ids_after_removal <= initial_device_ids + assert entity_ids_after_removal <= initial_entity_ids + assert device_ids_after_removal != initial_device_ids + assert entity_ids_after_removal != initial_entity_ids + + # Add back the second account + mock_opower_api.async_get_accounts.return_value = original_accounts + mock_opower_api.async_get_forecast.return_value = original_forecasts + + await coordinator.async_refresh() + await hass.async_block_till_done() + + devices = dr.async_entries_for_config_entry( + device_registry, mock_config_entry.entry_id + ) + entities = er.async_entries_for_config_entry( + entity_registry, mock_config_entry.entry_id + ) + device_ids_after_restore = {device.id for device in devices} + entity_ids_after_restore = {entity.entity_id for entity in entities} + # After restoring the second account, we should be back to the original + # number of devices and entities (IDs themselves may change on re-create). + assert len(device_ids_after_restore) == len(initial_device_ids) + assert len(entity_ids_after_restore) == len(initial_entity_ids) + + +async def test_stale_device_removed_on_load( + recorder_mock: Recorder, + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_opower_api: AsyncMock, + device_registry: dr.DeviceRegistry, +) -> None: + """Test that a stale device present before setup is removed on first load.""" + # Simulate a device that was created by a previous version / old account + # and is already registered before the integration sets up. + stale_device = device_registry.async_get_or_create( + config_entry_id=mock_config_entry.entry_id, + identifiers={(DOMAIN, "pge_stale_account_99999")}, + ) + assert device_registry.async_get(stale_device.id) is not None + + await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + # Stale device should have been removed on first coordinator update + assert device_registry.async_get(stale_device.id) is None + + # Active devices for known accounts should still be present, + # and the stale identifier should no longer be registered. + active_devices = dr.async_entries_for_config_entry( + device_registry, mock_config_entry.entry_id + ) + active_identifiers = { + identifier + for device in active_devices + for (_domain, identifier) in device.identifiers + } + assert "pge_111111" in active_identifiers + assert "pge_222222" in active_identifiers + assert "pge_stale_account_99999" not in active_identifiers