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 legacy Proximity entity #108730

Merged

Conversation

mib1185
Copy link
Contributor

@mib1185 mib1185 commented Jan 23, 2024

Breaking change

The proximity entity is deprecated (will be removed in 2024.8). It is superseded by sensor entities.
For each tracked person or device one sensor for the distance and the direction of travel to/from the monitored zone is created. Further for each Proximity configuration one sensor which shows the nearest device or person to the monitored zone is created.
With this you can use the Min/Max integration to determine the nearest and furthest distance.

Proposed change

This is the deprecation of the legacy Proximity entity, after sensor entities has been implemented (#101497)

image

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:

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.
  • Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

@home-assistant home-assistant bot added cla-signed deprecation Indicates a breaking change to happen in the future integration: proximity small-pr PRs with less than 30 lines. labels Jan 23, 2024
Copy link

@home-assistant home-assistant bot left a comment

Choose a reason for hiding this comment

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

There is a merge conflict.

@home-assistant
Copy link

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

@mib1185 mib1185 marked this pull request as ready for review January 23, 2024 16:46
@mkmer
Copy link
Contributor

mkmer commented Jan 24, 2024

2024.6 (Breaking change "words") or 2024.8 (in picture)?

@mib1185
Copy link
Contributor Author

mib1185 commented Jan 24, 2024

oops ... this change was postponed multiple times ... i've corrected the breaking change message

@mkmer
Copy link
Contributor

mkmer commented Jan 24, 2024

Not to slow down this update, but I thought we had to add config flow to any integration before updating things.
Maybe that should be the next PR for this one?

@mib1185
Copy link
Contributor Author

mib1185 commented Jan 24, 2024

nevertheless, you can change things/integrations without the need of migrating to a config flow, except changes on yaml based parameters (see ADR 7)

DOMAIN,
f"deprecated_proximity_entity_{friendly_name}",
breaks_in_ha_version="2024.8.0",
is_fixable=False,
Copy link
Member

Choose a reason for hiding this comment

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

Issues like these should be fixable and persistent so that the user can fix them when they've updated their automations and scripts.

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'm not sure if this should be persistent (the issue is re-created as long as these automations and/or scripts exists on ever HA reboot), nor that we can implement a automatic fix for that 🤔

further as soon as is_fixable=True is set, the description is not shown anymore (already tried to clear browser cache)

image

Copy link
Member

Choose a reason for hiding this comment

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

I think we need to put the strings as part of a repair flow in the strings.json.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, i got it work

Copy link
Member

Choose a reason for hiding this comment

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

We don't need to create a repair flow. A default repair flow will be created for us. Just write the strings like so:

"issues": {
"deprecated_service_calendar_list_events": {
"title": "Detected use of deprecated service `calendar.list_events`",
"fix_flow": {
"step": {
"confirm": {
"title": "[%key:component::calendar::issues::deprecated_service_calendar_list_events::title%]",
"description": "Use `calendar.get_events` instead which supports multiple entities.\n\nPlease replace this service and adjust your automations and scripts and select **submit** to close this issue."
}
}
}
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but this default repair flow will not do any re-check if the issue is really fixed, or do i oversee something?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that's correct. The user will fix it manually, hopefully after they have updated their automations and scripts. We'll create the issue again though, the next time they start Home Assistant if we detect automations or scripts with an entity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, this makes it quiet simpler to implement 👍

homeassistant/components/proximity/strings.json Outdated Show resolved Hide resolved
tests/components/proximity/test_init.py Show resolved Hide resolved
tests/components/proximity/test_init.py Outdated Show resolved Hide resolved
@home-assistant home-assistant bot marked this pull request as draft January 25, 2024 15:01
@mib1185 mib1185 marked this pull request as ready for review January 25, 2024 18:35
@mib1185 mib1185 marked this pull request as draft January 25, 2024 18:48
@mib1185 mib1185 marked this pull request as ready for review January 26, 2024 16:03
Copy link
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Thanks!

@MartinHjelmare MartinHjelmare merged commit d007327 into home-assistant:dev Jan 26, 2024
23 checks passed
@mib1185 mib1185 deleted the proximity/deprecate-legacy-entity branch January 26, 2024 18:14
@github-actions github-actions bot locked and limited conversation to collaborators Jan 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
by-code-owner cla-signed deprecation Indicates a breaking change to happen in the future integration: proximity Quality Scale: internal small-pr PRs with less than 30 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants