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

Bump aiohomekit to 1.0.0 #75198

Merged
merged 5 commits into from Jul 14, 2022

Conversation

Jc2k
Copy link
Member

@Jc2k Jc2k commented Jul 14, 2022

Proposed change

This bumps to the latest release of aiohomekit, which includes BLE and CoAP (Thread) support. The 2 new backends are switched off, and the goal of this release is to make sure there are no regressions in the existing IP/HTTP backend. We'll then test and switch on BLE in a follow up PR.

The diff is quite large, but we've already done all the API breaks on the 0.7.x channel, so no HA changes are needed to go along with this (well, subjecting to regression testing).

Jc2k/aiohomekit@0.7.22...1.0.0rc1

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
  • 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.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@probot-home-assistant
Copy link

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

@project-bot project-bot bot moved this from Needs review to By Code Owner in Dev Jul 14, 2022
@bdraco
Copy link
Member

bdraco commented Jul 14, 2022

mypy issue is fixed in #75200

@bdraco
Copy link
Member

bdraco commented Jul 14, 2022

All my IP devices are working great still on my production installs

Will do performance testing shortly to check for regressions

@bdraco
Copy link
Member

bdraco commented Jul 14, 2022

Turned on all the debug logs and watched the traffic.

To validate

  • Houston
  • Maui

@bdraco
Copy link
Member

bdraco commented Jul 14, 2022

Started up with devices unplugged and plugged them in to verify they recovered quickly

@bdraco
Copy link
Member

bdraco commented Jul 14, 2022

Only thing interesting in the profile is the json overhead and its really small. We could use orjson now for sending

@bdraco
Copy link
Member

bdraco commented Jul 14, 2022

Doing py-spys now

@bdraco
Copy link
Member

bdraco commented Jul 14, 2022

py-spys look great.

It took so long because its so efficient I was having trouble fixing everything in the profiles and py-spys

@bdraco
Copy link
Member

bdraco commented Jul 14, 2022

Existing device testing is all good

@bdraco
Copy link
Member

bdraco commented Jul 14, 2022

Testing TODO:

  • Add new device
  • unpair device
  • discovery
  • reload

@bdraco
Copy link
Member

bdraco commented Jul 14, 2022

Pair new is failing with

2022-07-14 13:41:59.949 ERROR (MainThread) [homeassistant.components.homekit_controller.config_flow] Pairing attempt failed with an unhandled exception
Traceback (most recent call last):
  File "/Users/bdraco/home-assistant/homeassistant/components/homekit_controller/config_flow.py", line 408, in async_step_pair
    self.finish_pairing = await discovery.async_start_pairing(self.hkid)
AttributeError: 'coroutine' object has no attribute 'async_start_pairing'

Copy link
Member

@bdraco bdraco left a comment

Choose a reason for hiding this comment

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

Pairing and unpairing is solid now with the last bump

Dev automation moved this from By Code Owner to Reviewer approved Jul 14, 2022
@Jc2k Jc2k force-pushed the homekit_controller_aiohomekit_1 branch from a5c5463 to f400267 Compare July 14, 2022 21:06
@Jc2k Jc2k marked this pull request as ready for review July 14, 2022 21:06
@Jc2k
Copy link
Member Author

Jc2k commented Jul 14, 2022

Bumped to final release tag and rebased on dev

@bdraco
Copy link
Member

bdraco commented Jul 14, 2022

The run is here https://github.com/home-assistant/core/actions/runs/2673138918
GHA is just being flakey

@bdraco
Copy link
Member

bdraco commented Jul 14, 2022

@bdraco bdraco merged commit ff297cb into home-assistant:dev Jul 14, 2022
Dev automation moved this from Reviewer approved to Done Jul 14, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jul 15, 2022
@frenck frenck added the noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear) label Jul 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
by-code-owner cla-signed dependency dependency-bump integration: homekit_controller noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear) small-pr PRs with less than 30 lines.
Projects
Dev
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants