Deprecate tplink alarm button entities

[sdb9696]

Breaking change

The alarm button entities have been migrated to the siren platform and will be removed from Home Assistant in 2025.4.0

Proposed change

Mark the alarm button stop_alarm and test_alarm for deprecation in 2025.4.0

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New integration (thank you!)
• New feature (which adds functionality to an existing integration)
• Deprecation (breaking change to happen in the future)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Additional information

• This PR fixes or closes issue: fixes #
• This PR is related to issue:
• Link to documentation pull request:

Checklist

• The code change is tested and works locally.
• Local tests pass. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist
• I have followed the perfect PR recommendations
• The code has been formatted using Ruff (ruff format homeassistant tests)
• Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

• I have reviewed two other open pull requests in this repository.

[home-assistant]

Hey there @rytilahti, @bdraco, mind taking a look at this pull request as it has been labeled with an integration (tplink) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of tplink can trigger bot actions by commenting:

• @home-assistant close Closes the pull request.
• @home-assistant rename Awesome new title Renames the pull request.
• @home-assistant reopen Reopen the pull request.
• @home-assistant unassign tplink Removes the current integration label and assignees on the pull request, add the integration domain after the command.
• @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
• @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

[rytilahti]

Tested to work as long as the automation concerns the entity and not the device, see the screenshots. I suppose this is a limitation in how core handles things and there is no way to get the first to trigger the issue, too?

[sdb9696]

Could you post the yaml from both automations? Maybe we could extend the check.

[sdb9696]

Also did your number entities stay the same after the change from Auto to Box?

[rytilahti]

Here's the yaml:

- id: '1726921029401'
  alias: alarm on sunset
  description: ''
  trigger:
  - platform: sun
    event: sunset
    offset: 0
  condition: []
  action:
  - device_id: a1735d6b46457c8fcdb0d13f6f6661a6
    domain: button
    entity_id: de5b28b6a9c90b799b135524363b5623
    type: press
  - action: button.press
    metadata: {}
    data: {}
    target:
      entity_id: button.smart_hub_test_alarm
  mode: single

The number entity (at least for the temp offset) shows now a box instead of a slider, which is much nicer for the UX.

[sdb9696]

Hmm, maybe I should pull the number change into a separate PR to go in before this one.

EDIT: #126397 opened to fix the number issue in a separate PR. Marking this as draft until it's merged.

[sdb9696]

#126397 merged so marking this as ready again.

Regarding the core function automations_with_entity not picking up the device level automation I've taken a look and as it's an issue with the core function I don't think it belongs in this PR.

[gjohansson-ST]

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)

[sdb9696]

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

[gjohansson-ST]

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.

[sdb9696]

Moved reply to comment below

[bdraco]

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.

[gjohansson-ST]

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

[gjohansson-ST]

Tested to work as long as the automation concerns the entity and not the device, see the screenshots. I suppose this is a limitation in how core handles things and there is no way to get the first to trigger the issue, too?

The first one is referencing a device and not an entity so to also deprecate such it needs to use automations_with_device and reference the device which the entity belongs too.

[sdb9696]

@gjohansson-ST and @bdraco answering both your questions here as they are related.

It a bit unexpected that this happens as part of the listcomp to build the list of entities since it has the side effect of possibly creating an issue

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)

The logic here is adapted from a lutron PR that I was pointed towards as a good example of how to do this. I recently implemented this for ring also. The logic is:

1. There's no entity registry entry for the deprecated entity so it's never been previously created - do not create the entity.
2. The entity registry entry is disabled - delete the entry and do not create the entity.
3. The entity registry entry exists and is not disabled - create the entity.
4. The entity registry entry exists, is not disabled, and is referenced in active scripts or automations. - create the entity and raise an issue.

1. is a good piece of logic because if a user starts using the integration during the 6 month period there's no need to create duplicate entities for the same feature. Combined with 2. it gives a user the ability to get rid of the deprecated entity immediately if they want.

Because of the logic in 1. and 2. I believe it is necessary to perform this logic as part of the list comprehension prior to creating the entities, as opposed to running them through it afterwards. Regarding the side effect of creating the issues during the building of the list, is this a stability/performance concern? I could look at doing this in a post step perhaps, maybe in async_added_to_hass or after async_forward_entry_setups because the entities will still have the deprecated info available in their entity descriptions.

[gjohansson-ST]

The logic here is adapted from a lutron PR that I was pointed towards as a good example of how to do this. I recently implemented this for ring also.

But you're not constructing it as lutron but instead building a very complex setup by expanding the entity description.
I'm not questioning the logic (it seems fine to me) but how you process it which is just unnecessary complex compared with what needed (lutron PR as example).

I think a simple helper would do which;

1. For entity description pass it to the helper which will check deprecation (raise issue or not) and give back if entity should be created or not based on the rules in play.
2. Make the list of entities to create based on the outcome from the helper.
3. Pass the list to add_entities.

[sdb9696]

1. For entity description pass it to the helper which will check deprecation (raise issue or not) and give back if entity should be created or not based on the rules in play.
2. Make the list of entities to create based on the outcome from the helper.
3. Pass the list to add_entities.

That is essentially what it's doing. It's just that this integration already has centralised helper functions to determine whether to add entities or not and it creates them in entity.py. So if another clause is being added to decide whether to create or not it has to go in the same place, it can't just live in the platform class with the standard pattern you have in 1. 2. & 3.

Regarding adding the actual deprecation info to the entity description, I think this is actually a good thing rather than maintaining a separate list of description keys in the helper function, because everything is done declaratively in the same place rather than spreading logic regarding whether to create entities into different functions.

[bdraco]

There is an unrelated uv.lock file in this PR

[sdb9696]

uv.lock removed

[bdraco]

Tagging this since the new feature was already added and we are missing the deprecation of the old entities