Amcrest switch platform by dparker18 · Pull Request #7876 · home-assistant/core

dparker18 · 2017-06-03T02:34:46Z

Description:

Added new amcrest switch platform to control your camera's motion detection setting. For example, disable it based on device presence.

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#2745

Example entry for `configuration.yaml` (if applicable):

# Example configuration.yaml entry
switch:
  - platform: amcrest
    host: IP_ADDRESS
    username: USERNAME
    password: PASSWORD

Checklist:

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

Documentation added/updated in home-assistant.github.io

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

Local tests with tox run successfully. Your PR cannot be merged unless tests pass
New dependencies have been added to the REQUIREMENTS variable ([example][ex-requir]).
New dependencies are only imported inside functions that use them ([example][ex-import]).
New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
New files were added to .coveragerc.

homeassistant · 2017-06-03T02:34:48Z

Hi @dparker18,

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!

mention-bot · 2017-06-03T02:34:51Z

@dparker18, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob, @fabaff and @robbiet480 to be potential reviewers.

houndci-bot · 2017-06-03T02:34:51Z

+        _LOGGER.debug("Pulling Motion Detection data from %s sensor.", self._name)
+        detection = self._camera.is_motion_detector_on()
+        self._state = STATE_ON if detection else STATE_OFF
+        #self._attrs['Motion Detection Config'] = self._camera.motion_detection


block comment should start with '# '

houndci-bot · 2017-06-03T02:34:51Z

+
+    def update(self):
+        """Update Motion Detection state."""
+        _LOGGER.debug("Pulling Motion Detection data from %s sensor.", self._name)


line too long (82 > 79 characters)

houndci-bot · 2017-06-03T02:34:52Z

+        self._camera.motion_detection = 'false'
+
+
+    def update(self):


too many blank lines (2)

houndci-bot · 2017-06-03T02:34:52Z

+        self._camera.motion_detection = 'true'
+
+
+    def turn_off(self, **kwargs):


too many blank lines (2)

houndci-bot · 2017-06-03T02:34:52Z

+        return self._state == STATE_ON
+
+
+    def turn_on(self, **kwargs):


too many blank lines (2)

houndci-bot · 2017-06-03T02:34:52Z

+        self._state = STATE_UNKNOWN
+
+
+    @property


too many blank lines (2)

houndci-bot · 2017-06-03T02:34:52Z

+        #self._attrs = {}
+        self._camera = camera
+        self._name = device_info.get(CONF_NAME)
+        #self._icon = 'mdi:{}'.format(SENSOR_TYPES.get(self._sensor_type)[2])


block comment should start with '# '

houndci-bot · 2017-06-03T02:34:52Z

+
+    def __init__(self, device_info, camera):
+        """Initialize the switch."""
+        #self._attrs = {}


block comment should start with '# '

houndci-bot · 2017-06-03T02:34:52Z

+    """Representation of a switch to toggle on/off motion detection."""
+
+
+    def __init__(self, device_info, camera):


too many blank lines (2)

houndci-bot · 2017-06-03T02:34:52Z

+
+import homeassistant.helpers.config_validation as cv
+from homeassistant.components.sensor import PLATFORM_SCHEMA
+from homeassistant.const import (CONF_HOST, CONF_NAME, CONF_USERNAME, CONF_PASSWORD, CONF_PORT, STATE_UNKNOWN, STATE_OFF, STATE_ON)


line too long (131 > 79 characters)

houndci-bot · 2017-06-03T02:40:24Z

+    def update(self):
+        """Update Motion Detection state."""
+        _LOGGER.debug("Pulling Motion Detection data from %s sensor.",
+            self._name)


continuation line under-indented for visual indent

tchellomello

Nice PR!!

tchellomello · 2017-06-03T04:25:43Z

+        config.get(CONF_HOST), config.get(CONF_PORT),
+        config.get(CONF_USERNAME), config.get(CONF_PASSWORD)).camera
+
+    try:


@dparker18 could you add the persistent_notification feature as https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/sensor/amcrest.py#L59-L70

balloob · 2017-06-05T06:15:47Z

I don't want to add any more amcrest platforms until the amcrest auth is generalized into a component instead of requiring each platform get the auth set in it's config.

amcrest:
  username: XX
  password: YY

sensor:
  platform: amcrest

camera:
  platform: amcrest

dparker18 · 2017-06-05T06:22:22Z

I can take a stab at it. Is there another component with similar structure you'd recommend looking at?

tchellomello · 2017-06-05T14:59:02Z

@balloob yes that makes sense. @dparker18 you can take a look at the https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/android_ip_webcam.py or https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/arlo.py

@dparker18 let me know if you need any help

cmsimike · 2017-06-07T15:30:42Z

I have been looking forward to this for a while. Thank you for your work!

tchellomello · 2017-06-12T20:43:53Z

@dparker18 I've been working on a new sensor for Amcrest platform too.
I just want to know if you already started to migrate the platform for your PR as requested by @balloob. If not, I'll create a PR with the changes here and then you can adapt your PR to the new format.

If you already started it, I'll wait for your PR and then I'll add the new sensor later.

Thank you!

dparker18 · 2017-06-13T03:07:48Z

Awesome, I have not started yet. Was hoping to this weekend, sounds like you may beat me to it!

…

Sent from my phone

On Jun 12, 2017, at 4:44 PM, Marcelo Moreira de Mello ***@***.***> wrote: @dparker18 I've been working on a new sensor for Amcrest platform too. I just want to know if you already started to migrate the platform for your PR as requested by @balloob. If not, I'll create a PR with the changes here and then you can adapt your PR to the new format. If you already started it, I'll wait for your PR and then I'll add the new sensor later. Thank you! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

tchellomello · 2017-06-24T07:04:11Z

@dparker18
I've submitted the PR #8184 that converted the Amcrest component to use a central hub.
If that gets approved, then you can adjust your code. Looking forward to have the switch functionality proposed by you.
Thank you!

balloob · 2017-07-14T03:05:14Z

@dparker18 the PR by @tchellomello has been merged. Do you have any plans to finish this PR ?

dparker18 · 2017-07-14T03:09:45Z

Definitely! Will look at it this weekend

tchellomello · 2017-07-26T08:09:01Z

@dparker18 are you planning to try to include this on 0.50 version? Do you need any help?

balloob · 2017-08-12T16:40:19Z

This PR seems to have gone stale. Closing it. You can reopen it when you're ready to finish it.

dparker18 added 3 commits June 2, 2017 15:06

working amcrest motion detection switch!

6db93fe

add amcrest switch coverage and reqs

d196144

amcrest switch cleanup

a03b484

homeassistant added new-platform platform: switch.amcrest cla-needed labels Jun 3, 2017

houndci-bot reviewed Jun 3, 2017

View reviewed changes

homeassistant added cla-signed and removed cla-needed labels Jun 3, 2017

style fixes

64b0717

balloobbot added platform requirement labels Jun 3, 2017

houndci-bot reviewed Jun 3, 2017

View reviewed changes

style fixes

1645903

homeassistant added the cla-signed label Jun 3, 2017

tchellomello reviewed Jun 3, 2017

View reviewed changes

dparker18 added 2 commits June 2, 2017 22:55

added persistent_notification back

f6aed01

added persistent_notification back

11efcc3

homeassistant added the cla-signed label Jun 3, 2017

tchellomello mentioned this pull request Jun 24, 2017

Refactored Amcrest to use central hub component #8184

Merged

6 tasks

balloob closed this Aug 12, 2017

home-assistant locked and limited conversation to collaborators Dec 11, 2017

ghost added integration: amcrest and removed platform: switch.amcrest labels Mar 21, 2019

		self._camera.motion_detection = 'true'


		def turn_off(self, **kwargs):

		"""Representation of a switch to toggle on/off motion detection."""


		def __init__(self, device_info, camera):

Uh oh!

Conversation

dparker18 commented Jun 3, 2017

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Uh oh!

homeassistant commented Jun 3, 2017

Uh oh!

mention-bot commented Jun 3, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tchellomello left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

balloob commented Jun 5, 2017

Uh oh!

dparker18 commented Jun 5, 2017

Uh oh!

tchellomello commented Jun 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmsimike commented Jun 7, 2017

Uh oh!

tchellomello commented Jun 12, 2017

Uh oh!

dparker18 commented Jun 13, 2017 via email

Uh oh!

tchellomello commented Jun 24, 2017

Uh oh!

balloob commented Jul 14, 2017

Uh oh!

dparker18 commented Jul 14, 2017 via email

Uh oh!

tchellomello commented Jul 26, 2017

Uh oh!

balloob commented Aug 12, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Example entry for `configuration.yaml` (if applicable):

tchellomello commented Jun 5, 2017 •

edited

Loading