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
Added ONVIF camera component #7979
Conversation
self._name, self._username, self._password) | ||
|
||
|
||
@asyncio.coroutine |
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.
too many blank lines (2)
_LOGGER.debug("WSDL for %s: %s", | ||
self._name, wsdl) | ||
_LOGGER.debug("Login details for %s: %s %s", | ||
self._name, self._username, self._password) |
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.
continuation line under-indented for visual indent
_LOGGER.debug("Using the device URL for %s: %s", | ||
self._name, device_url) | ||
_LOGGER.debug("WSDL for %s: %s", | ||
self._name, wsdl) |
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.
continuation line under-indented for visual indent
_LOGGER.debug("Using the base URL for %s: %s", | ||
self._name, self._base_url) | ||
_LOGGER.debug("Using the device URL for %s: %s", | ||
self._name, device_url) |
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.
continuation line under-indented for visual indent
_LOGGER.debug("Using the following URL for %s: %s", | ||
self._name, self._input) | ||
_LOGGER.debug("Using the base URL for %s: %s", | ||
self._name, self._base_url) |
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.
continuation line under-indented for visual indent
media_profile = media.GetProfiles()[0] | ||
self._input = media.GetStreamUri().Uri | ||
_LOGGER.debug("Using the following URL for %s: %s", | ||
self._name, self._input) |
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.
continuation line under-indented for visual indent
self._username = config.get(CONF_USERNAME) | ||
self._password = config.get(CONF_PASSWORD) | ||
media = ONVIFService(device_url,self._username, self._password, wsdl + 'media.wsdl') | ||
media_profile = media.GetProfiles()[0] |
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.
local variable 'media_profile' is assigned to but never used
device_url = self._base_url + 'onvif/device_service' | ||
self._username = config.get(CONF_USERNAME) | ||
self._password = config.get(CONF_PASSWORD) | ||
media = ONVIFService(device_url,self._username, self._password, wsdl + 'media.wsdl') |
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.
missing whitespace after ','
line too long (92 > 79 characters)
|
||
_LOGGER = logging.getLogger(__name__) | ||
|
||
REQUIREMENTS = ['onvif-py3==0.1.3','suds-py3==1.3.3.0','http://github.com/tgaugry/suds-passworddigest-py3/archive/86fc50e39b4d2b8997481967d6a7fe1c57118999.zip#suds-passworddigest-py3==0.1.2a'] |
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.
missing whitespace after ','
line too long (192 > 79 characters)
|
||
import voluptuous as vol | ||
|
||
from homeassistant.const import (CONF_NAME, CONF_USERNAME, CONF_PASSWORD, CONF_PORT) |
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.
line too long (84 > 79 characters)
_LOGGER.debug("WSDL for %s: %s", | ||
self._name, wsdl) | ||
_LOGGER.debug("Login details for %s: %s %s", | ||
self._name, self._username, self._password) |
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.
continuation line over-indented for visual indent
_LOGGER.debug("Using the device URL for %s: %s", | ||
self._name, device_url) | ||
_LOGGER.debug("WSDL for %s: %s", | ||
self._name, wsdl) |
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.
continuation line over-indented for visual indent
_LOGGER.debug("Using the base URL for %s: %s", | ||
self._name, self._base_url) | ||
_LOGGER.debug("Using the device URL for %s: %s", | ||
self._name, device_url) |
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.
continuation line over-indented for visual indent
_LOGGER.debug("Using the following URL for %s: %s", | ||
self._name, self._input) | ||
_LOGGER.debug("Using the base URL for %s: %s", | ||
self._name, self._base_url) |
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.
continuation line over-indented for visual indent
self._password, wsdl + 'media.wsdl') | ||
self._input = media.GetStreamUri().Uri | ||
_LOGGER.debug("Using the following URL for %s: %s", | ||
self._name, self._input) |
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.
continuation line over-indented for visual indent
REQUIREMENTS = ['onvif-py3==0.1.3', | ||
'suds-py3==1.3.3.0', | ||
'http://github.com/tgaugry/suds-passworddigest-py3' | ||
'/archive/86fc50e39b4d2b8997481967d6a7fe1c57118999.zip' |
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.
continuation line under-indented for visual indent
|
||
REQUIREMENTS = ['onvif-py3==0.1.3', | ||
'suds-py3==1.3.3.0', | ||
'http://github.com/tgaugry/suds-passworddigest-py3' |
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.
continuation line under-indented for visual indent
_LOGGER = logging.getLogger(__name__) | ||
|
||
REQUIREMENTS = ['onvif-py3==0.1.3', | ||
'suds-py3==1.3.3.0', |
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.
continuation line under-indented for visual indent
trailing whitespace
|
||
_LOGGER = logging.getLogger(__name__) | ||
|
||
REQUIREMENTS = ['onvif-py3==0.1.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.
trailing whitespace
import voluptuous as vol | ||
|
||
from homeassistant.const import (CONF_NAME, CONF_USERNAME, | ||
CONF_PASSWORD, CONF_PORT) |
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.
continuation line under-indented for visual indent
self._username = config.get(CONF_USERNAME) | ||
self._password = config.get(CONF_PASSWORD) | ||
media = ONVIFService(device_url, self._username, | ||
self._password, wsdl + 'media.wsdl') |
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.
continuation line under-indented for visual indent
device_url = self._base_url + 'onvif/device_service' | ||
self._username = config.get(CONF_USERNAME) | ||
self._password = config.get(CONF_PASSWORD) | ||
media = ONVIFService(device_url, self._username, |
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.
trailing whitespace
You need to rebase your branch and clean-up the requirements. |
rebased and rerun script/gen_requirements_all.py |
rebased again and generated requirements. hopefully it's good to go now. |
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.
Looks okay. A small comments
_LOGGER.debug("WSDL for %s: %s", | ||
self._name, wsdl) | ||
_LOGGER.debug("Login details for %s: %s %s", | ||
self._name, self._username, self._password) |
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.
Please cleanup the debugs, it is ok for test but to mutch for the end product.
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've removed the debug logs.
self._password, wsdl + 'media.wsdl') | ||
self._input = media.GetStreamUri().Uri | ||
_LOGGER.info("ONVIF Camera Using the following URL for %s: %s", | ||
self._name, self._input) |
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.
continuation line under-indented for visual indent
Removed Debug logging |
Requested changes done |
Most components are using "host" not "input" in the config yml. Is this a relic from it being a ffmpeg input config before? Would host be better? |
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.
Need some small cleanups
DEFAULT_PASSWORD = '888888' | ||
|
||
PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({ | ||
vol.Required(CONF_INPUT): cv.string, |
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.
Remove this
|
||
PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({ | ||
vol.Required(CONF_INPUT): cv.string, | ||
vol.Optional(CONF_EXTRA_ARGUMENTS): cv.string, |
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.
Remove this
|
||
self._manager = hass.data[DATA_FFMPEG] | ||
self._name = config.get(CONF_NAME) | ||
self._input = config.get(CONF_INPUT) |
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.
Remove
self._manager = hass.data[DATA_FFMPEG] | ||
self._name = config.get(CONF_NAME) | ||
self._input = config.get(CONF_INPUT) | ||
self._extra_arguments = config.get(CONF_EXTRA_ARGUMENTS) |
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.
Remove
wsdl = hass.config.config_dir + '/deps/onvif/wsdl/' | ||
self._base_url = 'http://{}:{}/'.format(self._input, port) | ||
device_url = self._base_url + 'onvif/device_service' | ||
self._username = config.get(CONF_USERNAME) |
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.
Is not need to store
self._base_url = 'http://{}:{}/'.format(self._input, port) | ||
device_url = self._base_url + 'onvif/device_service' | ||
self._username = config.get(CONF_USERNAME) | ||
self._password = config.get(CONF_PASSWORD) |
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.
Is not need to store
media = ONVIFService(device_url, self._username, | ||
self._password, wsdl + 'media.wsdl') | ||
self._input = media.GetStreamUri().Uri | ||
_LOGGER.info("ONVIF Camera Using the following URL for %s: %s", |
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.
Set this to debug level
"""Generate an HTTP MJPEG stream from the camera.""" | ||
from haffmpeg import CameraMjpeg | ||
|
||
stream = CameraMjpeg(self.hass.data[DATA_FFMPEG].binary, loop=self.hass.loop) |
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.
line too long (85 > 79 characters)
def async_camera_image(self): | ||
"""Return a still image response from the camera.""" | ||
from haffmpeg import ImageFrame, IMAGE_JPEG | ||
ffmpeg = ImageFrame(self.hass.data[DATA_FFMPEG].binary, loop=self.hass.loop) |
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.
line too long (84 > 79 characters)
|
||
self._name = config.get(CONF_NAME) | ||
self._ffmpeg_arguments = config.get(CONF_FFMPEG_ARGUMENTS) | ||
media = ONVIFService('http://{}:{}/'.format(config.get(CONF_HOST), |
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.
trailing whitespace
super().__init__() | ||
|
||
self._name = config.get(CONF_NAME) | ||
self._ffmpeg_arguments = config.get(CONF_FFMPEG_ARGUMENTS) |
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.
undefined name 'CONF_FFMPEG_ARGUMENTS'
|
||
PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({ | ||
vol.Required(CONF_HOST): cv.string, | ||
vol.Optional(CONF_FFMPEG_ARGUMENTS, default=DEFAULT_FFMPEG_ARGUMENTS): cv.string, |
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.
undefined name 'CONF_FFMPEG_ARGUMENTS'
line too long (85 > 79 characters)
DEFAULT_PORT = 5000 | ||
DEFAULT_USERNAME = 'admin' | ||
DEFAULT_PASSWORD = '888888' | ||
DEFAULT_FFMPEG_ARGUMENTS= '-q:v 2' |
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.
missing whitespace around operator
|
||
self._name = config.get(CONF_NAME) | ||
self._ffmpeg_arguments = '-q:v 2' | ||
media = ONVIFService('http://{}:{}/'.format(config.get(CONF_HOST), |
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.
Use the Format pattern for all params, don't mix +
and format
I know I'm a year late to the party, but we shouldn't have added a new non-pip dependency. I'm going to create homeassistant-specific package for this :( |
Description:
Added support for ONVIF IP camera's. This discovers the stream url via ONVIF.
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#2794
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:
tox
run successfully. Your PR cannot be merged unless tests passREQUIREMENTS
variable ([example][ex-requir]).requirements_all.txt
by runningscript/gen_requirements_all.py
..coveragerc
.