Video Manager: Check default route ip for RTSP availability#3893
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates the video diagnostic helper to consider RTSP streams bound to the default route (0.0.0.0) as accessible and refines the user-facing error message about unavailable video streams. Flow diagram for RTSP accessibility check including default routeflowchart TD
A[Compute all_streams from video.available_streams] --> B[Iterate over each stream endpoint]
B --> C[Lowercase endpoint to route]
C --> D{route startsWith rtsp://vehicle_ip_address?}
D -- Yes --> H[Mark endpoint as accessible]
D -- No --> E{route startsWith rtsp://0.0.0.0?}
E -- Yes --> H[Mark endpoint as accessible]
E -- No --> F[Ignore endpoint for RTSP accessibility]
H --> G[Collect accessible RTSP endpoints]
F --> B
G --> I{Any accessible RTSP endpoints?}
I -- Yes --> J[has_accessible_rtsp_streams returns true]
I -- No --> K[has_accessible_rtsp_streams returns false]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
has_accessible_rtsp_streamslogic would be clearer and more efficient if you used.some(...)instead offilter(...).isEmpty()for checking existence, matching the intent directly. - Consider extracting the RTSP route matching (checking for
vehicle_ip_addressor0.0.0.0) into a small helper function to avoid inlining this logic and make future adjustments easier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `has_accessible_rtsp_streams` logic would be clearer and more efficient if you used `.some(...)` instead of `filter(...).isEmpty()` for checking existence, matching the intent directly.
- Consider extracting the RTSP route matching (checking for `vehicle_ip_address` or `0.0.0.0`) into a small helper function to avoid inlining this logic and make future adjustments easier.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
b2a8764 to
bec4f23
Compare
|
You have used all of your free Bugbot PR reviews. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
|
Since you are editing this, we could add support to the other rtsp kinds as well, which would also show as not reachable today. |
|
@joaoantoniocardoso such as…? I’m not familiar with the possible options - just had an easy target of working IP with a false positive error message 😅 |
This can be done in another PR and an issue can be raised. |
@dgudiel reported that the existing error display check was giving a false positive for a default, working RadCam setup:
This PR includes the default route (
0.0.0.0) in that check, and clarifies the error message for when it does occur.Summary by Sourcery
Improve video diagnostic handling of RTSP stream availability for default-routed cameras.
Bug Fixes:
Enhancements:
Note
Low Risk
Low risk UI/diagnostic logic tweak that only affects when the Video Manager shows a warning; no auth, persistence, or backend behavior changes.
Overview
Updates
VideoDiagnosticHelper.vueto treat RTSP endpoints using the default route (rtsp://0.0.0.0) as accessible in addition to the vehicle IP, reducing false “no stream accessible” warnings.Rewords the warning text to more clearly indicate that streams may simply not be configured to reach the client IP.
Reviewed by Cursor Bugbot for commit b2a8764. Bugbot is set up for automated code reviews on this repo. Configure here.