search

starkillerOG / reolink_aio

Reolink NVR/camera API PyPI package
MIT License
65 stars 13 forks source link

Enable webhook usage through external application #56

Open nicolasbrailo opened 7 months ago

nicolasbrailo commented 7 months ago

The current API doesn't expose a way to parse an ONVIF message from the camera directly, without managing the connection itself. This makes it harder to:

This commit [PR] introduces a helper class that can parse a Reolink ONVIF message into a somewhat self-descriptive Python dictionary. This helper can be used from reolink_aio itself, or from a 3p application that only needs to parse Reolink ONVIF messages.

This PR also includes an example of how to use reolink_aio to manage camera subscriptions with a webhook to an external server.

nicolasbrailo commented 7 months ago

@starkillerOG from PR #52:

Regarding the example, would it be possible to combine both python scripts into 1: Have the webserver run first in the eventloop (webhook_cat.py) Then run the trigger_subscription I think it would also be nice if we could use the aiohttp web instead of flask, than we do not have to install any aditional dependency besides reolink_aio for the example to work.

That's an absolutely reasonable request. Unfortunately I'm not familiar with aiohttp, and what ChatGPT was able to come up with doesn't work very well with event loops from different coros. Fixing it is beyond the time I have available for this PR (at least for a usage example).

If the base commit (0a63b98 - adding an external onvif parser) is useful, I'd propose accepting only that, and leaving the usage example open until someone has time to figure out how to port it to aiohttp. How does that sound?

Also @starkillerOG @ PR #52:

keep the ONVIF_event_callback function unchanged since that has been extensively tested. So please first explain why a change would be needed there.

Also reasonable. I wouldn't say a change is needed, I would say that

  1. The current implementation conflates state-tracking and ONVIF parser into ONVIF_event_callback.
  2. If you need to use reolink_aio to manage the camera state, but not as a webhook callback, then you need to roll out your own Reolink ONVIF parser

By splitting ONVIF_event_callback into state + parsing, it becomes possible to reuse parsing for third party apps, while cleaning up ONVIF_event_callback to have less responsibilities.

I can't judge if the risk of changing this function is worth it, so I haven't included this particular commit into the PR. Happy to create a new PR if this one gets merged, and you think it may be valuable.

dough29 commented 6 months ago

Hello, thank you for this PR !

Looks like it's what I'm looking for 😎

nicolasbrailo commented 6 months ago

Hello, thank you for this PR !

Looks like it's what I'm looking for 😎

Thanks @dough29, I'm glad it's useful. The example in this PR is built around flask, but this project uses aiohttp - if you're brave enough to update the PR with aiohttp it may be useful for @starkillerOG to take in the PR too?

dough29 commented 6 months ago

@nicolasbrailo I'm not quite in ease with Python and those libs.

My need is to have a deamon running so it reads states from my Reolink Dorbell and sends webhook back to my home automation.

I'd be happy to help when I get time to.