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

Update ONVIF component to SUPPORT_STREAM #22569

Merged
merged 5 commits into from Mar 31, 2019

Conversation

@drjared88
Copy link
Contributor

commented Mar 30, 2019

Description:

Add SUPPORT_STREAM to ONVIF component to allow streaming to the Google Assistant.

Related issue (if applicable): fixes #<0.91.0-b3

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

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

If the code communicates with devices, web services, or third-party tools:

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New or updated 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:

  • Tests have been added to verify that the new code works.

drjared88 added some commits Mar 30, 2019

Show resolved Hide resolved homeassistant/components/onvif/camera.py Outdated
Show resolved Hide resolved homeassistant/components/onvif/camera.py Outdated
Show resolved Hide resolved homeassistant/components/onvif/camera.py Outdated
@dgomes

This comment has been minimized.

Copy link
Contributor

commented Mar 30, 2019

You need to fix lint errors else all good

Update camera.py
Remove extra spaces.
@drjared88

This comment has been minimized.

Copy link
Contributor Author

commented Mar 31, 2019

You need to fix lint errors else all good

I think I did this now but I’m new to this PR thing.

@hunterjm
Copy link
Contributor

left a comment

self._input starts as None and does not get set until obtain_input_uri is called in either async_camera_image or handle_async_mjpeg_stream is called at least once. This will mean the preload stream camera configuration will not work for this platform.

The setup_platform should be converted to async, and an async def initialize method or similar should be created to obtain the input URL as this component loads in order to work properly.

Show resolved Hide resolved homeassistant/components/onvif/camera.py Outdated

@ghost ghost assigned hunterjm Mar 31, 2019

@hunterjm hunterjm added this to the 0.91.0 milestone Mar 31, 2019

@balloob balloob merged commit e085383 into home-assistant:dev Mar 31, 2019

14 checks passed

Hound No violations found. Woof!
build Workflow: build
Details
ci/circleci: pre-install-all-requirements Your tests passed on CircleCI!
Details
ci/circleci: pre-test 3.5.5 Your tests passed on CircleCI!
Details
ci/circleci: pre-test 3.6 Your tests passed on CircleCI!
Details
ci/circleci: pre-test 3.7 Your tests passed on CircleCI!
Details
ci/circleci: pylint Your tests passed on CircleCI!
Details
ci/circleci: static-check Your tests passed on CircleCI!
Details
ci/circleci: test 3.5.5 Your tests passed on CircleCI!
Details
ci/circleci: test 3.6 Your tests passed on CircleCI!
Details
ci/circleci: test 3.7 Your tests passed on CircleCI!
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on patch-3 at 92.939%
Details

@ghost ghost removed the in progress label Mar 31, 2019

balloob added a commit that referenced this pull request Apr 1, 2019

Update ONVIF component to SUPPORT_STREAM (#22569)
* Update Onvif component to SUPPORT_STREAM

* Update camera.py

* Update camera.py

* Update camera.py

Remove extra spaces.

* lookup URL when camera is added to hass and add extra guards

@balloob balloob referenced this pull request Apr 3, 2019

Merged

0.91.0 #22688

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.