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
72.68k stars 30.43k forks source link

ONVIF integration failing to setup after update to 2023.4 on Hikvision iDS-7208HQHI-M1 #91398

Closed denpamusic closed 1 year ago

denpamusic commented 1 year ago

The problem

Since upgrading to 2023.4.0 and all the way to the current 2023.4.4 release, ONVIF integration doesn't work for my Hikvision iDS-7208HQHI-M1 NVR with the latest firmware V4.71.010.

Screenshot 2023-04-14 at 01-26-18 Settings – Home Assistant

There's nothing in the logs on default log level, however by changing the level to debug I've managed to obtain entries attached in the file below: home-assistant_2023-04-13T22-40-02.928Z.log

I've tried reloading integration and setting it up from the scratch on fresh HASS instance, sadly it was still failing to load. In the end, I've found out that rolling back onvif-zeep-async requirement in the ONVIF integration's manifest.json to version 1.2.2 fixes the issue. This allowed me to download diagnostics information that is attached to this issue.

What version of Home Assistant Core has the issue?

core-2023.4.0

What was the last working version of Home Assistant Core?

core-2023.3.6

What type of installation are you running?

Home Assistant Container

Integration causing the issue

onvif

Link to integration documentation on our website

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

Diagnostics information

config_entry-onvif-a766f4742085d42e0c75070a583d84a5.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 @hunterjm, mind taking a look at this issue as it has been labeled with an integration (onvif) you are listed as a code owner for? Thanks!

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

(message by CodeOwnersMention)


onvif documentation onvif source (message by IssueLinks)

bdraco commented 1 year ago

Thanks for the logs. Unfortunately the exception that is causing the Failure to setup is missing from the log

denpamusic commented 1 year ago

You're welcome. Sadly there're no errors in the log either with default log level or debug log. Just tried it again on fresh install. It says that configuration is successfully created but integration card still displays Failed to set up message.

Screenshot 2023-04-14 at 03-20-38 Settings – Home Assistant

I'm not really familiar with ONVIF, but maybe this has something to do with it:

With onvif-zeep-async 1.2.2 camera profiles are not empty:

2023-04-14 00:34:34.597 DEBUG (MainThread) [homeassistant.components.onvif] Camera video.home profiles = [Profile(index=7, token='ProfileToken019', name='ProfileName019', video=Video(encoding='H264', resolution=Resolution(width=960, height=576)), ptz=PTZ(continuous=True, relative=False, absolute=False, presets=[]), video_source_token='VideoSourceToken001'), Profile(index=8, token='ProfileToken020', name='ProfileName020', video=Video(encoding='H264', resolution=Resolution(width=960, height=576)), ptz=PTZ(continuous=True, relative=False, absolute=False, presets=[]), video_source_token='VideoSourceToken002'), Profile(index=9, token='ProfileToken021', name='ProfileName021', video=Video(encoding='H264', resolution=Resolution(width=960, height=576)), ptz=PTZ(continuous=True, relative=False, absolute=False, presets=[]), video_source_token='VideoSourceToken003'), Profile(index=10, token='ProfileToken022', name='ProfileName022', video=Video(encoding='H264', resolution=Resolution(width=960, height=576)), ptz=PTZ(continuous=True, relative=False, absolute=False, presets=[]), video_source_token='VideoSourceToken004'), Profile(index=11, token='ProfileToken023', name='ProfileName023', video=Video(encoding='H264', resolution=Resolution(width=960, height=576)), ptz=PTZ(continuous=True, relative=False, absolute=False, presets=[]), video_source_token='VideoSourceToken005'), Profile(index=12, token='ProfileToken024', name='ProfileName024', video=Video(encoding='H264', resolution=Resolution(width=960, height=576)), ptz=PTZ(continuous=True, relative=False, absolute=False, presets=['PTZPresetToken6001']), video_source_token='VideoSourceToken006'), Profile(index=13, token='ProfileToken025', name='ProfileName025', video=Video(encoding='H264', resolution=Resolution(width=960, height=576)), ptz=PTZ(continuous=True, relative=False, absolute=False, presets=[]), video_source_token='VideoSourceToken007')]

but with 1.2.3 they are:

2023-04-14 03:33:01.516 DEBUG (MainThread) [homeassistant.components.onvif] Camera video.home profiles = []
bdraco commented 1 year ago

You should get at least one of these messages if async_setup is returning False: https://github.com/home-assistant/core/blob/0ddccb26fa38f991f0975d30229c7bbe28412834/homeassistant/components/onvif/device.py#L128

Absent of more information, it looks like https://github.com/mvantellingen/python-zeep/pull/1369 might fix the issue when it gets merged

bdraco commented 1 year ago

So its likely hitting

            # No camera profiles to add
            if not self.profiles:
                return False
denpamusic commented 1 year ago

Thanks for the heads up. I'll try to apply the PR on my test machine and see if it helps.

bdraco commented 1 year ago

Still I would expect you would see an exception raised rather than an empty result on failure.

bdraco commented 1 year ago

Can you try adding a debug logging line and seeing what GetProfiles is returning? It looks like if it fails to return a list we return an empty list which is what you are seeing in the log.

diff --git a/homeassistant/components/onvif/device.py b/homeassistant/components/onvif/device.py
index 1556ae8a1f..c08bf6b65b 100644
--- a/homeassistant/components/onvif/device.py
+++ b/homeassistant/components/onvif/device.py
@@ -114,6 +114,7 @@ class ONVIFDevice:

             # No camera profiles to add
             if not self.profiles:
+                LOGGER.error("No camera profiles found for %s", self.name)
                 return False

             if self.capabilities.ptz:
@@ -307,6 +308,7 @@ class ONVIFDevice:
         """Obtain media profiles for this device."""
         media_service = self.device.create_media_service()
         result = await media_service.GetProfiles()
+        LOGGER.debug("%s: GetProfiles returned: %s", self.name, result)
         profiles: list[Profile] = []

         if not isinstance(result, list):
denpamusic commented 1 year ago

You were right. I does hit that line. Here's the log: home-assistant_2023-04-14T01-07-13.302Z.log

The PR on python-zeep sadly didn't help. I'll try to find what broke it between v1.2.2 and v1.2.3 of onvif-zeep-async, since it's working with 1.2.2.

bdraco commented 1 year ago

There were some fixes for WSAs and memory leaks in 1.2.3 that are interacting poorly with that implementation. Unfortunately we haven't been able to pin down the cause yet though even after buying a few test cameras.

bdraco commented 1 year ago

I'd try reverting the WSAs PR first https://github.com/hunterjm/python-onvif-zeep-async/pull/12

Hopefully thats not it because if we revert that one its going to break a lot of motion detection sensors for other cameras but we may be able to work around that by only enabling it on the the event services as thats were the problem was for the other devices

denpamusic commented 1 year ago

Strangely enough reverting https://github.com/hunterjm/python-onvif-zeep-async/pull/11 fixed it. And I don't even use SSL with this NVR.

bdraco commented 1 year ago

Well thats not good as it means its likely a bug in httpx or anyio

bdraco commented 1 year ago

We are waiting for https://github.com/agronholm/anyio/issues/374 to get fixed as well

bdraco commented 1 year ago

I think we could revert https://github.com/hunterjm/python-onvif-zeep-async/pull/11 and instead pass it a dummy wsdl_client with the ssl context set to avoid the memory leak since it will avoid creating a new one which is how we end up with the issue

https://github.com/mvantellingen/python-zeep/blob/377d9313b1b4807a31a5ee42227f1dc7e7e0471e/src/zeep/transports.py#L186

If we already pass it one.

And since we never use it, its not really a problem.

Its just an ugly fix

bdraco commented 1 year ago

@denpamusic Can you try onvif-zeep-async to 1.2.5?

denpamusic commented 1 year ago

Yes, sorry for long wait, gone down rabbit hole of libraries :) Updated onvif-zeep-async to 1.2.5 and can confirm that everything works fine now. Thanks for the help, much appreciated. Hopefully it doesn't cause additional problems.

bdraco commented 1 year ago

Thanks for the help, much appreciated.

Thanks for testing

Hopefully it doesn't cause additional problems.

Let's hope, but with vendor implementations one has to hope quite a bit as every change seems to come with side effects due to the variety of implementations

JonNieveSP commented 1 year ago

Dear friends. I tried to solve this problem in my home assistant updating onvif-zeep-async to 1.2.5. After i tried to reconnect my camera and i obtain the same problem. What can I do to solve the problem?

bdraco commented 1 year ago

How did you install 1.2.5? Did you modify the manifest.json to prevent it from downgrading again?