Camera services arm disarm including Netgear Arlo #7961

viswa-swami · 2017-06-08T16:16:00Z

Description:

Camera component needs a service called "Arm/Disarm" to be able to arm a camera on demand or via automation. Similarly for disarm.

Arm: It will enable motion detection and also record the video/snapshot depending on camera setting
Disarm: It will disable motion detection and will not record anything.

The arguments (entity_id) is optional so that with just 'arm' or 'disarm' call, user can arm/disarm all the cameras in a shot. Otherwise provide the specific entity_id of camera to arm/disarm if camera supports it.

This goes hand-in-hand with arlo camera where home assistant talks to the Arlo base station (Note that, for Arlo, we need to talk to base station/hub to control the cameras and not the cameras itself) to arm/disable all the cameras at once, for example.

These services are applicable to Foscam cameras too. For example, in my home setup, I currently have a shell command to arm/disarm the camera whenever we are not in home. This shell command can be avoided if the arm/disarm service is available in camera component itself. I can add the arm/disarm functionality for foscam cameras also once this codeset is done/committed/pushed.

I am not sure if we need to update the documentation anywhere on this concern. If there is any, let me know and i can update the same.

These status can be used to create a camera based sensor to indicate whether the cameras are armed or disarmed. That is next once these code are done.

Related issue (if applicable): fixes #

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

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

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).
New dependencies are only imported inside functions that use them (example).
New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
New files were added to .coveragerc.

If the code does not interact with devices:

Local tests with tox run successfully. Your PR cannot be merged unless tests pass
Tests have been added to verify that the new code works.

…tional so that with a single call we can arm all the cameras or specify a particular entity id to arm if applicable and possible in that camera type.

mention-bot · 2017-06-08T16:16:02Z

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

houndci-bot · 2017-06-08T16:16:07Z

homeassistant/components/camera/__init__.py

+    """Disarm all"""
+    hass.add_job(async_disarm, hass, entity_id)
+
+@callback


expected 2 blank lines, found 1

houndci-bot · 2017-06-08T16:16:08Z

homeassistant/components/camera/__init__.py

+    data =  {ATTR_ENTITY_ID: entity_id} if entity_id else None
+    hass.async_add_job(hass.services.async_call(DOMAIN, SERVICE_ARM, data))
+
+def disarm(hass, entity_id=None):


expected 2 blank lines, found 1

houndci-bot · 2017-06-08T16:16:08Z

homeassistant/components/camera/__init__.py

+@callback
+def async_arm(hass, entity_id=None):
+    """Arm all the cameras"""
+    data =  {ATTR_ENTITY_ID: entity_id} if entity_id else None


multiple spaces after operator

houndci-bot · 2017-06-08T16:16:08Z

homeassistant/components/camera/__init__.py

+    """Arm all"""
+    hass.add_job(async_arm, hass, entity_id)
+
+@callback


expected 2 blank lines, found 1

houndci-bot · 2017-06-08T16:16:08Z

homeassistant/components/camera/__init__.py

+    vol.Optional(ATTR_ENTITY_ID): cv.entity_ids,
+})
+
+def arm(hass, entity_id=None):


expected 2 blank lines, found 1

houndci-bot · 2017-06-08T16:20:06Z

homeassistant/components/camera/__init__.py

+    data = {ATTR_ENTITY_ID: entity_id} if entity_id else None
+    hass.async_add_job(hass.services.async_call(DOMAIN, SERVICE_ARM, data))
+
+def disarm(hass, entity_id=None):


expected 2 blank lines, found 1

viswa-swami · 2017-06-08T19:28:07Z

I just now read and found that I can edit the arlo component page and add the arm/disarm service. Please suggest if that is required to be added there or anywhere else would be appropriate.

pvizeli

Please use alarm_control_panel platform for arlo instead to copy that into camera platform.

viswa-swami · 2017-06-09T12:01:31Z

Arm/Disarm is just the same wording used between alarm and camera, but why would we need the entire control panel for a simple arm/disarm? This would seem to keep the component clean instead of adding a full fledged panel right?

viswa-swami · 2017-06-09T18:39:30Z

This arm/disarm service in camera component is quite useful in arming/disarming the cameras directly as a service or via automation then using it via alarm control panel platform.

viswa-swami · 2017-06-10T16:44:35Z

@pvizeli this is a generic arm/disarm for all cameras including foscam, amcrest and Arlo too. Currently I am using them by adding a separate python script and calling them using the shell_command platform/service.

Adding this would make it self reliant instead of each user needing to add scripts on their own and calling them.

pvizeli · 2017-06-13T15:43:13Z

I'll speak with @balloob. But you need write tests for that and add this to demo camera too. It also not allow to catch AttributeError on that point. So it need more work for add this into base component. I think that should be only a part of the platform and not inside all base component.

viswa-swami · 2017-06-13T21:44:10Z

@pvizeli thank you. I have some clarifications based on your comment. Can you please review and suggest accordingly.

Regarding tests:
Where do i need to write the tests ? Is there any format or document or any link where i should update them ?

Demo camera:
I am not very clear on what to add for the demo camera. I am assuming it is the components/camera/demo.py. Is that right? If so, I can add these async_arm() and async_disarm() to this as an empty function !?

AttributeError:
I added this exception because I hadn't added the async_arm() and async_disarm() for other cameras at that time. I have foscam and arlo cameras at my place. I can add the code for foscamm camera too. But for other camera models, I can add an empty function async_arm() and async_disarm(). That way, anyone who has those camera models can add them !?

balloob

Adding generic services to a component means that you will have to update the demo camera platform and write tests to make sure that everything works.

I am not 100% convinced that arm/disarm functionality is something that most camera's have. For example with Nest Cam you mute notifications, alerts are always on.

That's why it might be easier for now to add platform specific services: arlo_arm, arlo_disarm. You would register these services inside your platform, that way you don't have to write tests, update the demo platform etc.

balloob · 2017-06-15T05:42:35Z

homeassistant/components/camera/__init__.py

@@ -125,6 +199,16 @@ def brand(self):
        return None

    @property
+    def status(self):


Why add something new if we already have state that does something similar?

balloob · 2017-06-15T05:44:34Z

homeassistant/components/camera/arlo.py

+    @asyncio.coroutine
+    def async_arm(self):
+        """Camera arm."""
+        self._base_stn.mode = "armed"


You are not allowed to do I/O inside async methods.

Is there any example as to how to handle I/O, if we cannot do it inside the async methods ?

balloob · 2017-06-15T05:44:40Z

homeassistant/components/camera/arlo.py

+    @asyncio.coroutine
+    def async_disarm(self):
+        """Camera disarm."""
+        self._base_stn.mode = "disarmed"


Is there any example as to how to handle I/O, if we cannot do it inside the async methods ?

def set_mode(mode): self._base_stn.mode = mode yield from hass.async_add_job(set_mode, "disarmed")

balloob · 2017-06-15T05:45:00Z

homeassistant/const.py

@@ -315,6 +315,9 @@
 SERVICE_HOMEASSISTANT_STOP = 'stop'
 SERVICE_HOMEASSISTANT_RESTART = 'restart'

+SERVICE_ARM = 'arm'


Please keep these in the component

I didn't see a const.py inside the component/ directory. Shall I just create a const.py inside components/ directory or shall I just add this inside the init.py or something !?

With inside the component I mean define them inside camera/__init__.py

balloob · 2017-06-15T05:45:54Z

homeassistant/components/camera/__init__.py

+                elif service.service == SERVICE_DISARM:
+                    yield from camera.async_disarm()
+            except AttributeError:
+                pass


We should not just let this pass. We should also not try to update a camera that didn't had an attribute.

I can check if the camera has that attribute using hasattr() and then call that if it has that attribute.

viswa-swami · 2017-06-15T14:07:02Z

Thanks @balloob 10000 for your suggestions.

The reasoning behind why I did it as a generic component was because I have foscam cameras inside my home and Arlo for outdoor. For privacy reasons, I keep the foscam cameras turned off when we are home, and turn on/enable motion detection only when we are not in home.

Currently I had to write a separate python script and call it via shell_command for automation. To avoid it, I created a generic camera arm/disarm service so that I could just invoke that service instead of separate script.

I was not sure about nest cameras, but cameras like foscam, amcrest and Arlo of course has these capabilities of enabling and disabling motion/sound detection via APIs.

If you think it will be useful for others too as a service, please suggest and I can work on adding the demo and tests. But if you think it would be an Overkill at the moment, I can add this just for Arlo.

@balloob, can you please recommend which route to go?

viswa-swami · 2017-06-15T22:56:19Z

In parallel, I am also modifying the code based on your review comments. Currently I have removed the AttributeError related code, as well as moved the I/O out of the async functions based on the "Working with Async" documentation in home-assistant.io. I will update the PR once that is ready and tested in my home.

balloob · 2017-06-16T05:12:35Z

For Foscam you're talking about both turning the camera on and turning on the motion detection. I would expect an 'arm' service to only turn on the motion detection. Turning a device on/off is a separate command. In that case I also wonder if arm and disarm are the wrong names. What about set_motion_detection ?

In that case we also would have detecting_motion be a new property that returns a boolean and is part of the device attributes.

Like Pascal said, if you are going to add this to the camera component, you will also have to update the demo camera platform. The methods will have to fake the data on the instance and report if motion reporting is on as if it is backed by a device. See the light demo for an example.

viswa-swami · 2017-06-16T12:44:14Z

Sorry for the confusion. I was explaining my use case as to how I operate the camera in my home.

This arm and disarm only takes care of enabling the motion detection for the given camera entity. Based on your comment, instead of arm and disarm, shall I rename the service also to "enable_motion_detection" and "disable_motion_detection" respectively ?

The "status" that I have added does this purpose of indicating whether the camera is armed or disarmed. It can potentially be used for a sensor also.

I have already moved the macros to camera/init.py as well as added to the demo.py. I will update the PR a bit later. I have also added code to remove the AttributeError and verified it here.

viswa-swami · 2017-06-27T19:38:38Z

@balloob Posted the PR after verifying the functionality with actual code and that was working fine !!! Couldn't seem to figure out why the test_demo is failing though ! Looking at the demo code to see if something got missed !

…ble the motion in __init__.py and missed to add it to as a job.

houndci-bot · 2017-06-27T20:02:01Z

homeassistant/components/camera/__init__.py

+def disable_motion_detection(hass, entity_id=None):
+    """Disable Motion Detection."""
+    data = {ATTR_ENTITY_ID: entity_id} if entity_id else None
+    hass.async_add_job(hass.services.async_call(DOMAIN, SERVICE_DISEN_MOTION, data))


line too long (84 > 79 characters)

houndci-bot · 2017-06-27T20:02:01Z

homeassistant/components/camera/__init__.py

+def enable_motion_detection(hass, entity_id=None):
+    """Enable Motion Detection."""
+    data = {ATTR_ENTITY_ID: entity_id} if entity_id else None
+    hass.async_add_job(hass.services.async_call(DOMAIN, SERVICE_EN_MOTION, data))


line too long (81 > 79 characters)

viswa-swami · 2017-06-27T21:05:57Z

@balloob Figured out the problem, and fixed it. Have submitted the latest code for PR.

Please review and let me know your suggestions if any.

Thanks for your help and patience... :)

tchellomello · 2017-06-30T07:18:18Z

@viswa-swami FYI https://pypi.python.org/pypi/pyarlo/0.0.5 is available 👍

This reverts commit ed20f7e.

* Revert "Version bump to 0.49.0.dev0 (#8266)" This reverts commit 8e4394f. * Revert "Adding done_message to alert (#8116)" This reverts commit 5e56bc7. * Revert "Camera services arm disarm including Netgear Arlo (#7961)" This reverts commit ed20f7e. * Revert "Make Android app shortcut use 'Home Assistant' as name instead of just 'Assistant'. (#8261)" This reverts commit 0bcb783.

@balloob

* Added camera service calls to arm/disarm the cameras. Entity id is optional so that with a single call we can arm all the cameras or specify a particular entity id to arm if applicable and possible in that camera type. * Added camera service calls to arm/disarm the cameras. Entity id is optional so that with a single call we can arm all the cameras or specify a particular entity id to arm if applicable and possible in that camera type. * Added camera service calls to arm/disarm the cameras. Entity id is optional so that with a single call we can arm all the cameras or specify a particular entity id to arm if applicable and possible in that camera type. * Fixed the spaces and indentation related issues that houndci found * Fixed the spaces and indentation related issues that houndci found * Missed the const file which has the macros defined. * Fixed the CI build error * Fixed the CI build error because of unused variable in exception case * Updating the arlo code based on comment from @balloob. Changed the arm and disarm to enable_motion_detection and disable_motion_detection respectively. Similarly fixed the AttributeError handling. Added dummy code to the demo camera also. Moved out the definitions in const.py into the camera __init__ file * Fixed the comments posted by houndci-bot * Fixed the comments posted by houndci-bot * Fixed the comments posted by houndci-bot * Fixed the comments posted by travis-ci integration bot * Fixed the comments posted by travis-ci integration bot * Fixed the comments posted by travis-ci integration bot for demo.py: expected 2 lines, found 1 * Updated code in camera __init__.py to use the get function instead of directly calling the member in the structure. * Updated code in camera __init__.py * Posting the updated code for PR based on @balloob's suggestions/recommendations * Removed the arlo reference from demo code. Copy-paste error * Removed the unused import found by hound bot * Expected 2 lines before function, but found only 1. * Based on @balloob's comments, moved these constants to the camera/arlo.py * Added test_demo.py to test the motion enabled and motion disabled in camera component * Fixing issues found by houndci-bot * Fixing issues found by houndci-bot * Fixing the code as per @balloob's suggestions * Fixing the code as per @balloob's suggestions * Fixing the test_demo failure. Tried to rewrite a base function to enable the motion in __init__.py and missed to add it to as a job. * Fixing the hound bot comment * Update arlo.py * Update arlo.py

balloob · 2017-07-01T05:24:45Z

Cherry-picked for 0.48

viswa-swami · 2017-07-01T15:36:18Z

Thank you @balloob for including this in the upcoming release.

@balloob

* Added camera service calls to arm/disarm the cameras. Entity id is optional so that with a single call we can arm all the cameras or specify a particular entity id to arm if applicable and possible in that camera type. * Added camera service calls to arm/disarm the cameras. Entity id is optional so that with a single call we can arm all the cameras or specify a particular entity id to arm if applicable and possible in that camera type. * Added camera service calls to arm/disarm the cameras. Entity id is optional so that with a single call we can arm all the cameras or specify a particular entity id to arm if applicable and possible in that camera type. * Fixed the spaces and indentation related issues that houndci found * Fixed the spaces and indentation related issues that houndci found * Missed the const file which has the macros defined. * Fixed the CI build error * Fixed the CI build error because of unused variable in exception case * Updating the arlo code based on comment from @balloob. Changed the arm and disarm to enable_motion_detection and disable_motion_detection respectively. Similarly fixed the AttributeError handling. Added dummy code to the demo camera also. Moved out the definitions in const.py into the camera __init__ file * Fixed the comments posted by houndci-bot * Fixed the comments posted by houndci-bot * Fixed the comments posted by houndci-bot * Fixed the comments posted by travis-ci integration bot * Fixed the comments posted by travis-ci integration bot * Fixed the comments posted by travis-ci integration bot for demo.py: expected 2 lines, found 1 * Updated code in camera __init__.py to use the get function instead of directly calling the member in the structure. * Updated code in camera __init__.py * Posting the updated code for PR based on @balloob's suggestions/recommendations * Removed the arlo reference from demo code. Copy-paste error * Removed the unused import found by hound bot * Expected 2 lines before function, but found only 1. * Based on @balloob's comments, moved these constants to the camera/arlo.py * Added test_demo.py to test the motion enabled and motion disabled in camera component * Fixing issues found by houndci-bot * Fixing issues found by houndci-bot * Fixing the code as per @balloob's suggestions * Fixing the code as per @balloob's suggestions * Fixing the test_demo failure. Tried to rewrite a base function to enable the motion in __init__.py and missed to add it to as a job. * Fixing the hound bot comment * Update arlo.py * Update arlo.py

…e-assistant#8271) * Revert "Version bump to 0.49.0.dev0 (home-assistant#8266)" This reverts commit 8e4394f. * Revert "Adding done_message to alert (home-assistant#8116)" This reverts commit 5e56bc7. * Revert "Camera services arm disarm including Netgear Arlo (home-assistant#7961)" This reverts commit ed20f7e. * Revert "Make Android app shortcut use 'Home Assistant' as name instead of just 'Assistant'. (home-assistant#8261)" This reverts commit 0bcb783.

viswa-swami added 3 commits June 8, 2017 12:02

Added camera service calls to arm/disarm the cameras. Entity id is op…

705205b

…tional so that with a single call we can arm all the cameras or specify a particular entity id to arm if applicable and possible in that camera type.

Added camera service calls to arm/disarm the cameras. Entity id is op…

1ef7055

…tional so that with a single call we can arm all the cameras or specify a particular entity id to arm if applicable and possible in that camera type.

Added camera service calls to arm/disarm the cameras. Entity id is op…

8669c7f

…tional so that with a single call we can arm all the cameras or specify a particular entity id to arm if applicable and possible in that camera type.

homeassistant added platform: camera.arlo cla-signed labels Jun 8, 2017

houndci-bot reviewed Jun 8, 2017

View reviewed changes

Fixed the spaces and indentation related issues that houndci found

e6deae2

homeassistant added the cla-signed label Jun 8, 2017

houndci-bot reviewed Jun 8, 2017

View reviewed changes

viswa-swami added 2 commits June 8, 2017 12:20

Fixed the spaces and indentation related issues that houndci found

eb11fd8

Missed the const file which has the macros defined.

70808e5

balloobbot added core platform labels Jun 8, 2017

Fixed the CI build error

c4a91c9

homeassistant added the cla-signed label Jun 8, 2017

Fixed the CI build error because of unused variable in exception case

e2c0113

pvizeli requested changes Jun 9, 2017

View reviewed changes

balloob reviewed Jun 15, 2017

View reviewed changes

Fixing the test_demo failure. Tried to rewrite a base function to ena…

5c95020

…ble the motion in __init__.py and missed to add it to as a job.

homeassistant added the cla-signed label Jun 27, 2017

houndci-bot reviewed Jun 27, 2017

View reviewed changes

Fixing the hound bot comment

33362c9

balloob approved these changes Jun 30, 2017

View reviewed changes

Merge branch 'dev' into arlo_services_arm_disarm

6c97791

homeassistant added the cla-signed label Jun 30, 2017

Update arlo.py

488a71e

balloob approved these changes Jun 30, 2017

View reviewed changes

Update arlo.py

19c3d62

balloob merged commit ed20f7e into home-assistant:dev Jul 1, 2017

viswa-swami mentioned this pull request Jul 1, 2017

0.48 #8269

Closed

balloob added this to the 0.48 milestone Jul 1, 2017

balloob added a commit that referenced this pull request Jul 1, 2017

Revert "Camera services arm disarm including Netgear Arlo (#7961)"

10e63d6

This reverts commit ed20f7e.

balloob mentioned this pull request Jul 1, 2017

0.48 #8270

Merged

balloob mentioned this pull request Jul 13, 2017

0.49 #8468

Merged

balloob added the cherry-picked label Jul 13, 2017

home-assistant locked and limited conversation to collaborators Oct 20, 2017

ghost added integration: arlo and removed platform: camera.arlo labels Mar 21, 2019

Uh oh!

Camera services arm disarm including Netgear Arlo #7961

Camera services arm disarm including Netgear Arlo #7961

Uh oh!

Conversation

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Uh oh!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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