Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate tplink alarm button entities #126349

Merged
merged 6 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions homeassistant/components/tplink/binary_sensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ async def async_setup_entry(
device = parent_coordinator.device

entities = CoordinatedTPLinkFeatureEntity.entities_for_device_and_its_children(
hass=hass,
device=device,
coordinator=parent_coordinator,
feature_type=Feature.Type.BinarySensor,
Expand Down
20 changes: 19 additions & 1 deletion homeassistant/components/tplink/button.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,17 @@

from kasa import Feature

from homeassistant.components.button import ButtonEntity, ButtonEntityDescription
from homeassistant.components.button import (
DOMAIN as BUTTON_DOMAIN,
ButtonEntity,
ButtonEntityDescription,
)
from homeassistant.components.siren import DOMAIN as SIREN_DOMAIN
from homeassistant.core import HomeAssistant
from homeassistant.helpers.entity_platform import AddEntitiesCallback

from . import TPLinkConfigEntry
from .deprecate import DeprecatedInfo, async_cleanup_deprecated
from .entity import CoordinatedTPLinkFeatureEntity, TPLinkFeatureEntityDescription


Expand All @@ -25,9 +31,19 @@ class TPLinkButtonEntityDescription(
BUTTON_DESCRIPTIONS: Final = [
TPLinkButtonEntityDescription(
key="test_alarm",
deprecated_info=DeprecatedInfo(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw that you made the PR #125594 but for me it just seems overly complicated?
Should/Could async_check_create_deprecated not be a simple helper function which you can run the entities through to deprecate them instead of moving it into the entity description and thus making it more complex (which I don't think is needed)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created home-assistant/architecture#1133 because I think this is a common pattern that could/should be provided by core.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know. My question is still why is this not just a simple helper instead of building this complex thing?
As that discussion is not close to be finalized I think we still need to address it here as this PR is ongoing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved reply to comment below

Copy link
Member

@bdraco bdraco Sep 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have strong feelings on where the DeprecatedInfo gets stored. That should be worked out to a solution that everyone is ok with in home-assistant/architecture#1133

For the tplink integration, its fine to move forward with this design and adapt to the final outcome of the arch discussion later. I don't want to hold this PR up as the new entity has already been merged in a previous PR and we need to give users proper notice that the other entity is going away.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with leaving that for the arch discussion to not block this PR 👍

platform=BUTTON_DOMAIN,
new_platform=SIREN_DOMAIN,
breaks_in_ha_version="2025.4.0",
),
),
TPLinkButtonEntityDescription(
key="stop_alarm",
deprecated_info=DeprecatedInfo(
platform=BUTTON_DOMAIN,
new_platform=SIREN_DOMAIN,
breaks_in_ha_version="2025.4.0",
),
),
]

Expand All @@ -46,13 +62,15 @@ async def async_setup_entry(
device = parent_coordinator.device

entities = CoordinatedTPLinkFeatureEntity.entities_for_device_and_its_children(
hass=hass,
device=device,
coordinator=parent_coordinator,
feature_type=Feature.Type.Action,
entity_class=TPLinkButtonEntity,
descriptions=BUTTON_DESCRIPTIONS_MAP,
child_coordinators=children_coordinators,
)
async_cleanup_deprecated(hass, BUTTON_DOMAIN, config_entry.entry_id, entities)
async_add_entities(entities)


Expand Down
111 changes: 111 additions & 0 deletions homeassistant/components/tplink/deprecate.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
"""Helper class for deprecating entities."""

from __future__ import annotations

from collections.abc import Sequence
from dataclasses import dataclass
from typing import TYPE_CHECKING

from homeassistant.components.automation import automations_with_entity
from homeassistant.components.script import scripts_with_entity
from homeassistant.core import HomeAssistant
from homeassistant.helpers import entity_registry as er
from homeassistant.helpers.issue_registry import IssueSeverity, async_create_issue

from .const import DOMAIN

if TYPE_CHECKING:
from .entity import CoordinatedTPLinkFeatureEntity, TPLinkFeatureEntityDescription


@dataclass(slots=True)
class DeprecatedInfo:
"""Class to define deprecation info for deprecated entities."""

platform: str
new_platform: str
breaks_in_ha_version: str


def async_check_create_deprecated(
hass: HomeAssistant,
unique_id: str,
entity_description: TPLinkFeatureEntityDescription,
) -> bool:
"""Return true if the entity should be created based on the deprecated_info.

If deprecated_info is not defined will return true.
If entity not yet created will return false.
If entity disabled will return false.
"""
if not entity_description.deprecated_info:
return True

deprecated_info = entity_description.deprecated_info
platform = deprecated_info.platform

ent_reg = er.async_get(hass)
entity_id = ent_reg.async_get_entity_id(
platform,
DOMAIN,
unique_id,
)
if not entity_id:
return False
sdb9696 marked this conversation as resolved.
Show resolved Hide resolved

entity_entry = ent_reg.async_get(entity_id)
assert entity_entry
return not entity_entry.disabled


def async_cleanup_deprecated(
hass: HomeAssistant,
platform: str,
entry_id: str,
entities: Sequence[CoordinatedTPLinkFeatureEntity],
) -> None:
"""Remove disabled deprecated entities or create issues if necessary."""
ent_reg = er.async_get(hass)
for entity in entities:
if not (deprecated_info := entity.entity_description.deprecated_info):
continue

Check warning on line 71 in homeassistant/components/tplink/deprecate.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/tplink/deprecate.py#L71

Added line #L71 was not covered by tests
bdraco marked this conversation as resolved.
Show resolved Hide resolved

assert entity.unique_id
entity_id = ent_reg.async_get_entity_id(
platform,
DOMAIN,
entity.unique_id,
)
assert entity_id
# Check for issues that need to be created
entity_automations = automations_with_entity(hass, entity_id)
entity_scripts = scripts_with_entity(hass, entity_id)

for item in entity_automations + entity_scripts:
async_create_issue(
hass,
DOMAIN,
f"deprecated_entity_{entity_id}_{item}",
breaks_in_ha_version=deprecated_info.breaks_in_ha_version,
is_fixable=False,
is_persistent=False,
severity=IssueSeverity.WARNING,
translation_key="deprecated_entity",
translation_placeholders={
"entity": entity_id,
"info": item,
"platform": platform,
"new_platform": deprecated_info.new_platform,
},
)

# Remove entities that are no longer provided and have been disabled.
unique_ids = {entity.unique_id for entity in entities}
for entity_entry in er.async_entries_for_config_entry(ent_reg, entry_id):
if (
entity_entry.domain == platform
and entity_entry.disabled
and entity_entry.unique_id not in unique_ids
):
ent_reg.async_remove(entity_entry.entity_id)
continue
27 changes: 23 additions & 4 deletions homeassistant/components/tplink/entity.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
)

from homeassistant.const import EntityCategory
from homeassistant.core import callback
from homeassistant.core import HomeAssistant, callback
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers import device_registry as dr
from homeassistant.helpers.device_registry import DeviceInfo
Expand All @@ -36,6 +36,7 @@
PRIMARY_STATE_ID,
)
from .coordinator import TPLinkDataUpdateCoordinator
from .deprecate import DeprecatedInfo, async_check_create_deprecated

_LOGGER = logging.getLogger(__name__)

Expand Down Expand Up @@ -87,6 +88,8 @@
class TPLinkFeatureEntityDescription(EntityDescription):
"""Base class for a TPLink feature based entity description."""

deprecated_info: DeprecatedInfo | None = None


def async_refresh_after[_T: CoordinatedTPLinkEntity, **_P](
func: Callable[Concatenate[_T, _P], Awaitable[None]],
Expand Down Expand Up @@ -251,18 +254,25 @@ def __init__(

def _get_unique_id(self) -> str:
"""Return unique ID for the entity."""
key = self.entity_description.key
return self._get_feature_unique_id(self._device, self.entity_description)

@staticmethod
def _get_feature_unique_id(
device: Device, entity_description: TPLinkFeatureEntityDescription
) -> str:
"""Return unique ID for the entity."""
key = entity_description.key
# The unique id for the state feature in the switch platform is the
# device_id
if key == PRIMARY_STATE_ID:
return legacy_device_id(self._device)
return legacy_device_id(device)

# Historically the legacy device emeter attributes which are now
# replaced with features used slightly different keys. This ensures
# that those entities are not orphaned. Returns the mapped key or the
# provided key if not mapped.
key = LEGACY_KEY_MAPPING.get(key, key)
return f"{legacy_device_id(self._device)}_{key}"
return f"{legacy_device_id(device)}_{key}"

@classmethod
def _category_for_feature(cls, feature: Feature | None) -> EntityCategory | None:
Expand Down Expand Up @@ -334,6 +344,7 @@ def _entities_for_device[
_D: TPLinkFeatureEntityDescription,
](
cls,
hass: HomeAssistant,
device: Device,
coordinator: TPLinkDataUpdateCoordinator,
*,
Expand Down Expand Up @@ -368,6 +379,11 @@ def _entities_for_device[
feat, descriptions, device=device, parent=parent
)
)
and async_check_create_deprecated(
hass,
cls._get_feature_unique_id(device, desc),
desc,
)
bdraco marked this conversation as resolved.
Show resolved Hide resolved
]
return entities

Expand All @@ -377,6 +393,7 @@ def entities_for_device_and_its_children[
_D: TPLinkFeatureEntityDescription,
](
cls,
hass: HomeAssistant,
device: Device,
coordinator: TPLinkDataUpdateCoordinator,
*,
Expand All @@ -393,6 +410,7 @@ def entities_for_device_and_its_children[
# Add parent entities before children so via_device id works.
entities.extend(
cls._entities_for_device(
hass,
device,
coordinator=coordinator,
feature_type=feature_type,
Expand All @@ -412,6 +430,7 @@ def entities_for_device_and_its_children[
child_coordinator = coordinator
entities.extend(
cls._entities_for_device(
hass,
child,
coordinator=child_coordinator,
feature_type=feature_type,
Expand Down
1 change: 1 addition & 0 deletions homeassistant/components/tplink/number.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ async def async_setup_entry(
children_coordinators = data.children_coordinators
device = parent_coordinator.device
entities = CoordinatedTPLinkFeatureEntity.entities_for_device_and_its_children(
hass=hass,
device=device,
coordinator=parent_coordinator,
feature_type=Feature.Type.Number,
Expand Down
1 change: 1 addition & 0 deletions homeassistant/components/tplink/select.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ async def async_setup_entry(
device = parent_coordinator.device

entities = CoordinatedTPLinkFeatureEntity.entities_for_device_and_its_children(
hass=hass,
device=device,
coordinator=parent_coordinator,
feature_type=Feature.Type.Choice,
Expand Down
1 change: 1 addition & 0 deletions homeassistant/components/tplink/sensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ async def async_setup_entry(
device = parent_coordinator.device

entities = CoordinatedTPLinkFeatureEntity.entities_for_device_and_its_children(
hass=hass,
device=device,
coordinator=parent_coordinator,
feature_type=Feature.Type.Sensor,
Expand Down
6 changes: 6 additions & 0 deletions homeassistant/components/tplink/strings.json
Original file line number Diff line number Diff line change
Expand Up @@ -311,5 +311,11 @@
"device_authentication": {
"message": "Device authentication error {func}: {exc}"
}
},
"issues": {
"deprecated_entity": {
"title": "Detected deprecated `{platform}` entity usage",
"description": "We detected that entity `{entity}` is being used in `{info}`\n\nWe have created a new `{new_platform}` entity and you should migrate `{info}` to use this new entity.\n\nWhen you are done migrating `{info}` and are ready to have the deprecated `{entity}` entity removed, disable the entity and restart Home Assistant."
}
}
}
3 changes: 2 additions & 1 deletion homeassistant/components/tplink/switch.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ async def async_setup_entry(
device = parent_coordinator.device

entities = CoordinatedTPLinkFeatureEntity.entities_for_device_and_its_children(
device,
hass=hass,
device=device,
coordinator=parent_coordinator,
feature_type=Feature.Switch,
entity_class=TPLinkSwitch,
Expand Down
16 changes: 16 additions & 0 deletions tests/components/tplink/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from kasa.smart.modules.alarm import Alarm
from syrupy import SnapshotAssertion

from homeassistant.components.automation import DOMAIN as AUTOMATION_DOMAIN
from homeassistant.components.tplink import (
CONF_AES_KEYS,
CONF_ALIAS,
Expand Down Expand Up @@ -184,6 +185,21 @@ async def snapshot_platform(
), f"state snapshot failed for {entity_entry.entity_id}"


async def setup_automation(hass: HomeAssistant, alias: str, entity_id: str) -> None:
"""Set up an automation for tests."""
assert await async_setup_component(
hass,
AUTOMATION_DOMAIN,
{
AUTOMATION_DOMAIN: {
"alias": alias,
"trigger": {"platform": "state", "entity_id": entity_id, "to": "on"},
"action": {"action": "notify.notify", "metadata": {}, "data": {}},
}
},
)


def _mock_protocol() -> BaseProtocol:
protocol = MagicMock(spec=BaseProtocol)
protocol.close = AsyncMock()
Expand Down
Loading