home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
74.15k stars 31.12k forks source link

Homekit Adds extra media players and input booleans regardless of include/exclusions rules #86731

Closed pewter77 closed 1 year ago

pewter77 commented 1 year ago

The problem

Setting Homekit up through the UI automatically creates media_player accessories for the TV, but also adds a good number of the media_player entities to the main bridge that aren't TV type. This also occurs with input booleans.

Attached will be some diagnostics showing that I've setup rules for inclusions but they're being ignored with some entities.

What version of Home Assistant Core has the issue?

2023.1.7

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant OS

Integration causing the issue

Homekit

Link to integration documentation on our website

https://www.home-assistant.io/integrations/homekit/

Diagnostics information

config_entry-homekit-860406da9ad66b741cfc4c1f3d1c42ec.json.txt

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

No response

home-assistant[bot] commented 1 year ago

Hey there @bdraco, mind taking a look at this issue as it has been labeled with an integration (homekit) you are listed as a code owner for? Thanks!

Code owner commands Code owners of `homekit` can trigger bot actions by commenting: - `@home-assistant close` Closes the issue. - `@home-assistant rename Awesome new title` Change the title of the issue. - `@home-assistant reopen` Reopen the issue. - `@home-assistant unassign homekit` Removes the current integration label and assignees on the issue, add the integration domain after the command.

(message by CodeOwnersMention)


homekit documentation homekit source (message by IssueLinks)

bdraco commented 1 year ago
          "include_domains": [
            "light",
            "media_player",
            "climate",
            "fan",
            "scripts",
            "input_boolean"
          ],
          "include_entity_globs": [
            "light.*"
          ],
          "include_entities": [
            "fan.study_air_purifier",
            "media_player.nvidia_shield_atv",
            "media_player.living_room_tv",
            "script.find_remote",
            "script.force_emby_scan",
            "sensor.study_air_purifier_filter_life_remaining",
            "sensor.study_air_purifier_humidity",
            "sensor.study_air_purifier_pm2_5",
            "sensor.study_air_purifier_temperature",
            "input_boolean.sleep_mode",
            "input_boolean.stretching"
          ],

You have input_boolean and media_player in include_domains so it will include everything in that domain

pewter77 commented 1 year ago

But that isn't how the UI flow seems to work which makes it confusing. You have to a choose a domain and then only those entities in those domains are shown on the next page's dropdown.

So if I choose the light domain only select_domains

Then I only have the option here to choose lights

light_dropdown

In fact when setting up homekit integration the first time you don't even get the option to change what is included anymore and have to reconfigure after. So the whole process is a bit confusing and doesn't seem to follow the documentation which says that if entities show as included include them otherwise if it doesn't check domains?

2. Only includes
- Entity listed in entities include: include
- Otherwise, entity matches domain include: include
- Otherwise, entity matches glob include: include
- Otherwise: exclude

If being honest, the documentation is hard to follow.

anth-dinosaur commented 1 year ago

Just adding my 2c -

I think @bdraco is right -- @pewter77 you indeed do have input_boolean in the included domains, so everything is showing up. But, you have also pointed out a bug in the way the UI is generating the config: that entry shouldn't be there since you have explicitly specified which input_booleans to include.

I have recently had a similar issue with devices not respecting what I thought I had configured in the UI. I needed to deselect EVERYTHING from being included, and then re-set up, before the config was correctly generated.

bdraco commented 1 year ago

Just adding my 2c -

I think @bdraco is right -- @pewter77 you indeed do have input_boolean in the included domains, so everything is showing up. But, you have also pointed out a bug in the way the UI is generating the config: that entry shouldn't be there since you have explicitly specified which input_booleans to include.

@MrChuckDanger Do you have reproduction steps?

I have recently had a similar issue with devices not respecting what I thought I had configured in the UI. I needed to deselect EVERYTHING from being included, and then re-set up, before the config was correctly generated.

@

anth-dinosaur commented 1 year ago

Now that I review the logs, I realize my issue was slightly different than UI generating an unexpected config. I had added a few entities to be included, 3 out of 4 of which weren't showing up within HomeKit, despite reload of the integration. However, I am seeing them having been properly configured in the diagnostic config that I had downloaded (but only quickly looked at).

It took me doing the deselect/reselect process to get them to come up. I can try if I can reproduce this if it would be helpful, but the scope is probably much different than the issue described above.

bdraco commented 1 year ago

If you have repeatable reproduction steps that clearly show a bug, I can write a test and fix whatever is causing it. Short of that, there isn't much I can do here besides trying random combinations over and over and hope I hit a problem.

pewter77 commented 1 year ago

Just adding my 2c -

I think @bdraco is right -- @pewter77 you indeed do have input_boolean in the included domains, so everything is showing up. But, you have also pointed out a bug in the way the UI is generating the config: that entry shouldn't be there since you have explicitly specified which input_booleans to include.

I have recently had a similar issue with devices not respecting what I thought I had configured in the UI. I needed to deselect EVERYTHING from being included, and then re-set up, before the config was correctly generated.

This was my point, that I was choosing the input-boolean domain and then choosing 2 out of 4 of the booleans to share to homekit. Then all of them were being shared to homekit and the same was happening with my media players, I choose the media player domain but then chose specifically only 2 of them to share and it shared them all. I'll try to get some diagnostics @bdraco but right now I've had to set it up in exclude only mode to get it to work correctly and I'm away for work for a few weeks.

jclendineng commented 1 year ago

I can replicate, I am using "include" and everything works great, I aded some HomePods and they all showed up as remotes in HomeKit even though they were never selected to pass to HK.

bdraco commented 1 year ago

I can replicate, I am using "include" and everything works great, I aded some HomePods and they all showed up as remotes in HomeKit even though they were never selected to pass to HK.

Please post diagnostics and replication steps

jclendineng commented 1 year ago

I think I figured out what MAY have happened, the default "System Options" in integrations is "Enable newly added entities." So when I added HomePods, they were added automatically, though they shouldn't have been since I am "include" only and "Remotes" are not selected. This is a brand new setup, I just moved from Hubitat. I THINK if I add an exclude for remotes to configuration.yaml it might force to to drop but the it should respect the UI selection as well IMO.

Edit: I do have this:

  "options": {
    "filter": {
      "include_domains": [
        "remote",
        "switch"

Even though remotes is not selected, that's the issue.

Edit 2: An interesting observation, "remotes" is added to domains when selected in the dropdown in the UI meaning even if no remotes are selected in the UI it defaults to all available remotes. If I manually exclude remotes they do indeed go away...

mikeknoop commented 1 year ago

Expected: using include mode, only entities explicitly selected in the entity selector are included.

Actual: any domains selected will include all entities unless you pick at least one entity from that domain.

Workarounds:

Repro steps (using current UI flow):

  1. Install HASS Bridge, select no domains
  2. Don't pair yet (confusing flow here, but whatever)
  3. Click configure, use bridge and include and select two different domains
  4. On next screen, choose only one entity from domain 1
  5. Now pair. Observe the one selected entity is correctly included from domain 1. However, all entities from domain 2 are incorrectly included.

Comments: I think I get why this happens -- the nuance the docs/UI get wrong is this logic "otherwise" check applies only to a singular domain not across domains, which is why my listed workarounds work.

Suggested fixes:

  1. Change logic to apply across domains
  2. Change set up flow to put the full configure screen into the install flow
bdraco commented 1 year ago

This behavior is expected. If you select a domain to include but select no entities to include in that domain the whole domain will be included. I guess the message here could be made more clear:

d888b843d85905dfc901ed494db91ad8
mikeknoop commented 1 year ago

Correct, we are in the realm of debating preferred UX and user expectations, not code bugs. I'd argue given exclude exists to oppose include, include should be explicit. Another potential solve is to divorce the domain selector from filtering the entity selector and bring them onto the same screen. That'll help educate the distinction between "include an entire domain" and "include a specific set of entities".

bdraco commented 1 year ago

I'm not inclined to change the UX as its going to come back with complaints about having to check the boxes for everything in a domain when a user wants to include an entire domain.

I'll open a PR to clarify the text so hopefully its less confusing.

bdraco commented 1 year ago
Screenshot 2023-04-08 at 12 01 33 PM
pewter77 commented 1 year ago

This issue went off the rails, because what was happening is:

Based on what you've said and the changes to the UX that is exactly how it's NOT supposed to work. Based on the quote below from months ago and your comments two days ago BDRACO, they're in direct contractions with each other and my experience. I still believe there is something wrong with the logic that defines how these things work or the UI (unless an update has been rolled out to address it.) As I again don't wish to re-setup everything in my house right now I'll leave this as is and re-open if I run into this issue again.

          "include_domains": [
            "light",
            "media_player",
            "climate",
            "fan",
            "scripts",
            "input_boolean"
          ],
          "include_entity_globs": [
            "light.*"
          ],
          "include_entities": [
            "fan.study_air_purifier",
            "media_player.nvidia_shield_atv",
            "media_player.living_room_tv",
            "script.find_remote",
            "script.force_emby_scan",
            "sensor.study_air_purifier_filter_life_remaining",
            "sensor.study_air_purifier_humidity",
            "sensor.study_air_purifier_pm2_5",
            "sensor.study_air_purifier_temperature",
            "input_boolean.sleep_mode",
            "input_boolean.stretching"
          ],

You have input_boolean and media_player in include_domains so it will include everything in that domain