Ensure homekit_controller recieves zeroconf c# updates

[bdraco]

Proposed change

If an integration has a homekit config flow step
homekit_controller would not see updates for
devices that were paired with it and would not
rescan for changes.

Revision: We only send updates to homekit_controller if the
device is already paired.

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)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

# Example configuration.yaml

Additional information

• This PR fixes or closes issue: fixes Unable to pair tado bridge correctly through home kit controller #35532
• 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:

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

The integration reached or maintains the following Integration Quality Scale:

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

[probot-home-assistant]

Hey there @robbiet480, @Kane610, mind taking a look at this pull request as its been labeled with a integration (zeroconf) you are listed as a codeowner for? Thanks!
_{^{(message by CodeOwnersMention)}}

[probot-home-assistant]

Hey there @Jc2k, mind taking a look at this pull request as its been labeled with a integration (homekit_controller) you are listed as a codeowner for? Thanks!
_{^{(message by CodeOwnersMention)}}

[Jc2k]

@bdraco ! that was quick. Thanks a lot. It would have been ages before I had the time to put a PR together.

I think when I've talked about this with HA devs before the sticking point was that it's fine for homekit_controller to get the c# updates always but it shouldn't offer a discovery of its own if a native integration has intercepted its zeroconf record. So homekit_controller would also need to apply the tests that handle_homekit does.

[bdraco]

@bdraco ! that was quick. Thanks a lot. It would have been ages before I had the time to put a PR together.

I think when I've talked about this with HA devs before the sticking point was that it's fine for homekit_controller to get the c# updates always but it shouldn't offer a discovery of its own if a native integration has intercepted its zeroconf record. So homekit_controller would also need to apply the tests that handle_homekit does.

@Jc2k

I thought about adding the check in zeroconf to not offer discovery in that case, but I think its better to let the user choose if they want to integrate via homekit_controller (local) or the device's api (usually cloud). If they have already paired it via homekit it is not going to show up so it shouldn't impact most users.

When I think about this from a brand perspective, Home Assistant's tag line is "Open source home automation that puts local control and privacy first." It makes sense to me to always offer the homekit_controller option as seems inline with product vision.

Additionally, the recent myq api changed and everything is broken incident is another reminder that users may have a better experience pairing via homekit_controller instead of the cloud api.

The above is just my opinion though so if the direction is to add the check, I can modify this PR to accomplish that.

[Jc2k]

I totally agree with you everything you have said, and have made similar arguments in the past myself. But I think the core devs opinion was that an integration that knows about only device X will always work better for device X than one that speaks a generic protocol. I can see their point - homekit_controller has to support loads of differing homekit stacks and we've seen how varied they are at following the spec. Things like supporting false as a bool value but not true or not supporting spaces in JSON. When we've solved those bugs for a new family of devices it works great. But I do still worry about giving users a poor onboarding experience.

The biggest issue I have is one of discoverability. If you are privacy minded or prefer non cloud solutions for stability and longevity but don't know about HomeKit then you are unlikely to find out about homekit_controller. And there are some devices where we can offer push based motion sensors and the native integration can't. So you might be missing out on capabilities too.

The compromise that @balloob suggested a while back was to offer a choice if the user was in "Advanced" mode in the UI. But i'm not sure what that would look like.

[balloob]

I would love to see people using Homekit Controller over cloud options. If there is both a homekit and a local device specific integration, I would say the device specific one wins. But before we can even start to implement such things…

We need to educate users what Homekit Controller is. Users don't understand that controlling their devices via Homekit is an option without owning an Apple product. Caused complains. Lots of issues how to get rid of Homekit controller discoveries 🙄 It's what caused us to introduce unique IDs and ignoring flows.

I think that in a perfect world, we would be able to show a UI that groups the two discoveries and has a link to "What is Homekit Controller and why I should care".

[bdraco]

@Jc2k @balloob Adjusted so only devices that are already paired get sent to homekit_controller This avoids the second discovery via homekit_controller if another integration claims it but still gets homekit controller the updates it needs.

Edit: The point about the user being confused about multiple ways to integrate is well taken. Ideally the UI could ask them how they want to integrate and give them some information on local vs cloud since we know the integration method from the config flow. That’s for another day though and likely not really worth it until there are more integrations that can be discovered via HomeKit

[balloob]

I am wondering here if this should be the responsibility of the zeroconf integration or if homekit_controller should register with the zeroconf ServiceBrowser directly instead.

[balloob]

I think it's fine in this PR. Let's keep it.

[Jc2k]

@balloob thanks for clarifying! The constant distress from the people that thought Apple was going to take over their network or something weird, whilst simultaneously relying on not private not local synchronous polling based integrations was quite taxing! I think we are pretty aligned on how it should work long term, now if only I had a few more days in a week :-)

@bdraco Nice. That will probably get most occurrences. Only edge case I can think of is if it is paired but not with HA (or a different HA). Personally I have done both but mostly for dev environment reasons. Only other option I can think of is to make the model matching code separate from homekit_handle and reuse it from homekit controllers config flow. But maybe too much coupling in that approach.

[bdraco]

@bdraco Nice. That will probably get most occurrences. Only edge case I can think of is if it is paired but not with HA (or a different HA). Personally I have done both but mostly for dev environment reasons. Only other option I can think of is to make the model matching code separate from homekit_handle and reuse it from homekit controllers config flow. But maybe too much coupling in that approach.

I wasn't clear if the edge case was that it will show up in discovery a second time or if homekit_controller won't see the c# update?

It looks like there is already code in homekit_controller to guard against offering to pair if its paired elsewhere.

        if paired:
            # Device is paired but not to us - ignore it
            _LOGGER.debug("HomeKit device %s ignored as already paired", hkid)
            return self.async_abort(reason="already_paired")

[Jc2k]

Ah, I had tunnel vision and forgot that homekit_controller would filter it back out. Then yep, this works great.