Skip to content

fix(Video): show receiver settings for discovered streams - #14993

Open
bosism wants to merge 2 commits into
mavlink:masterfrom
bosism:fix/show-discovered-camera-receiver-settings
Open

fix(Video): show receiver settings for discovered streams#14993
bosism wants to merge 2 commits into
mavlink:masterfrom
bosism:fix/show-discovered-camera-receiver-settings

Conversation

@bosism

@bosism bosism commented Aug 30, 2026

Copy link
Copy Markdown

Bug Description

When MAVLink camera discovery supplies a stream endpoint, the Video settings page hides local receiver-policy controls along with the source and connection controls. This prevents users from configuring receiver behavior for automatically discovered streams even though those settings are applied by VideoManager.

No linked issue.

Root Cause

The affected settings shared an !autoStreamConfig visibility condition with controls that discovery actually owns. The condition hid Stop video when disarmed, Low Latency Mode, RTP jitter latency, and RTSP auto-reconnect whenever a stream was discovered automatically.

Solution

Remove !autoStreamConfig only from the four local receiver-policy controls. Video source selection remains locked, connection fields remain hidden, and the Aspect Ratio override remains hidden for discovered streams.

A QML UI regression test switches the Video settings page into the discovered-stream state and verifies both sides of the behavior: the four receiver controls stay visible, while source/connection/aspect-ratio restrictions remain in place.

Testing

  • Tested locally
  • Added regression test
  • Tested with simulator (SITL)
  • Tested with hardware

Tests and checks run:

  • Linux Debug build with tests and GStreamer enabled
  • TopLevelViewsTest: 9 passed, including _testDiscoveredCameraReceiverSettingsVisible
  • Settings QML generator tests: 118 passed
  • Pre-commit checks for all three changed files

Platforms Tested

  • Linux
  • Windows
  • macOS
  • Android
  • iOS

Flight Stacks Tested

  • PX4
  • ArduPilot
  • N/A

Checklist

  • I have read the Contribution Guidelines
  • My code follows the project's coding standards
  • I have added a test that reproduces the bug
  • New and existing relevant tests pass locally

By submitting this pull request, I confirm that my contribution is made under the terms of the project's dual license (Apache 2.0 and GPL v3).

Keep local receiver-policy controls visible when MAVLink provides the stream endpoint, while continuing to lock source selection and connection fields and to hide the aspect-ratio override. Add UI regression coverage for the discovered-stream state.
@codecov

codecov Bot commented Aug 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 34.57%. Comparing base (f29efd3) to head (ac0a7f7).
⚠️ Report is 350 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14993      +/-   ##
==========================================
+ Coverage   25.47%   34.57%   +9.10%     
==========================================
  Files         769      945     +176     
  Lines       65912    85586   +19674     
  Branches    30495    39446    +8951     
==========================================
+ Hits        16788    29593   +12805     
+ Misses      37285    36330     -955     
- Partials    11839    19663    +7824     
Flag Coverage Δ
unittests 34.57% <ø> (+9.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 660 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 91f8223...ac0a7f7. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

⚠️ Build results unavailable — artifact download from one or more platform workflows failed
(likely artifact retention expiry or transient API error). The combined report cannot be generated for this run.

See the Build Results workflow run for details.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@DonLakeFlyer DonLakeFlyer added this to the Release V5.1 milestone Aug 30, 2026
Comment thread src/AppSettings/pages/Video.SettingsUI.json Outdated
Comment thread src/AppSettings/pages/Video.SettingsUI.json Outdated
Comment thread test/QmlUITests/TopLevelViewsTest.cc Outdated
Rely on the settings generator to append Fact userVisible conditions, and remove the redundant asynchronous check after synchronously overriding autoStreamConfig in the UI regression test.
@DonLakeFlyer

Copy link
Copy Markdown
Contributor

Can you squash into a single commit and rebase to current upstream? That should fix the ci failure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants