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.

Sign up for GitHub

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

Jump to bottom

Refactor HomeKit to allow supported features/device class to change #101719

Merged
merged 37 commits into from Oct 10, 2023
Merged

Refactor HomeKit to allow supported features/device class to change #101719

merged 37 commits into from Oct 10, 2023

Conversation

bdraco
Copy link
Member

@bdraco bdraco commented Oct 9, 2023 •
edited

needs #101760

Proposed change

Refactor HomeKit to allow supported features/device class to change

change_props

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

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 Black (black --fast 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:

@bdraco

This comment was marked as outdated.

Sign in to view
@bdraco

This comment was marked as outdated.

Sign in to view
bdraco
bdraco commented Oct 9, 2023
View reviewed changes
homeassistant/components/homekit/accessories.py Outdated Show resolved Hide resolved
homeassistant/components/homekit/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/homekit/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/homekit/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/homekit/__init__.py Show resolved Hide resolved
homeassistant/components/homekit/__init__.py Show resolved Hide resolved
homeassistant/components/homekit/__init__.py Show resolved Hide resolved
homeassistant/components/homekit/accessories.py Outdated Show resolved Hide resolved
@bdraco bdraco marked this pull request as ready for review October 10, 2023 00:20
emontnemery
emontnemery approved these changes Oct 10, 2023
View reviewed changes
Copy link
Contributor

@emontnemery emontnemery left a comment

Choose a reason for hiding this comment

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

LGTM, just a couple of minor comments

homeassistant/components/homekit/accessories.py Outdated Show resolved Hide resolved
homeassistant/components/homekit/accessories.py Outdated Show resolved Hide resolved
@frenck frenck marked this pull request as draft October 10, 2023 08:45
@emontnemery emontnemery mentioned this pull request Oct 10, 2023
Merged
20 tasks
bdraco added a commit to home-assistant/home-assistant.io that referenced this pull request Oct 10, 2023
@bdraco
Remove text about needing to reset for config changes from HomeKit
1720357 
home-assistant/core#101719 adds support for reloading accessories
when the config changes. It should not be needed to reset them anymore when the config
changes.

HomeKit cache does still cache the name so I left that, as well as the block on resetting
because there is always the case where iCloud does not accept the change and it still
does have to be reset, but this should be the execptional case now.
@bdraco bdraco mentioned this pull request Oct 10, 2023
Merged
8 tasks
@bdraco bdraco marked this pull request as ready for review October 10, 2023 15:32
@bdraco bdraco marked this pull request as draft October 10, 2023 15:38
@bdraco
Copy link
Member Author

bdraco commented Oct 10, 2023

Will leave as draft until the break out merges so this doesn't get accidentally merged before it

@bdraco bdraco marked this pull request as ready for review October 10, 2023 15:57
@bdraco
Copy link
Member Author

bdraco commented Oct 10, 2023

Thanks. Good to merge this early in the cycle. I think it will fix a lot of other issues as well

@bdraco bdraco merged commit 7b4b8e7 into dev Oct 10, 2023
21 checks passed
@bdraco bdraco deleted the hk_change branch October 10, 2023 16:20
@bdraco bdraco mentioned this pull request Oct 11, 2023
Merged
@github-actions github-actions bot locked and limited conversation to collaborators Oct 11, 2023
@bdraco bdraco added the noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear) label Oct 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@emontnemery emontnemery emontnemery approved these changes

Assignees
No one assigned
Labels
by-code-owner cla-signed Hacktoberfest integration: homekit noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear) Quality Scale: No score
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

HomeKit media player loses source list
2 participants
@bdraco @emontnemery