tsightler / ring-mqtt

Ring devices to MQTT Bridge
MIT License
592 stars 106 forks source link

Feature Request: Update Snapshots #138

Closed ronaldheft closed 3 years ago

ronaldheft commented 3 years ago

Awesome work on adding snapshots! Could snapshot requesting be configurable in some manner? For example, either updating on a set interval or requesting an update via an MQTT message? Thanks!

ballakers commented 3 years ago

No problem, thanks for clarifying and bearing with me - I read most of the comments but must have missed that bit or misunderstood - wasn't too worried and just wanted to mention it. I'll wait for you to push it to the main branch of course - don't want you to do un-needed work for a few of us. When pushed to the main branch I'm guessing it will show as an update to the add-on correct?

Thanks again for this especially if you don't use the add-on yourself.

broyuken commented 3 years ago

You can use my fork for now

https://github.com/broyuken/ring-mqtt-ha-addon

tsightler commented 3 years ago

I really appreciate the assist on that @broyuken

ronaldheft commented 3 years ago

I pulled down the latest dev branch in my install and I'll report back if I find anything. Thanks @tsightler!

ronaldheft commented 3 years ago

Alright, seeing this error in my logs, and I haven't received an updated snapshot since pulling down the new version:

2021-02-17T21:26:40.425Z ring-mqtt Error: Snapshot for Driveway failed to refresh after 70 attempts.  This is normal behavior since this camera is unable to capture snapshots while streaming
    at RingCamera.<anonymous> (/app/ring-mqtt-dev/node_modules/ring-client-api/lib/api/ring-camera.js:364:19)
    at Generator.next (<anonymous>)
    at fulfilled (/app/ring-mqtt-dev/node_modules/ring-client-api/lib/api/ring-camera.js:5:58)

I am obviously not streaming anything from my camera right now.

tsightler commented 3 years ago

That error message is from the underlying ring-client-api, so I'm not sure what I can do about it. I just call getSnapshot() it's up to ring-client-api to give me a snapshot back. For battery cameras it tries 70 times with a delay of 1/2 second each time, so 35 seconds and, if it can't get one, it spits out this message. Battery cameras can't take a snapshot while it's recording so if there's a motion even that starts recording this message might pop up I guess if the snapshot happens to occur at the same time.

ronaldheft commented 3 years ago

@tsightler Thanks for investigating and the info. My cameras are back up. I realized my detection for when the camera are updated broke with the latest changes, and seeing that in the logs led me to believe they weren't updating. They are fine, just my custom logic around them needed to be changed (using the new timestamp... yay!).

tsightler commented 3 years ago

Hi all. As this has been running reliably for me for over a week, and I've heard no other reports of issues, I decided to push out 4.3.2 of both ring-mqtt and the addon which includes this support. Note that there is one change in the configuration, instead of "enable_snapshot", the option for controlling this mode is now called "snapshot_mode" and can set to "disabled" (default), "motion", "interval", or "all".

Due to the way Home Assistant handles configuration options during upgrades, the old enable_snapshot parameter may still be in the configuraiton, but should be ignored. It can be manually removed with no side effects.

I've tested it on both of my development HA instances and as well as my production HA instance, so hopefully it will be OK. As this addresses the initial request I'm going to close this issue now. If there are bugs that need to be addressed, please open a new issue.