Skip to content

Add reboot command to onvif camera component#29069

Closed
KoljaWindeler wants to merge 12 commits into
home-assistant:devfrom
KoljaWindeler:dev
Closed

Add reboot command to onvif camera component#29069
KoljaWindeler wants to merge 12 commits into
home-assistant:devfrom
KoljaWindeler:dev

Conversation

@KoljaWindeler
Copy link
Copy Markdown

Description:

Addition to send a Reboot command to an onvif compatible camera. mine used to hang up approximately every three weeks. Triggering this service once a week provides stable operation.

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.

@homeassistant
Copy link
Copy Markdown
Contributor

Hi @KoljaWindeler,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

Comment thread homeassistant/components/onvif/camera.py Outdated
Copy link
Copy Markdown
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please register the service under the onvif domain.

@MartinHjelmare MartinHjelmare changed the title added reboot command to onvif camera component Add reboot command to onvif camera component Nov 26, 2019
ret = await self._camera.devicemgmt.SystemReboot()
_LOGGER.debug("Camera '%s' Reboot command returned '%s'", self._name, ret)
except exceptions.ONVIFError as err:
if "Bad Request" in err.reason:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we log anything in other error cases?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true, fixed

camera for camera in all_cameras if camera.entity_id in entity_ids
]
for camera in target_cameras:
await camera.async_perform_reboot()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we speed this up by awaiting all camera calls concurrently, eg by using asyncio.gather?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possible i guess, but fairly unlikely if you ask me. To be honest: I'd improve it but don't know how asyncio,gather works.

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare Nov 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awaiting asyncio.gather(*coros) will schedule all coroutines on the event loop and return when all are done. Exceptions are either propagated immediately or returned in the result list.

https://docs.python.org/3/library/asyncio-task.html#asyncio.gather

Awaiting tasks in loops, like the for loop here, should be avoided if the task order doesn't matter and there's not a problem with executing the tasks "at the same time". But some devices or APIs can't handle this so in those cases it's ok to keep awaiting in the loop.

all_cameras = hass.data[ONVIF_DATA][ENTITIES]
entity_ids = await async_extract_entity_ids(hass, service)
target_cameras = []
if not entity_ids:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are dropping this logic for all other integrations in #29178. Let's not use it here.

@balloob
Copy link
Copy Markdown
Member

balloob commented Jan 10, 2020

This PR seems to have gone stale. Closing it.

@balloob balloob closed this Jan 10, 2020
@lock lock Bot locked and limited conversation to collaborators Jan 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants