feat: add support for fetching the rtsp url without srtp #261
Conversation
WalkthroughThe changes introduce a new property, Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
tests/test_api.py (1)
84-96: LGTM! The URL validation looks good.The changes correctly validate the RTSP URL formats, including the new
rtsps_no_srtp_urlproperty. The double validation through the loop helps ensure URL consistency.Consider adding test cases for edge cases such as:
- Invalid connection host
- Empty RTSP alias
- Special characters in RTSP alias
Example test case:
def test_rtsp_url_edge_cases(): # Test invalid connection host camera.api.connection_host = None with pytest.raises(ValueError): _ = channel.rtsps_no_srtp_url # Test empty RTSP alias channel.rtsp_alias = "" with pytest.raises(ValueError): _ = channel.rtsps_no_srtp_url
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/uiprotect/data/devices.py(2 hunks)tests/test_api.py(1 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/uiprotect/data/devices.py
[warning] 301-301: src/uiprotect/data/devices.py#L301
Added line #L301 was not covered by tests
🔇 Additional comments (2)
src/uiprotect/data/devices.py (2)
276-276: LGTM!
The private attribute follows the established pattern for URL caching in the class.
298-306: Implementation looks good but needs test coverage.
The property implementation follows the established pattern and correctly handles edge cases. However, line 301 is not covered by tests.
Run the following script to verify the URL format matches go2rtc's expectations:
✅ Verification successful
The search results show that the RTSPS URL format is already being tested in tests/test_api.py, which verifies both SRTP-enabled and non-SRTP URLs. The format matches between the implementation and tests:
- With SRTP:
rtsps://{host}:{port}/{alias}?enableSrtp - Without SRTP:
rtsps://{host}:{port}/{alias}
Let me verify if there are any other RTSPS URL formats in the codebase:
Let me check if there are any configuration files or examples that might show the expected URL format:
RTSPS URL format is correctly implemented and verified by tests
The implementation matches the URL format that's being verified in the test suite:
- SRTP enabled:
rtsps://{host}:{port}/{alias}?enableSrtp - SRTP disabled:
rtsps://{host}:{port}/{alias}
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the RTSPS URL format matches go2rtc's expectations
# Test: Search for RTSPS URL handling in go2rtc documentation or code
rg -A 5 'rtsps://'
Length of output: 1517
Script:
#!/bin/bash
# Search for any other RTSPS URL patterns or documentation
rg -i 'rtsps.*://' --type md
Length of output: 30
Script:
#!/bin/bash
# Search for RTSPS configuration or examples in yaml/json files
rg -i 'rtsps' --type yaml --type json -A 5
Length of output: 42
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 301-301: src/uiprotect/data/devices.py#L301
Added line #L301 was not covered by tests
go2rtcdoesn't supportsrtpyet and it doesn't need it anyways since we always use tcp withrtspshttps://github.com/AlexxIT/go2rtc/?tab=readme-ov-file#source-rtsp
Summary by CodeRabbit
New Features
rtsps_no_srtp_url) for enhanced streaming capabilities.Bug Fixes
These changes enhance the functionality and reliability of camera management features for users.