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

Add support for Amcrest alarm_triggered to binary_sensors #30932

Open
wants to merge 2 commits into
base: dev
from

Conversation

@lonestriker
Copy link

lonestriker commented Jan 18, 2020

On my Amcrest AD110 Video Doorbell Camera, the normal Home Assistant amcrest component's motion_detected binary sensor does not match the actual motion detection that's presented through the Amcrest iOS app. The motion_detected binary sensor appears to map to just the video motion detection that is just one component of the actual detection. The doorbell under the covers seemingly merges the video motion with the PIR motion data to determine if a motion detection alert should be sent to the smartphone app, recorded locally in the microSD and/or sent to their cloud.

The AlarmLocal event accessible via amcrest-python appears analogous to the actual motion detection on the doorbell. I've successfully used this updated change as a custom_component and verified that alarm_triggered events correspond to actual motion detection events sent to my smartphone.

Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#<home-assistant.io PR number goes here>

Example entry for configuration.yaml (if applicable):

amcrest:
  - host: 192.168.1.100
    name: Doorbell
    binary_sensors:
      - alarm_triggered

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox.
  • 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.
  • New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.
lonestriker added 2 commits Jan 18, 2020
…inary sensor

Add alarm_triggered binary alarm.  On my Amcrest AD110 Video Doorbell Camera, the normal Home Assistant motion_detected binary sensor does not match the actual motion detection that's presented through the Amcrest iOS app. The motion_detected binary sensor maps to the video motion detection that is just one component of the detection. The doorbell seemingly merges the video motion with the PIR motion data to determine if a motion detection alert should be sent to the smartphone app, recorded locally in the microSD and/or sent to their cloud.

The AlarmLocal event accessible via amcrest-python is analogous to the actual motion detection on the doorbell.
@homeassistant

This comment has been minimized.

Copy link
Contributor

homeassistant commented Jan 18, 2020

Hi @lonestriker,

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!

@digiblur

This comment has been minimized.

Copy link
Contributor

digiblur commented Jan 23, 2020

Very nice @lonestriker I will test this one out. I was digging through the various alarms looking for the button too thinking it was tied into an input. Thanks for doing this PR!

@pnbruckner

This comment has been minimized.

Copy link
Contributor

pnbruckner commented Jan 29, 2020

@lonestriker just an FYI, I've submitted #30843, which is also changing binary_sensor.py (among other parts.) I've also done the work (but haven't submitted it in a PR yet, since I'm waiting on 30843 to get approved & merged), that will convert the motion_detected (i.e., VideoMotion) binary sensor from polling to an event stream. It's generic enough to make it extremely easy to add other event based binary sensors, such as AlarmLocal, as you've added here. If this PR isn't accepted by the time I submit the second PR I described above, I can go ahead and add the AlarmLocal binary sensor (as well as update the docs, which is also required.)

@lonestriker

This comment has been minimized.

Copy link
Author

lonestriker commented Jan 30, 2020

Thanks @pnbruckner. No hurry from my end and if there's a better mechanism in the future to go pub/sub instead of polling, that's a much better solution. I'm running my modified code locally, so I'm covered. Was just trying to get it out for others to benefit and not get false positive motion events. Thanks for the review and update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.