-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
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
Conversation
add_devices_callback([WelcomeCamera(data, camera_name, home)]) | ||
|
||
|
||
class WelcomeCamera(Camera): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- D200: One-line docstring should fit on one line with quotes (found 3)
There was a problem hiding this comment.
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
298664b
to
3f50226
Compare
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. |
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) |
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. |
There was a problem hiding this comment.
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 😉
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. |
389bfff
to
6f733af
Compare
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): |
There was a problem hiding this comment.
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
Looks good! 🐬 |
@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 ? |
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. |
Description:
This change is introducing two things:
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):Checklist:
If user exposed functionality or configuration variables are added/changed:
If code communicates with devices:
tox
run successfully. Your PR cannot be merged unless tests passREQUIREMENTS
variable (example).requirements_all.txt
by runningscript/gen_requirements_all.py
..coveragerc
.If the code does not interact with devices:
tox
run successfully. Your PR cannot be merged unless tests pass