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

Introducing support for Netatmo Welcome Camera #2233

Merged
merged 11 commits into from Jun 10, 2016

Conversation

jabesq
Copy link
Contributor

@jabesq jabesq commented Jun 4, 2016

Description:
This change is introducing two things:

  • Global component for Netatmo devices: it allows to share credentials between Netatmo Weather Station, Welcome camera and also for Thermosat (even if not supported yet)
  • Support for Netatmo Welcome Camera: Currently it only provides the live feed of the camera. The final goal will be to also to use this camera as a binary sensor and a device tracker

Related issue (if applicable): fixes #

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

Example entry for configuration.yaml (if applicable):

#Netatmo Component
netatmo:
    api_key: API_KEY
    secret_key: SECRET_KEY
    username: username
    password: password

#Cameras
camera:
    platform: netatmo
    home: home_name (Optional)
    cameras: (Optional)
        - camera_name1
        - camera_name2

sensor:
  platform: netatmo
  modules:
    module_name1:
      - temperature
      - humidity
      - noise
      - pressure
      - co2
      - rain
      - sum_rain_1
      - sum_rain_24
    module_name2:
      - temperature
    rainmeter_name3:
      - rain
      - sum_rain_1
      - sum_rain_24

Checklist:

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

If code communicates with devices:

  • 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.

add_devices_callback([WelcomeCamera(data, camera_name, home)])


class WelcomeCamera(Camera):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

farcy v1.1

  • D200: One-line docstring should fit on one line with quotes (found 3)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get it what you want here @balloobbot

@jabesq jabesq force-pushed the netatmo branch 5 times, most recently from 298664b to 3f50226 Compare June 4, 2016 12:38
@rmkraus
Copy link
Contributor

rmkraus commented Jun 4, 2016

Is it possible to have the Netatmo component search for the cameras and sensors then create what is found? Take a look at the Hue component, it does this.

@jabesq
Copy link
Contributor Author

jabesq commented Jun 4, 2016

It already works like this for the camera (but I detail on the documentation how to filter for a specific camera if the user has several and want to display only one)
For the sensor, I just kept the behavior of the current netatmo sensor. The only change is that credentials are now stored inside a new global component Netatmo

@rmkraus
Copy link
Contributor

rmkraus commented Jun 4, 2016

Great! 👍

Maybe a later PR could cover discovery of Netatmo sensors, but that would definitely be too much for one PR.

@@ -0,0 +1,98 @@
"""
Support for the NetAtmo Weather Service.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it needs a quick update 😉

@rmkraus
Copy link
Contributor

rmkraus commented Jun 4, 2016

Looks really good. I had a few nitpicky docstring comments (sorry) and a few more questions.

In particular, how will the camera and sensor platforms react when improper credentials are provided? They don't seem to individually check to ensure that these credentials are good. Especially since this would be a breaking change, the platforms should probably put good information in the logs.

Now that I'm typing, what if the sensors platform still accepted the credentials, as is it did before, but threw a deprecation warning to the log? That all helps ease the transition of breaking changes.

As Netatmo is providing several type of device (sensor, camera), a new Netatmo
component needs to be created in order to centralize the Netatmo login data.
Currently this change only impacts the Netatmo Weather station
This new API will provide access to the Welcome Camera
This change introduces support for Netatmo Welcome camera. Currently, it will
add all detected camera to Home Assistant, camera filtering (similar to the one
used for weather station modules) will be added later
self.camera_names.append(camera['name'])
return self.camera_names

def update(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should throttle this call, otherwise it will be called once for each camera that is failing inside camera_image.

As the update function updates the data for all cameras, we should prevent this
function to be called several time during an interval
@balloob balloob merged commit 213a738 into home-assistant:dev Jun 10, 2016
@balloob
Copy link
Member

balloob commented Jun 10, 2016

Looks good! 🐬

@jabesq jabesq deleted the netatmo branch June 10, 2016 08:36
@balloob
Copy link
Member

balloob commented Sep 10, 2016

@jabesq it looks like you deleted the 0.5 release of the Python Netatmo lib that you had Home Assistant point at for this PR, breaking Home Assistant. What's going on ?

@jabesq
Copy link
Contributor Author

jabesq commented Sep 10, 2016

Ooops, I did a force rebase on philipelt's netatmo API (which has credentials to the pypi archive) as I plan to retake my work for the Welcome camera but my rebase might have delete my tag, I'll recreate it so home-assistant can find it again. But my goal was that home-assistant to use directly the pypi archive.

@home-assistant home-assistant locked and limited conversation to collaborators Mar 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants