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
74k stars 31.05k forks source link

Unifi Protect - Cant Add Doorbell Text #131003

Closed spmfox closed 5 hours ago

spmfox commented 1 week ago

The problem

I was trying out a new automation that sets the doorbell text and it does not work - the UI says "Error running action - unknown error".

What version of Home Assistant Core has the issue?

2024.11.2

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant Container

Integration causing the issue

Unifi Protect

Link to integration documentation on our website

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

Diagnostics information

protect-error.txt

Example YAML snippet

action: unifiprotect.add_doorbell_text
metadata: {}
data:
  device_id: <redacted>
  message: <redacted>

Anything in the logs that might be useful for us?

Unifi OS: v4.0.21
Unifi Protect: 5.0.51
Device: G4 Doorbell Pro

Additional information

No response

home-assistant[bot] commented 1 week ago

unifiprotect documentation unifiprotect source

RaHehl commented 1 week ago

@spmfox Works perfectly with my G4 Doorbell Pro PoE. Are you sure you provided the correct device_id? The error AttributeError: 'Controller' object has no attribute 'bootstrap' suggests you might have used the console device_id instead

spmfox commented 6 days ago

Yeah I just double checked, it's using the device_id for the doorbell.

RaHehl commented 6 days ago

What permissions does the user you set up Protect with in HA have?

image

spmfox commented 5 days ago

Okay we're getting somewhere - the user was underprivileged for sure. I was unable to even set the text from the device page in Home Assistant. Thank you @RaHehl

Once fixing the user, and reloading the integration, I was able to make the change manually from the device page.

Changing it manually got me curious why this works, so looking at the logs HA called a different action then I was trying to use.

action: text.set_value
metadata: {}
data:
  value: NO SOLICITORS
target:
  entity_id: text.cam_doorbell_doorbell

This works - but the original method I was trying (which is the way per the documentation) still does not work. I'm happy with any method so I think I'm good, however perhaps the developers want to chime in and determine if this is a bug or maybe the documentation needs to be updated and the other (old?) method removed.

RaHehl commented 5 days ago

@spmfox Thank you for sharing your findings! I’m happy to look into this further, but could you please provide logs after adjusting the permissions? As mentioned, it’s working on my end, so having additional details from your setup could help pinpoint the issue.

spmfox commented 2 days ago

Okay, I reproduced the issue with debug logging on using this YAML:

action: unifiprotect.add_doorbell_text
metadata: {}
data:
  device_id: <REDACTED>
  message: TEST

AttributeError: 'Controller' object has no attribute 'bootstrap'

unifi-protect-debug.log

bdraco commented 1 day ago

Probably the device is linked to multiple integrations so we need to check the domain here:

diff --git a/homeassistant/components/unifiprotect/data.py b/homeassistant/components/unifiprotect/data.py
index 4ad8892ca01..baecc7f8323 100644
--- a/homeassistant/components/unifiprotect/data.py
+++ b/homeassistant/components/unifiprotect/data.py
@@ -349,6 +349,7 @@ def async_ufp_instance_for_config_entry_ids(
             entry.runtime_data.api
             for entry_id in config_entry_ids
             if (entry := hass.config_entries.async_get_entry(entry_id))
+            and entry.domain == DOMAIN
             and hasattr(entry, "runtime_data")
         ),
         None,
RaHehl commented 1 day ago

@bdraco I honestly don't quite know what a domain means in this context, but it sounds plausible. I had similar theories when I looked into it — that there might be multiple instances, possibly even orphaned configurations or something like that.

Should I go ahead and prepare a PR with your suggestion?

bdraco commented 1 day ago

@bdraco I honestly don't quite know what a domain means in this context, but it sounds plausible. I had similar theories when I looked into it — that there might be multiple instances, possibly even orphaned configurations or something like that.

DOMAIN in this context is unifiprotect.

Should I go ahead and prepare a PR with your suggestion?

That would be great.

You should be able to get test coverage for it by mocking the device registry and creating another config entry with any other domain with the same mac address of the camera.

bdraco commented 1 day ago

Example of a test for another integration that fixed a similar issue

https://github.com/home-assistant/core/blob/055c38a3c819ccfc042a155af7759e39fe0d38c1/tests/components/tplink/test_init.py#L426

RaHehl commented 23 hours ago

I'll try to implement that tomorrow.

RaHehl commented 6 hours ago

@bdraco done: https://github.com/home-assistant/core/pull/131724