IP Webcam#5614
Conversation
balloobbot
added
component
platform
and removed
integration: android_ip_webcam
new-platform
platform: binary_sensor.android_ip_webcam
labels
There was a problem hiding this comment.
Please don't copy paste this. Instead move the other implementation to the camera component.
There was a problem hiding this comment.
This was only necessary for the camera_image function. Removing this because of removal of camera_image.
There was a problem hiding this comment.
I do not understand why you implement this method and use extract_image_from_mjpeg if you have a method above that does exactly the same but async
There was a problem hiding this comment.
FYI This is a carryover from camera/mjpeg.py
There was a problem hiding this comment.
Oh it's necessary in mjpeg.py in case users have Digest authentication or don't have a still image URL set.
There was a problem hiding this comment.
Please don't use requests. Use aiohttp instead.
There was a problem hiding this comment.
When you extract it into a third party lib, have it take an aiohttp session in the constructor. That way you don't need to do any closing work because you can use the one managed by hass.
There was a problem hiding this comment.
This is the kind of thing that is perfect to be extracted into a third party lib.
There was a problem hiding this comment.
Change the constructor here to take in a websession and the config as parameters. You don't need the hass object.
There was a problem hiding this comment.
'requests.auth.HTTPBasicAuth' imported but unused
There was a problem hiding this comment.
'contextlib.closing' imported but unused
|
Ok to merge when linting fixed 👍 |
|
I would like to mention that the sensor platform is not available on the free app. You would need the (currently $3.99) paid app to enable sensors. I have not reviewed any of the code, but in order to prevent confusion, it may be a good idea to only expose those options available in the free version by default. Then, if a person uses the paid version, have an option in the yaml to specify the full sensors are available. |
|
@DavidSpivey I don't have the paid app but I have access to all the sensors... |
|
@balloob Did you not want me to finish converting this to async then? |
|
@robbiet480 I'm terribly sorry about that. Totally missed the option in the free version. You're right. |
|
Good point @robbiet480, yes to async 👍 |
There was a problem hiding this comment.
instead pylint: disable=unused-argument use for device in devices.values()
There was a problem hiding this comment.
This doesn't work in a constructor when you're async'ing. So remove this.
robbiet480
force-pushed
the
android-ip-camera
branch
from
44ce6e4 to
b90b33d
Compare
|
You can only use |
7 participants
Description: This adds support for IP Webcam, a recently discovered Android app with many of the same features you would find in a very high end IP camera.
Enabling this component will expose:
The default configuration looks like this when IP Webcam is running from my Nexus 5:
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:
If the code communicates with devices, web services, or third-party tools:
toxrun successfully. Your PR cannot be merged unless tests passREQUIREMENTSvariable (example).requirements_all.txtby runningscript/gen_requirements_all.py..coveragerc.If the code does not interact with devices:
toxrun successfully. Your PR cannot be merged unless tests pass