From f7f0f490154eebc7ce43779456e89e3f162df0bb Mon Sep 17 00:00:00 2001 From: Michael <35783820+mib1185@users.noreply.github.com> Date: Wed, 31 Jul 2024 15:36:57 +0200 Subject: [PATCH] Move lifespan attributes into own sensors for legacy Ecovacs bots (#122740) * move available property to base entity class * add lifespan sensors * apply suggestion, simplify the method * don't touch internals in tests * apply suggestion * apply suggestions --- homeassistant/components/ecovacs/const.py | 6 + .../components/ecovacs/controller.py | 10 ++ homeassistant/components/ecovacs/entity.py | 5 + homeassistant/components/ecovacs/sensor.py | 84 +++++++++- homeassistant/components/ecovacs/strings.json | 3 + homeassistant/components/ecovacs/vacuum.py | 6 +- tests/components/ecovacs/conftest.py | 11 +- .../ecovacs/snapshots/test_sensor.ambr | 148 ++++++++++++++++++ tests/components/ecovacs/test_sensor.py | 33 ++++ 9 files changed, 294 insertions(+), 12 deletions(-) diff --git a/homeassistant/components/ecovacs/const.py b/homeassistant/components/ecovacs/const.py index 65044c016f9..ac7a268f1bd 100644 --- a/homeassistant/components/ecovacs/const.py +++ b/homeassistant/components/ecovacs/const.py @@ -21,6 +21,12 @@ SUPPORTED_LIFESPANS = ( LifeSpan.ROUND_MOP, ) +LEGACY_SUPPORTED_LIFESPANS = ( + "main_brush", + "side_brush", + "filter", +) + class InstanceMode(StrEnum): """Instance mode.""" diff --git a/homeassistant/components/ecovacs/controller.py b/homeassistant/components/ecovacs/controller.py index 0bef2e8fdd7..c22fb240536 100644 --- a/homeassistant/components/ecovacs/controller.py +++ b/homeassistant/components/ecovacs/controller.py @@ -74,6 +74,8 @@ class EcovacsController: self._authenticator, ) + self._added_legacy_entities: set[str] = set() + async def initialize(self) -> None: """Init controller.""" mqtt_config_verfied = False @@ -117,6 +119,14 @@ class EcovacsController: await self._mqtt.disconnect() await self._authenticator.teardown() + def add_legacy_entity(self, device: VacBot, component: str) -> None: + """Add legacy entity.""" + self._added_legacy_entities.add(f"{device.vacuum['did']}_{component}") + + def legacy_entity_is_added(self, device: VacBot, component: str) -> bool: + """Check if legacy entity is added.""" + return f"{device.vacuum['did']}_{component}" in self._added_legacy_entities + @property def devices(self) -> list[Device]: """Return devices.""" diff --git a/homeassistant/components/ecovacs/entity.py b/homeassistant/components/ecovacs/entity.py index 9d3092f37b4..36103be4d11 100644 --- a/homeassistant/components/ecovacs/entity.py +++ b/homeassistant/components/ecovacs/entity.py @@ -150,6 +150,11 @@ class EcovacsLegacyEntity(Entity): self._event_listeners: list[EventListener] = [] + @property + def available(self) -> bool: + """Return True if the entity is available.""" + return super().available and self.state is not None + async def async_will_remove_from_hass(self) -> None: """Remove event listeners on entity remove.""" for listener in self._event_listeners: diff --git a/homeassistant/components/ecovacs/sensor.py b/homeassistant/components/ecovacs/sensor.py index 256198693fb..28c4efbd0c6 100644 --- a/homeassistant/components/ecovacs/sensor.py +++ b/homeassistant/components/ecovacs/sensor.py @@ -4,7 +4,7 @@ from __future__ import annotations from collections.abc import Callable from dataclasses import dataclass -from typing import Generic +from typing import Any, Generic from deebot_client.capabilities import CapabilityEvent, CapabilityLifeSpan from deebot_client.events import ( @@ -17,6 +17,7 @@ from deebot_client.events import ( StatsEvent, TotalStatsEvent, ) +from sucks import VacBot from homeassistant.components.sensor import ( SensorDeviceClass, @@ -37,11 +38,12 @@ from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.typing import StateType from . import EcovacsConfigEntry -from .const import SUPPORTED_LIFESPANS +from .const import LEGACY_SUPPORTED_LIFESPANS, SUPPORTED_LIFESPANS from .entity import ( EcovacsCapabilityEntityDescription, EcovacsDescriptionEntity, EcovacsEntity, + EcovacsLegacyEntity, EventT, ) from .util import get_supported_entitites @@ -158,6 +160,25 @@ LIFESPAN_ENTITY_DESCRIPTIONS = tuple( ) +@dataclass(kw_only=True, frozen=True) +class EcovacsLegacyLifespanSensorEntityDescription(SensorEntityDescription): + """Ecovacs lifespan sensor entity description.""" + + component: str + + +LEGACY_LIFESPAN_SENSORS = tuple( + EcovacsLegacyLifespanSensorEntityDescription( + component=component, + key=f"lifespan_{component}", + translation_key=f"lifespan_{component}", + native_unit_of_measurement=PERCENTAGE, + entity_category=EntityCategory.DIAGNOSTIC, + ) + for component in LEGACY_SUPPORTED_LIFESPANS +) + + async def async_setup_entry( hass: HomeAssistant, config_entry: EcovacsConfigEntry, @@ -183,6 +204,32 @@ async def async_setup_entry( async_add_entities(entities) + async def _add_legacy_entities() -> None: + entities = [] + for device in controller.legacy_devices: + for description in LEGACY_LIFESPAN_SENSORS: + if ( + description.component in device.components + and not controller.legacy_entity_is_added( + device, description.component + ) + ): + controller.add_legacy_entity(device, description.component) + entities.append(EcovacsLegacyLifespanSensor(device, description)) + + if entities: + async_add_entities(entities) + + def _fire_ecovacs_legacy_lifespan_event(_: Any) -> None: + hass.create_task(_add_legacy_entities()) + + for device in controller.legacy_devices: + config_entry.async_on_unload( + device.lifespanEvents.subscribe( + _fire_ecovacs_legacy_lifespan_event + ).unsubscribe + ) + class EcovacsSensor( EcovacsDescriptionEntity[CapabilityEvent], @@ -253,3 +300,36 @@ class EcovacsErrorSensor( self.async_write_ha_state() self._subscribe(self._capability.event, on_event) + + +class EcovacsLegacyLifespanSensor(EcovacsLegacyEntity, SensorEntity): + """Legacy Lifespan sensor.""" + + entity_description: EcovacsLegacyLifespanSensorEntityDescription + + def __init__( + self, + device: VacBot, + description: EcovacsLegacyLifespanSensorEntityDescription, + ) -> None: + """Initialize the entity.""" + super().__init__(device) + self.entity_description = description + self._attr_unique_id = f"{device.vacuum['did']}_{description.key}" + + if (value := device.components.get(description.component)) is not None: + value = int(value * 100) + self._attr_native_value = value + + async def async_added_to_hass(self) -> None: + """Set up the event listeners now that hass is ready.""" + + def on_event(_: Any) -> None: + if ( + value := self.device.components.get(self.entity_description.component) + ) is not None: + value = int(value * 100) + self._attr_native_value = value + self.schedule_update_ha_state() + + self._event_listeners.append(self.device.lifespanEvents.subscribe(on_event)) diff --git a/homeassistant/components/ecovacs/strings.json b/homeassistant/components/ecovacs/strings.json index d2e385c79c7..ea216f11694 100644 --- a/homeassistant/components/ecovacs/strings.json +++ b/homeassistant/components/ecovacs/strings.json @@ -119,6 +119,9 @@ "lifespan_lens_brush": { "name": "Lens brush lifespan" }, + "lifespan_main_brush": { + "name": "[%key:component::ecovacs::entity::sensor::lifespan_brush::name%]" + }, "lifespan_side_brush": { "name": "Side brush lifespan" }, diff --git a/homeassistant/components/ecovacs/vacuum.py b/homeassistant/components/ecovacs/vacuum.py index d28e632580f..e690038ff71 100644 --- a/homeassistant/components/ecovacs/vacuum.py +++ b/homeassistant/components/ecovacs/vacuum.py @@ -142,11 +142,6 @@ class EcovacsLegacyVacuum(EcovacsLegacyEntity, StateVacuumEntity): return None - @property - def available(self) -> bool: - """Return True if the vacuum is available.""" - return super().available and self.state is not None - @property def battery_level(self) -> int | None: """Return the battery level of the vacuum cleaner.""" @@ -173,6 +168,7 @@ class EcovacsLegacyVacuum(EcovacsLegacyEntity, StateVacuumEntity): data: dict[str, Any] = {} data[ATTR_ERROR] = self.error + # these attributes are deprecated and can be removed in 2025.2 for key, val in self.device.components.items(): attr_name = ATTR_COMPONENT_PREFIX + key data[attr_name] = int(val * 100) diff --git a/tests/components/ecovacs/conftest.py b/tests/components/ecovacs/conftest.py index e53cfcc8a3d..22039d6c0bc 100644 --- a/tests/components/ecovacs/conftest.py +++ b/tests/components/ecovacs/conftest.py @@ -10,6 +10,7 @@ from deebot_client.device import Device from deebot_client.exceptions import ApiError from deebot_client.models import Credentials import pytest +from sucks import EventEmitter from homeassistant.components.ecovacs import PLATFORMS from homeassistant.components.ecovacs.const import DOMAIN @@ -128,10 +129,10 @@ def mock_vacbot(device_fixture: str) -> Generator[Mock]: vacbot.vacuum = load_json_object_fixture( f"devices/{device_fixture}/device.json", DOMAIN ) - vacbot.statusEvents = Mock() - vacbot.batteryEvents = Mock() - vacbot.lifespanEvents = Mock() - vacbot.errorEvents = Mock() + vacbot.statusEvents = EventEmitter() + vacbot.batteryEvents = EventEmitter() + vacbot.lifespanEvents = EventEmitter() + vacbot.errorEvents = EventEmitter() vacbot.battery_status = None vacbot.fan_speed = None vacbot.components = {} @@ -175,7 +176,7 @@ async def init_integration( mock_config_entry.add_to_hass(hass) await hass.config_entries.async_setup(mock_config_entry.entry_id) - await hass.async_block_till_done() + await hass.async_block_till_done(wait_background_tasks=True) yield mock_config_entry diff --git a/tests/components/ecovacs/snapshots/test_sensor.ambr b/tests/components/ecovacs/snapshots/test_sensor.ambr index 07ebd400870..659edfde2cf 100644 --- a/tests/components/ecovacs/snapshots/test_sensor.ambr +++ b/tests/components/ecovacs/snapshots/test_sensor.ambr @@ -1,4 +1,152 @@ # serializer version: 1 +# name: test_legacy_sensors[123][sensor.e1234567890000000003_filter_lifespan:entity-registry] + EntityRegistryEntrySnapshot({ + 'aliases': set({ + }), + 'area_id': None, + 'capabilities': None, + 'config_entry_id': , + 'device_class': None, + 'device_id': , + 'disabled_by': None, + 'domain': 'sensor', + 'entity_category': , + 'entity_id': 'sensor.e1234567890000000003_filter_lifespan', + 'has_entity_name': True, + 'hidden_by': None, + 'icon': None, + 'id': , + 'labels': set({ + }), + 'name': None, + 'options': dict({ + }), + 'original_device_class': None, + 'original_icon': None, + 'original_name': 'Filter lifespan', + 'platform': 'ecovacs', + 'previous_unique_id': None, + 'supported_features': 0, + 'translation_key': 'lifespan_filter', + 'unique_id': 'E1234567890000000003_lifespan_filter', + 'unit_of_measurement': '%', + }) +# --- +# name: test_legacy_sensors[123][sensor.e1234567890000000003_filter_lifespan:state] + StateSnapshot({ + 'attributes': ReadOnlyDict({ + 'friendly_name': 'E1234567890000000003 Filter lifespan', + 'unit_of_measurement': '%', + }), + 'context': , + 'entity_id': 'sensor.e1234567890000000003_filter_lifespan', + 'last_changed': , + 'last_reported': , + 'last_updated': , + 'state': '40', + }) +# --- +# name: test_legacy_sensors[123][sensor.e1234567890000000003_main_brush_lifespan:entity-registry] + EntityRegistryEntrySnapshot({ + 'aliases': set({ + }), + 'area_id': None, + 'capabilities': None, + 'config_entry_id': , + 'device_class': None, + 'device_id': , + 'disabled_by': None, + 'domain': 'sensor', + 'entity_category': , + 'entity_id': 'sensor.e1234567890000000003_main_brush_lifespan', + 'has_entity_name': True, + 'hidden_by': None, + 'icon': None, + 'id': , + 'labels': set({ + }), + 'name': None, + 'options': dict({ + }), + 'original_device_class': None, + 'original_icon': None, + 'original_name': 'Main brush lifespan', + 'platform': 'ecovacs', + 'previous_unique_id': None, + 'supported_features': 0, + 'translation_key': 'lifespan_main_brush', + 'unique_id': 'E1234567890000000003_lifespan_main_brush', + 'unit_of_measurement': '%', + }) +# --- +# name: test_legacy_sensors[123][sensor.e1234567890000000003_main_brush_lifespan:state] + StateSnapshot({ + 'attributes': ReadOnlyDict({ + 'friendly_name': 'E1234567890000000003 Main brush lifespan', + 'unit_of_measurement': '%', + }), + 'context': , + 'entity_id': 'sensor.e1234567890000000003_main_brush_lifespan', + 'last_changed': , + 'last_reported': , + 'last_updated': , + 'state': '80', + }) +# --- +# name: test_legacy_sensors[123][sensor.e1234567890000000003_side_brush_lifespan:entity-registry] + EntityRegistryEntrySnapshot({ + 'aliases': set({ + }), + 'area_id': None, + 'capabilities': None, + 'config_entry_id': , + 'device_class': None, + 'device_id': , + 'disabled_by': None, + 'domain': 'sensor', + 'entity_category': , + 'entity_id': 'sensor.e1234567890000000003_side_brush_lifespan', + 'has_entity_name': True, + 'hidden_by': None, + 'icon': None, + 'id': , + 'labels': set({ + }), + 'name': None, + 'options': dict({ + }), + 'original_device_class': None, + 'original_icon': None, + 'original_name': 'Side brush lifespan', + 'platform': 'ecovacs', + 'previous_unique_id': None, + 'supported_features': 0, + 'translation_key': 'lifespan_side_brush', + 'unique_id': 'E1234567890000000003_lifespan_side_brush', + 'unit_of_measurement': '%', + }) +# --- +# name: test_legacy_sensors[123][sensor.e1234567890000000003_side_brush_lifespan:state] + StateSnapshot({ + 'attributes': ReadOnlyDict({ + 'friendly_name': 'E1234567890000000003 Side brush lifespan', + 'unit_of_measurement': '%', + }), + 'context': , + 'entity_id': 'sensor.e1234567890000000003_side_brush_lifespan', + 'last_changed': , + 'last_reported': , + 'last_updated': , + 'state': '60', + }) +# --- +# name: test_legacy_sensors[123][states] + list([ + 'sensor.e1234567890000000003_main_brush_lifespan', + 'sensor.e1234567890000000003_side_brush_lifespan', + 'sensor.e1234567890000000003_filter_lifespan', + ]) +# --- # name: test_sensors[5xu9h3][sensor.goat_g1_area_cleaned:entity-registry] EntityRegistryEntrySnapshot({ 'aliases': set({ diff --git a/tests/components/ecovacs/test_sensor.py b/tests/components/ecovacs/test_sensor.py index 19b4c8ce09b..53c57999776 100644 --- a/tests/components/ecovacs/test_sensor.py +++ b/tests/components/ecovacs/test_sensor.py @@ -1,5 +1,7 @@ """Tests for Ecovacs sensors.""" +from unittest.mock import Mock + from deebot_client.event_bus import EventBus from deebot_client.events import ( BatteryEvent, @@ -152,3 +154,34 @@ async def test_disabled_by_default_sensors( ), f"Entity registry entry for {entity_id} is missing" assert entry.disabled assert entry.disabled_by is er.RegistryEntryDisabler.INTEGRATION + + +@pytest.mark.usefixtures( + "entity_registry_enabled_by_default", "mock_vacbot", "init_integration" +) +@pytest.mark.parametrize(("device_fixture"), ["123"]) +async def test_legacy_sensors( + hass: HomeAssistant, + device_registry: dr.DeviceRegistry, + entity_registry: er.EntityRegistry, + snapshot: SnapshotAssertion, + mock_vacbot: Mock, +) -> None: + """Test that sensor entity snapshots match.""" + mock_vacbot.components = {"main_brush": 0.8, "side_brush": 0.6, "filter": 0.4} + mock_vacbot.lifespanEvents.notify("dummy_data") + await hass.async_block_till_done(wait_background_tasks=True) + + states = hass.states.async_entity_ids() + assert snapshot(name="states") == states + + for entity_id in hass.states.async_entity_ids(): + assert (state := hass.states.get(entity_id)), f"State of {entity_id} is missing" + assert snapshot(name=f"{entity_id}:state") == state + + assert (entity_entry := entity_registry.async_get(state.entity_id)) + assert snapshot(name=f"{entity_id}:entity-registry") == entity_entry + + assert entity_entry.device_id + assert (device_entry := device_registry.async_get(entity_entry.device_id)) + assert device_entry.identifiers == {(DOMAIN, "E1234567890000000003")}