Netatmo binary sensor #3455

jabesq · 2016-09-19T20:51:16Z

Description:
This PR create a new platform which provides binary sensor based on event seen by the Netatmo camera

Related issue (if applicable): fixes #

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

Example entry for configuration.yaml (if applicable):

binary_sensor:
   platform: netatmo
   home: home_name
   cameras:
     - camera_name1
     - camera_name2
   monitored_conditions:
     - Someone known
     - Someone unknown 
     - Motion

Checklist:

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

Documentation added/updated in home-assistant.io

If code communicates with devices, web services, or a:

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.

pvizeli · 2016-09-19T21:12:14Z

homeassistant/components/binary_sensor/netatmo.py

+        return self._name
+
+    @property
+    def unique_id(self):


Remove that. The HA core do that same and is more robust.

Nah, if a platform implements it it will be better. They know exactly which is unique. However, a name is not very unique? Do you have access to a serial?

Yes, the netatmo api provides an unique ID for each camera/device

Change Done

balloob · 2016-09-20T06:12:14Z

homeassistant/components/binary_sensor/netatmo.py

+        return self._name
+
+    @property
+    def unique_id(self):


Nah, if a platform implements it it will be better. They know exactly which is unique. However, a name is not very unique? Do you have access to a serial?

balloob · 2016-09-20T06:13:46Z

homeassistant/components/netatmo.py

@@ -50,7 +49,4 @@ def setup(hass, config):
        _LOGGER.error("Unable to connect to NatAtmo API")
        return False

-    for component in 'camera', 'sensor':


Why did you remove discovery ?

It was creating issue as some person can have the weather station but does not have the camera, or they could have multiple camera and displaying only one.

My original goal was to use the discovery if no camera was settup but if one was setup, then do not use the discovery, but currently it did not work well as two feed of the same camera could be displayed

Other platforms fix this by checking the data to see if there is a camera, sensor etc.

I want to keep the discovery in because it is helps users get started. If the camera component will only setup 1 camera, then that is also that has to be fixed.

No the camera component can setup as many camera as you want, but the thing is that might don't want to have all the information from the camera, for instance, the binary sensor create 3 sensor by camera by default, bu user might don't want to have the 'Someone known' sensor if they have a more precise device tracker (moreover with my next update which should implement presence detection based on the Netatmo camera).

I would me more than happy to use the discovery, but discovery use the default config (all camera, all sensors) but if I setup a specific config it creates duplicate in the interface (the feed from discovery and the one from specific setup)

Make it configurable but default it to on as automatic setup of platforms helps new users get started.

Ok do you have example that I can copy

balloob · 2016-09-20T06:17:09Z

homeassistant/components/binary_sensor/netatmo.py

+            return None
+
+
+class WelcomeData(object):


This is a direct copy of the one in camera ? Why not make it a generic object, instantiated once in the main component?

You 're right and as this code will also be use for the coming presence detection platform, it make more sense to put inside components/netatmo.py

rofrantz · 2016-09-20T17:01:10Z

You can use two aproches in order to use discovery:

automatic capability: check whether the discovered device has any capability reported
Or
manual capability detection: test them youself in the code to see if u have a camera, wheather or anything other capability

The last one could be done in the netdisco ant it would emit in your case only two entities discovered and then in HA two different components would listen for them and render them in the frontend

jabesq · 2016-09-20T17:21:52Z

@rofrantz I used the second approach but it was not perfect as the Netatmo API will return all the devices that you own, but if you have one camera in one home and an another one in another home you might don't want to display both in you home-assistant interface.

My idea was to use the discovery if no specific camera has been setup in the configuration.yaml (then it will use the default camera), but if a camera is setup then discovery is ignored.

rofrantz · 2016-09-20T17:28:12Z

Still sounds like a custom setup meaning that u have 2 cameras in 2 different locations in the same HA instance; why dont u hide one of two entities if thats the issue or add a default camera flag.
Although as I read this again indeed its a custom setup with a delicate solution which would only complica the HA logic, so yeah we better drop it :)

jabesq · 2016-09-26T15:29:45Z

This PR also fixes an issue with voluptuous that will be introduce by #3287 in release 0.29 #3486

jabesq · 2016-09-26T15:31:19Z

homeassistant/components/sensor/netatmo.py

@@ -50,7 +49,7 @@
 }

 MODULE_SCHEMA = vol.Schema({
-    vol.Required(CONF_MODULE_NAME, default=[]):
+    vol.Required(cv.string, default=[]):


Fix for #3287

Because this fix was bundled with a PR for a feature, I have missed it.

OK, I'm preparing another PR for this fix solely

balloob · 2016-09-30T15:45:01Z

homeassistant/components/camera/netatmo.py

-        if config[CONF_CAMERAS] != []:
-            if camera_name not in config[CONF_CAMERAS]:
-                continue
+        if CONF_CAMERAS in config:


Can you simplify this by moving it outside of the loop (it won't change during the loop) and also convert to be 1 if statement.

balloob · 2016-09-30T15:46:07Z

homeassistant/components/netatmo.py

@@ -50,7 +49,4 @@ def setup(hass, config):
        _LOGGER.error("Unable to connect to NatAtmo API")
        return False

-    for component in 'camera', 'sensor':


Make it configurable but default it to on as automatic setup of platforms helps new users get started.

balloob · 2016-09-30T15:46:36Z

homeassistant/components/binary_sensor/netatmo.py

+    sensors = config.get(CONF_MONITORED_CONDITIONS, SENSOR_TYPES)
+
+    for camera_name in data.get_camera_names():
+        if config[CONF_CAMERAS] != []:


Can you simplify this by moving it outside of the loop (it won't change during the loop) and also convert to be 1 if statement.

This is difficult to simplify as this solution allows me to have only one for loop instead of two

Signed-off-by: Hugo D. (jabesq) <jabesq@gmail.com>

And global config to disable discovery for netatmo devices Signed-off-by: Hugo D. (jabesq) <jabesq@gmail.com>

balloob · 2016-10-09T15:45:10Z

🐬

jabesq force-pushed the Netatmo-BinSensor branch from 5a2e300 to a378793 Compare September 19, 2016 20:55

pvizeli reviewed Sep 19, 2016

View reviewed changes

balloob requested changes Sep 20, 2016

View reviewed changes

jabesq force-pushed the Netatmo-BinSensor branch from a378793 to 220ceab Compare September 20, 2016 16:47

jabesq commented Sep 26, 2016

View reviewed changes

jabesq force-pushed the Netatmo-BinSensor branch 3 times, most recently from 715d7d7 to cd354a7 Compare September 30, 2016 13:26

balloob reviewed Sep 30, 2016

View reviewed changes

balloob requested changes Sep 30, 2016

View reviewed changes

jabesq added 5 commits October 2, 2016 15:04

Bug fixes and optimization for Netatmo devices

f60916e

Signed-off-by: Hugo D. (jabesq) <jabesq@gmail.com>

Basic support for Netatmo welcome binary sensors

10a0a28

Signed-off-by: Hugo D. (jabesq) <jabesq@gmail.com>

pylint fixes

b76022b

Bug Fixing and optimization for Netatmo devices

fcda9f5

Signed-off-by: Hugo D. (jabesq) <jabesq@gmail.com>

Add unique_id for cameras to avoid duplicate

8c602f3

And global config to disable discovery for netatmo devices Signed-off-by: Hugo D. (jabesq) <jabesq@gmail.com>

jabesq force-pushed the Netatmo-BinSensor branch from cd354a7 to 8c602f3 Compare October 2, 2016 13:11

balloob approved these changes Oct 9, 2016

View reviewed changes

balloob merged commit 1d0df63 into home-assistant:dev Oct 9, 2016

jabesq deleted the Netatmo-BinSensor branch October 18, 2016 16:40

home-assistant locked and limited conversation to collaborators Mar 17, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Netatmo binary sensor #3455

Netatmo binary sensor #3455

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Netatmo binary sensor #3455

Netatmo binary sensor #3455

Uh oh!

Conversation

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!