Add PARAM_EXT to parameter tab - #15030
Open
julianoes wants to merge 5 commits into
Open
Conversation
QGCCameraParamIO carried its own switch statements for encoding and decoding the 128 byte PARAM_EXT value blob. ParameterManager needs the same conversion for the extended parameter protocol, so move it to QGCMAVLink rather than duplicate it. The encode path previously used std::max where std::min was meant, reading past the end of a CUSTOM value shorter than 128 bytes.
Extended parameters were only reachable through the camera definition file, which means a camera's parameters could not be inspected or changed unless the definition happened to describe them. ParameterManager now queries them directly. Once the classic parameter download completes, PARAM_EXT_REQUEST_LIST goes out to every non-autopilot component heard from, and to any that appears later. Components which don't implement the protocol simply never answer. The query is held back until the classic download is done so the two don't compete for bandwidth, and missing indices are re-requested a bounded number of times. Ext facts are kept in their own map: they take no part in the load progress, the parameter cache, or the .params file, and they must be written with PARAM_EXT_SET rather than PARAM_SET. componentIds(), parameterNames() and getParameter() merge both maps, so the parameter editor picks them up with no UI change and lists them under their component. Writes go through a state machine mirroring the PARAM_SET one, handling PARAM_ACK_IN_PROGRESS as "keep waiting" and reporting rejections to the user. Because acks are broadcast, a parameter shown in both the parameter view and the camera settings UI stays in sync whichever side writes it. VehicleCameraControl no longer warns about ext values outside its camera definition, since a full list query makes those expected traffic. MockLinkCamera serves a small set of ext parameters on camera 1 only, so the tests also cover components that ignore the query.
Contributor
Build ResultsPlatform Status
All builds passed. Pre-commit
Pre-commit hooks: 2 passed, 77 failed, 7 skipped. Test Resultslinux-coverage-integration: 52 passed, 0 skipped Code CoverageCoverage: 70.7% No baseline available for comparison Artifact Sizes
Updated: 2026-09-04 00:43:43 UTC • Commit: 210289d • Triggered by: MacOS |
|
Tested with ArduPilot (SITL and hardware). Thanks! |
DonLakeFlyer
requested review from
DonLakeFlyer
and
a balanced review from Copilot
September 3, 2026 15:33
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Extended parameters are omitted from exports, collision recovery is incorrect, and one recovery test is ineffective.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds MAVLink PARAM_EXT parameters to the parameter editor alongside classic parameters.
Changes:
- Adds PARAM_EXT discovery, reading, writing, retries, and type conversion.
- Integrates extended parameters with camera handling and parameter-editor accessors.
- Expands MockLink camera simulation and ParameterManager tests.
File summaries
| File | Description |
|---|---|
test/FactSystem/ParameterManagerTest.h |
Declares PARAM_EXT tests and helpers. |
test/FactSystem/ParameterManagerTest.cc |
Tests extended parameter download and writes. |
src/MAVLink/QGCMAVLinkTypes.h |
Forward-declares PARAM_EXT message types. |
src/MAVLink/QGCMAVLink.h |
Declares PARAM_EXT conversion helpers. |
src/MAVLink/QGCMAVLink.cc |
Implements PARAM_EXT value encoding and decoding. |
src/FactSystem/ParameterManager.h |
Defines extended parameter state and APIs. |
src/FactSystem/ParameterManager.cc |
Implements PARAM_EXT lifecycle and editor integration. |
src/Comms/MockLink/MockLinkCamera.h |
Defines simulated extended camera parameters. |
src/Comms/MockLink/MockLinkCamera.cc |
Simulates PARAM_EXT request, value, set, and ACK handling. |
src/Comms/MockLink/MockLink.h |
Exposes the camera simulator to tests. |
src/Comms/MockLink/MockLink.cc |
Implements camera simulator access. |
src/Camera/VehicleCameraControl.cc |
Downgrades expected unknown PARAM_EXT messages to debug logs. |
src/Camera/QGCCameraIO.cc |
Reuses shared PARAM_EXT conversion helpers. |
Review details
- Files reviewed: 13/13 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
A PARAM_EXT_VALUE for a name the classic protocol already owns returned before the index was removed from the waiting map. The index stayed marked missing, was re-requested until the retries ran out, and was then warned about as never received even though it had arrived. Clear the index and restart the timeout as soon as the message is decoded, since the request has been answered whatever we then decide to do with the value. The same applies to a value we cannot decode at all.
MAV_PARAM_EXT_TYPE_CUSTOM is an opaque blob of bytes with no length on the wire. Decoding it as text truncates at the first null and mangles anything that isn't printable UTF-8, so the parameter view was showing a value that could be neither trusted nor written back. Camera definitions already give custom parameters no control for the same reason. Skip them, after the index book keeping so the download doesn't go looking for them again. Representing them properly needs a byte typed fact, an editor that can show and accept the bytes, and an answer to what the length actually is, which is worth doing on its own rather than as a footnote here. While here, record why extended parameters stay out of the .params file: nothing in the format distinguishes them from classic ones, so loading a file before the component has answered PARAM_EXT_REQUEST_LIST would send a plain PARAM_SET for a parameter that only accepts PARAM_EXT_SET.
The dropped index was configured from the test body, which runs after _connectMockLink has waited for initial connect. By then the list had already been streamed, so nothing was ever dropped and the test passed whether or not indexed recovery worked. Make it a MockConfiguration option instead, applied while the link is being constructed, and have the camera count the PARAM_EXT_REQUEST_READ by index it serves. Asserting the parameter was absent before recovery would race the retry timer against however long initial connect takes; the count proves the same thing without a timing window, since nothing else asks for a parameter that way. Verified by dropping the option again, which now fails the test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This adds PARAM_EXT parameters to the parameters tab, as requested in https://discord.com/channels/1022170275984457759/1022185820683255908/threads/1544457580351332386.
Tested using PX4 SITL together with this MAVSDK camera example.
Type of Change
Testing
Platforms Tested
Flight Stacks Tested
Screenshots
Checklist
Related Issues
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).