From 2ec33ea589edd0149bc1783527111aa3b33ab3ec Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 4 May 2023 15:45:07 -0500 Subject: [PATCH] Improve reliablity of onvif subscription renewals upstream changelog: https://github.com/hunterjm/python-onvif-zeep-async/compare/v2.0.0...v2.1.0 --- homeassistant/components/onvif/event.py | 75 +++++--------------- homeassistant/components/onvif/manifest.json | 2 +- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- 4 files changed, 21 insertions(+), 60 deletions(-) diff --git a/homeassistant/components/onvif/event.py b/homeassistant/components/onvif/event.py index 92f76b6a950..9b473f6b788 100644 --- a/homeassistant/components/onvif/event.py +++ b/homeassistant/components/onvif/event.py @@ -9,7 +9,7 @@ import datetime as dt from aiohttp.web import Request from httpx import RemoteProtocolError, RequestError, TransportError from onvif import ONVIFCamera, ONVIFService -from onvif.client import NotificationManager +from onvif.client import NotificationManager, retry_connection_error from onvif.exceptions import ONVIFError from zeep.exceptions import Fault, ValidationError, XMLParseError @@ -327,20 +327,7 @@ class PullPointManager: async def _async_start_pullpoint(self) -> bool: """Start pullpoint subscription.""" try: - try: - started = await self._async_create_pullpoint_subscription() - except RequestError: - # - # We should only need to retry on RemoteProtocolError but some cameras - # are flaky and sometimes do not respond to the Renew request so we - # retry on RequestError as well. - # - # For RemoteProtocolError: - # http://datatracker.ietf.org/doc/html/rfc2616#section-8.1.4 allows the server - # to close the connection at any time, we treat this as a normal and try again - # once since we do not want to declare the camera as not supporting PullPoint - # if it just happened to close the connection at the wrong time. - started = await self._async_create_pullpoint_subscription() + started = await self._async_create_pullpoint_subscription() except CREATE_ERRORS as err: LOGGER.debug( "%s: Device does not support PullPoint service or has too many subscriptions: %s", @@ -382,6 +369,7 @@ class PullPointManager: finally: self.async_schedule_pullpoint_renew(next_attempt) + @retry_connection_error() async def _async_create_pullpoint_subscription(self) -> bool: """Create pullpoint subscription.""" @@ -447,6 +435,11 @@ class PullPointManager: ) self._pullpoint_subscription = None + @retry_connection_error() + async def _async_call_pullpoint_subscription_renew(self) -> None: + """Call PullPoint subscription Renew.""" + await self._pullpoint_subscription.Renew(SUBSCRIPTION_RELATIVE_TIME) + async def _async_renew_pullpoint(self) -> bool: """Renew the PullPoint subscription.""" if ( @@ -458,20 +451,7 @@ class PullPointManager: # The first time we renew, we may get a Fault error so we # suppress it. The subscription will be restarted in # async_restart later. - try: - await self._pullpoint_subscription.Renew(SUBSCRIPTION_RELATIVE_TIME) - except RequestError: - # - # We should only need to retry on RemoteProtocolError but some cameras - # are flaky and sometimes do not respond to the Renew request so we - # retry on RequestError as well. - # - # For RemoteProtocolError: - # http://datatracker.ietf.org/doc/html/rfc2616#section-8.1.4 allows the server - # to close the connection at any time, we treat this as a normal and try again - # once since we do not want to mark events as stale - # if it just happened to close the connection at the wrong time. - await self._pullpoint_subscription.Renew(SUBSCRIPTION_RELATIVE_TIME) + await self._async_call_pullpoint_subscription_renew() LOGGER.debug("%s: Renewed PullPoint subscription", self._name) return True except RENEW_ERRORS as err: @@ -655,6 +635,7 @@ class WebHookManager: self._renew_or_restart_job, ) + @retry_connection_error() async def _async_create_webhook_subscription(self) -> None: """Create webhook subscription.""" LOGGER.debug( @@ -689,20 +670,7 @@ class WebHookManager: async def _async_start_webhook(self) -> bool: """Start webhook.""" try: - try: - await self._async_create_webhook_subscription() - except RequestError: - # - # We should only need to retry on RemoteProtocolError but some cameras - # are flaky and sometimes do not respond to the Renew request so we - # retry on RequestError as well. - # - # For RemoteProtocolError: - # http://datatracker.ietf.org/doc/html/rfc2616#section-8.1.4 allows the server - # to close the connection at any time, we treat this as a normal and try again - # once since we do not want to declare the camera as not supporting webhooks - # if it just happened to close the connection at the wrong time. - await self._async_create_webhook_subscription() + await self._async_create_webhook_subscription() except CREATE_ERRORS as err: self._event_manager.async_webhook_failed() LOGGER.debug( @@ -720,6 +688,12 @@ class WebHookManager: await self._async_unsubscribe_webhook() return await self._async_start_webhook() + @retry_connection_error() + async def _async_call_webhook_subscription_renew(self) -> None: + """Call PullPoint subscription Renew.""" + assert self._webhook_subscription is not None + await self._webhook_subscription.Renew(SUBSCRIPTION_RELATIVE_TIME) + async def _async_renew_webhook(self) -> bool: """Renew webhook subscription.""" if ( @@ -728,20 +702,7 @@ class WebHookManager: ): return False try: - try: - await self._webhook_subscription.Renew(SUBSCRIPTION_RELATIVE_TIME) - except RequestError: - # - # We should only need to retry on RemoteProtocolError but some cameras - # are flaky and sometimes do not respond to the Renew request so we - # retry on RequestError as well. - # - # For RemoteProtocolError: - # http://datatracker.ietf.org/doc/html/rfc2616#section-8.1.4 allows the server - # to close the connection at any time, we treat this as a normal and try again - # once since we do not want to mark events as stale - # if it just happened to close the connection at the wrong time. - await self._webhook_subscription.Renew(SUBSCRIPTION_RELATIVE_TIME) + await self._async_call_webhook_subscription_renew() LOGGER.debug("%s: Renewed Webhook subscription", self._name) return True except RENEW_ERRORS as err: diff --git a/homeassistant/components/onvif/manifest.json b/homeassistant/components/onvif/manifest.json index 9fc0d417838..c16147cc629 100644 --- a/homeassistant/components/onvif/manifest.json +++ b/homeassistant/components/onvif/manifest.json @@ -8,5 +8,5 @@ "documentation": "https://www.home-assistant.io/integrations/onvif", "iot_class": "local_push", "loggers": ["onvif", "wsdiscovery", "zeep"], - "requirements": ["onvif-zeep-async==2.0.0", "WSDiscovery==2.0.0"] + "requirements": ["onvif-zeep-async==2.1.0", "WSDiscovery==2.0.0"] } diff --git a/requirements_all.txt b/requirements_all.txt index c4889227f02..1b08bf1bf6f 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -1258,7 +1258,7 @@ ondilo==0.2.0 onkyo-eiscp==1.2.7 # homeassistant.components.onvif -onvif-zeep-async==2.0.0 +onvif-zeep-async==2.1.0 # homeassistant.components.opengarage open-garage==0.2.0 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index f3cf2a64a6c..bb6958a6752 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -945,7 +945,7 @@ omnilogic==0.4.5 ondilo==0.2.0 # homeassistant.components.onvif -onvif-zeep-async==2.0.0 +onvif-zeep-async==2.1.0 # homeassistant.components.opengarage open-garage==0.2.0