Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect implementation of event detection for Nest cameras #92

Closed
jrluw opened this issue May 30, 2017 · 33 comments
Closed

Incorrect implementation of event detection for Nest cameras #92

jrluw opened this issue May 30, 2017 · 33 comments

Comments

@jrluw
Copy link

jrluw commented May 30, 2017

Event detection for Nestcams (as currently implemented) doesn't seem to work, at least in conjunction with homeassistant. Is this because the requirements for "is_ongoing" are too stringent? (Maybe the Nest API is very precise about events, and an event is only "ongoing" for a few milliseconds.) If that's the case, should this be fixed so that "ongoing" is more lenient? (E.g., cache the old last_event, check whether it differs from the new last_event. If so, and the time between the two is small enough, is_ongoing returns true.)

@jrluw
Copy link
Author

jrluw commented Jun 1, 2017

Would be great if you can say whether it's supposed to work. If not, I can try to fix it.

@jkoelker
Copy link
Owner

jkoelker commented Jun 2, 2017

I'm not sure, I don't use nestcams at all as I don't have the hardware. That said if its not working for whatever reason, and someone patches it, I'd accept the patches.

@ruthless0ne
Copy link

@jrluw did you ever get anywhere with this?

@awarecan
Copy link
Collaborator

awarecan commented Jun 5, 2018

@jrluw can you try python-nest 4.0.1 and home-assistant 0.71.0b0 (rc branch)

@jonas73x
Copy link

jonas73x commented Sep 1, 2018

this still doesn't seem to work for me at least. I have a Nest Cam IQ indoor and
print (napi.structures[0].cameras[0].person_detected)
print (napi.structures[0].cameras[0].motion_detected)
always returns False, despite me standing in front of the camera and the Nest app sending me "person detected notifications". Is this expected to work now? And, thanks for an awesome python module. love it.

@awarecan
Copy link
Collaborator

awarecan commented Sep 1, 2018

I have heard some other users said people detection not working when familiar face enabled on IQ and Hello. But motion detection should work.

The only camera I have is Nest Indoor, which is working fine.

Can you post the raw HTTP response in here?

@jonas73x
Copy link

jonas73x commented Sep 1, 2018

thank your for your quick reply. unfortunately i don't think i know how to get the raw HTTP response. tips very welcome otherwise i'll try to figure it out.

@jonas73x
Copy link

jonas73x commented Sep 1, 2018

not sure if this is what you are looking for but i did try to run some CURL calls and got this back:
event: put
data: {"path":"/devices/cameras/WHAT_LOOKS_TO_BE_A_TOKEN/last_event/has_person","data":true}

@awarecan
Copy link
Collaborator

awarecan commented Sep 1, 2018

So, there is no begin_time or end_time in the JSON. In that case we won't return person_detected.

Can you check

print(napi.structures[0].cameras[0].last_event.has_person)

See if it can back to False after you trigger it to True, or how quick it does.

@jonas73x
Copy link

jonas73x commented Sep 1, 2018

print(napi.structures[(napi.structu 0].cameras[0].last_event.has_person) turns to True as it's supposed to and seems to turn back to False about 45 seconds after the person has moved out of the camera view. And also assuming no other motion/event is detected. I can get the start end time with
https://developer-api.nest.com/devices/cameras/device_id/last_event/start_time
and
https://developer-api.nest.com/devices/cameras/device_id/last_event/end_time

@jonas73x
Copy link

jonas73x commented Sep 4, 2018

i think the problem is the time zone. checking both the start_time/end_time urls above as well as
napi.structures[0].cameras[0].last_event.start_time
napi.structures[0].cameras[0].last_event.end_time
they are in GMT time-zone while I'm actually in GMT+2.
assuming now = datetime.datetime.now(self.end_time.tzinfo) correctly picks up GMT +2 which then screws up the is_ongoing comparison. my address is correctly set in my account and the Nest app timeline shows the correct time so this seems to be an API issue. any suggestions/ideas how to fix it would be greatly appreciated.

@awarecan
Copy link
Collaborator

awarecan commented Sep 4, 2018

Maybe https://developer-api.nest.com/structures/structure_id/time_zone

@jonas73x
Copy link

jonas73x commented Sep 4, 2018

the strange thing is if i curl that URL i get:
event: put
data: {"path":"/structures/XXXX/time_zone","data":"Europe/Stockholm"}
which is correct, yet the times are wrong by the 2 hours.

@jonas73x
Copy link

jonas73x commented Sep 5, 2018

apologies for the many messages. i now have the timezone sorted. the person_detected still doesn't turn to true with the current code as end_time is always set to a couple of seconds after start_time regardless of if the event is still ongoing or not. i can however fix that with making the is_ongoing time check a bit more lenient. final question though, as i still can't seem to get the listening to event updates to work. is the "if you want to change to push mode..." example on the pypi.org/project/python-nest/ page supposed to work? if i try that code i get no output whatsoever despite motion_detected now being triggered to true. it just sits in the endless loop and i have to kill it.

@thegame3202
Copy link

thegame3202 commented Sep 27, 2018

So I just spent a fair amount of time trying to figure this out (I'm not a coder). However, it seems that HomeAssistant is looking for the binary_sensor at THAT SPECIFIC POINT IN TIME. Since the IQ cams tend to only show motion_detected for a few seconds TOPS, I don't think HomeAssistant is seeing those events (As you said @jonas73x ).

Hopefully this helps someone. Happy to help with any testing needed.

Quick edit: What if we did "If start_time and end_time are within last 30 seconds, return state". That should work, right?

@jonas73x
Copy link

when i did some testing even only a 5 or 10 second interval seemed to be enough to make it trigger to true.

@mikestaniforth
Copy link

when i did some testing even only a 5 or 10 second interval seemed to be enough to make it trigger to true.

Did you manage to get it working? Would you mind posting the code if so?

@rodhall
Copy link

rodhall commented Dec 8, 2018

I have a Nest Hello and an IQ Cam Outdoor. Weirdly, sound detection seems to work but motion and person detection do not.

@twinchoke
Copy link

I also have a Nest Hello and none of the sensors are working

@snicker
Copy link
Collaborator

snicker commented Dec 26, 2018

Ran into this issue starting a few weeks ago and could not figure out why. Recently installed a Nest Hello and sensors seemed to be working OK in HomeAssistant at first, but suddenly died out.

After reading this thread and doing some testing, I can say that the Nest API is to blame in an indirect way. This is due to the delay in the Nest API reporting events and the end_time of the events occurring before the event is received by any subscribers. This means the check in CameraEvent.is_ongoing almost always fails.

I don't have a "correct" answer to this other than adding a 10-30 second buffer to event end_time reported by the API. Not sure if that's the direction @jkoelker wants to drive the project.

In the meantime, I was able to hack this up for my HomeAssistant instance and monkey patch the end_time property on CameraEvent, and now all sensors are working properly (motion, person, and sound):

from nest.nest import CameraEvent
from datetime import timedelta

_pre_patch = CameraEvent.end_time

def end_time(self):
    time = self.pre_patch_end_time
    if time:
        return time + timedelta(seconds = 30)

CameraEvent.pre_patch_end_time = _pre_patch
CameraEvent.end_time = property(end_time)

To add this into HomeAssistant (sorry to muddle up your issue with another project @jkoelker), grab the following file from my repository, drop it in your custom_components folder and enable it in your configuration.yaml by adding monkey_patch_nest_camera_event_end_date: on a new line.

https://github.com/snicker/twohunnid_ha/blob/master/custom_components/monkey_patch_nest_camera_event_end_date.py

@jkoelker
Copy link
Owner

@snicker If it fixes the issue, then I'm all for it ;) HomeAssistant is the largest consumer of this library that I know of, so getting it working there is a major win.

@snicker
Copy link
Collaborator

snicker commented Dec 27, 2018

cool, happy to throw together a PR once we get some feedback from others. It seems like this might be isolated to only certain camera models, so I don't want to make a sweeping change for all models if we can limit it to just the Hello/IQ.

@twinchoke
Copy link

Hi, I've added the patch to my system and sensors now appear to trigger. However, once motion detection is triggered to on, it seems to stay on and doesn't return to off. Is this correct operation. Thanks.

@snicker
Copy link
Collaborator

snicker commented Dec 27, 2018

@twinchoke: what model of camera are you using?

@twinchoke
Copy link

@snicker I'm using a Nest Hello and it would appear that after a reboot, the sensors are returning to the off state. I'm just testing at the moment with person detection.

@medic459
Copy link

I'm seeing the same behavior (with a nest hello)..
Still...it's progress?

@wrkn4alivn
Copy link

@snicker Thanks for the patch, that resolved the issue for me as well. FYI I was getting no sensor events at all in HA with my Nest Hello, but I also noticed that every once in a while I was missing events from my original Outdoor cams.

@jhdedrick
Copy link

I have two "Nest Cam IQ Outdoor" and a Nest Hello. I'm running a manual install of HA on Win 10 under Python 3.7. All cameras appear to trigger properly with this patch.

(I just had to replace two instances of "hass" with "self" in the Python script.)

@tankman316
Copy link

@snicker - thank you for the patch. This also fixed the issue for me. Was only impacting my Nest Outdoor IQ cam. My non-IQ cam was fine.

@galbelli
Copy link

Thanks for the patch @snicker - worked great for Nest IQ outdoor.

@rodhall
Copy link

rodhall commented Jan 16, 2019

This is working for both my IQ Outdoor and my Nest Hello. I just implemented so will run for a while and report back any stability issues. Both motion and person detection are working now.

@snicker
Copy link
Collaborator

snicker commented Jan 16, 2019

sweet! sorry for the delay all, I just wanted to get a few more positive pieces of reinforcement here. I'll submit a PR this evening (or maybe tomorrow.. life is very demanding at the moment!)

@Stonewallace
Copy link

Just found this thread after having the same issue as above. Sensors Person and Motion not working before the patch was applied. Working perfectly after patch. Sensors even return to "Clear" after about a minute or so. Hassio 0.85.1 running in Docker/Ubuntu, Nest Hello doorbell.

jkoelker added a commit that referenced this issue Jan 18, 2019
add 30 seconds to end_time of CameraEvent. Fixes #92 and #154 ... sorta
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests