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

Bump amcrest to 1.2.7 for correct RTSP port #22099

Merged
merged 1 commit into from Mar 16, 2019

Conversation

Projects
None yet
5 participants
@pnbruckner
Copy link
Contributor

commented Mar 16, 2019

amcrest 1.2.7 now includes camera's configured RTSP port in generated URL. Necessary to make new stream component work correctly if camera does not use default RTSP port.

Related issue (if applicable): N/A

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

Example entry for configuration.yaml (if applicable):

N/A

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 the code communicates with devices, web services, or third-party tools:

  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
Bump amcrest to 1.2.7 for correct RTSP port
amcrest 1.2.7 now includes camera's configured RTSP port in generated URL. Necessary to make new stream component work correctly if camera does not use default RTSP port.
@pnbruckner

This comment has been minimized.

Copy link
Contributor Author

commented Mar 16, 2019

Should this be a hot fix to avoid breaking cameras configured with non-default RTSP ports?

@dshokouhi

This comment has been minimized.

Copy link
Contributor

commented Mar 16, 2019

good catch @pnbruckner !! FYI @hunterjm

@hunterjm hunterjm self-requested a review Mar 16, 2019

@hunterjm
Copy link
Contributor

left a comment

Can be merged when tests pass. Targeting 0.90 so amcrest users with special configurations won't run into issues with stream component.

@hunterjm hunterjm modified the milestone: 0.90.0 Mar 16, 2019

@hunterjm

This comment has been minimized.

Copy link
Contributor

commented Mar 16, 2019

Looking at other PR activity (#22040 and #21664), I'm not sure what has already made it into beta, so will let @balloob make the call on including in 0.90.

@pnbruckner

This comment has been minimized.

Copy link
Contributor Author

commented Mar 16, 2019

FYI, if you look at the 0.90.0b2 commit history for the amcrest component, #21664 & #21720 that I made, and #21983 that @dshokouhi made, have made it into the beta. However, looking at the commit history on dev you can see that #22040 I made has not made it into the beta. So, yes, if this one goes into the beta, then 22040 should as well, otherwise the new exceptions introduced in amcrest 1.2.6 will not be handled properly.

@Danielhiversen Danielhiversen merged commit 9e4bd88 into home-assistant:dev Mar 16, 2019

4 checks passed

Hound No violations found. Woof!
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on rtsp-port at 92.775%
Details

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

@pnbruckner pnbruckner deleted the pnbruckner:rtsp-port branch Mar 16, 2019

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