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

Start preparing for homekit_controller config entries #21564

Merged
merged 7 commits into from Mar 7, 2019

Conversation

Projects
None yet
5 participants
@Jc2k
Copy link
Contributor

commented Mar 1, 2019

Description:

This branch is the first part of my work to migrate homekit_controller to config entries.

This branch does not activate the config entries code yet, but it does have quite a bit of test coverage for the new code. It's quite a large diff so i'd like to try and lock in this first part - the other related code is here.

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.
@MartinHjelmare
Copy link
Member

left a comment

I haven't looked at tests.

Will import from config yaml step be added later?

@Jc2k

This comment has been minimized.

Copy link
Contributor Author

commented Mar 5, 2019

Will import from config yaml step be added later?

Strictly speaking there is no config yaml for homekit_controller - it was never properly integrated in that regard. However pairing resulted in JSON fragments getting created in .homekit/*.json. This is already handled in this PR.

In a previous version I used the config yaml import step to import these pairing fragments but I decided it was a bad user exeperience. HomeKit ID's change every time you pair and I am aware some users have tried pairing and unpairing multiple times in an effort to make their devices work. So multiple ghosts would be added to their system. These ghosts would be unnamed as there is insufficient metadata to give them a useful name. There is no mechanism to detect the difference between a device that is off / out of range and one that has changed its ID because of a reset + repair operation.

Instead I automatically migrate the pairing data during discovery. This means we only add known live records to the system.

@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

Right. I had forgotten there was no config schema in this component.

Jc2k added some commits Mar 5, 2019

@Jc2k

This comment has been minimized.

Copy link
Contributor Author

commented Mar 5, 2019

Right i think thats everything I can do right now from the current round of feedback.

Jc2k added some commits Mar 5, 2019

@Jc2k

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2019

Good spots, thanks. Should all be fixed in c2250fd.

@MartinHjelmare
Copy link
Member

left a comment

Great!

@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented Mar 6, 2019

Can be merged when build passes.

@balloob balloob merged commit dbf129d into home-assistant:dev Mar 7, 2019

4 checks passed

Hound No violations found. Woof!
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.01%) to 92.758%
Details

@ghost ghost removed the in progress label Mar 7, 2019

@Jc2k Jc2k referenced this pull request Mar 10, 2019

Merged

HomeKit controller config flow fixes #21898

3 of 3 tasks complete

@Jc2k Jc2k deleted the Jc2k:homekit_controller_config_flow_1 branch Mar 11, 2019

@balloob balloob referenced this pull request Mar 20, 2019

Merged

0.90.0 #22216

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.