Skip to content

Add support for read-only parameters#14007

Open
julianoes wants to merge 6 commits intomasterfrom
pr-readonly-params
Open

Add support for read-only parameters#14007
julianoes wants to merge 6 commits intomasterfrom
pr-readonly-params

Conversation

@julianoes
Copy link
Copy Markdown
Contributor

Description

This parses the readonly flag for a parameter, so that it's clear in the UI when a param can't be set.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)
  • CI/Build changes
  • Other

Testing

  • Tested locally
  • Added/updated unit tests
  • Tested with simulator (SITL)
  • Tested with hardware

Platforms Tested

  • Linux
  • Windows
  • macOS
  • Android
  • iOS

Flight Stacks Tested

  • PX4
  • ArduPilot

Screenshots

image

Checklist

  • I have read the Contribution Guidelines
  • I have read the Code of Conduct
  • My code follows the project's coding standards
  • I have added tests that prove my fix/feature works
  • New and existing unit tests pass locally

Related Issues

Goes together with PX4/PX4-Autopilot#26522.


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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Feb 19, 2026

Build Results

Platform Status

Platform Status Details
Linux Passed View
Windows Passed View
MacOS Passed View
Android Passed View

All builds passed.

Pre-commit

Check Status Details
pre-commit Failed (non-blocking) View

Pre-commit hooks: 4 passed, 36 failed, 7 skipped.

Test Results

linux-coverage: 78 passed, 0 skipped
linux-sanitizers: 78 passed, 0 skipped
Total: 156 passed, 0 skipped

Code Coverage

Coverage: 58.2%

No baseline available for comparison

Artifact Sizes

Artifact Size
QGroundControl 247.79 MB
QGroundControl-aarch64 177.24 MB
QGroundControl-installer-AMD64 134.84 MB
QGroundControl-installer-AMD64-ARM64 77.44 MB
QGroundControl-installer-ARM64 106.17 MB
QGroundControl-linux 338.32 MB
QGroundControl-mac 188.81 MB
QGroundControl-windows 188.82 MB
QGroundControl-x86_64 172.36 MB
No baseline available for comparison

Updated: 2026-04-14 23:15:49 UTC • Triggered by: Android

@julianoes
Copy link
Copy Markdown
Contributor Author

Added a commit to display a lock and add a checkbox to filter them out.

image

@hamishwillee
Copy link
Copy Markdown
Collaborator

That is very cool. I'm slightly surprised it doesn't already exist since ArduPilot support read only params

@julianoes
Copy link
Copy Markdown
Contributor Author

I need to check how read only params work in ArduPilot then, so this is compatible.

@hamishwillee
Copy link
Copy Markdown
Collaborator

I need to check how read only params work in ArduPilot then, so this is compatible.

Indeed! Though if they don't have an icon like this then they would probably appreciate one :-)

@julianoes
Copy link
Copy Markdown
Contributor Author

@hamishwillee from my research I don't see how AP would be communicating this to QGC. I think we can leave this PR as is.

@hamishwillee
Copy link
Copy Markdown
Collaborator

Cool. I have no more comments. I guess get Don or Holden to look at it now.

@Ryanf55
Copy link
Copy Markdown
Contributor

Ryanf55 commented Mar 3, 2026

FWIW, ArduPilot likes OEM's to use this script.
https://github.com/ArduPilot/ardupilot/blob/master/libraries/AP_Scripting/applets/param-set.lua

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds end-to-end support for “read-only” parameter metadata so the Parameter UI can indicate non-editable parameters and optionally hide them from the list.

Changes:

  • Parse a new readOnly boolean from parameter metadata JSON into FactMetaData.
  • Update the parameter editor dialog to show read-only messaging and prevent editing/saving.
  • Add UI affordances: a lock icon in the parameter list and a “Hide read-only” filter wired through ParameterEditorController.

Reviewed changes

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

Show a summary per file
File Description
src/QmlControls/ParameterEditorDialog.qml Disables editing for read-only facts, shows read-only messaging/value-only display.
src/QmlControls/ParameterEditorController.h Adds hideReadOnly property/signal/slot to support filtering.
src/QmlControls/ParameterEditorController.cc Implements read-only filtering in list build/search and rebuilds lists on toggle.
src/QmlControls/ParameterEditor.qml Adds “Hide read-only” checkbox and displays a lock icon for read-only facts in the table.
src/FactSystem/FactMetaData.h Adds JSON key constant for readOnly.
src/FactSystem/FactMetaData.cc Parses readOnly from JSON metadata.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread src/FactSystem/FactMetaData.cc Outdated
Comment thread src/QmlControls/ParameterEditor.qml Outdated
Comment thread src/QmlControls/ParameterEditor.qml Outdated
Comment thread src/FactSystem/FactMetaData.cc Outdated
}
metaData->setVolatileValue(volatileValue);

if (json.contains(_readOnlyJsonKey)) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This screws up the positioning of the setVolatileValue with the rest of it's code. Also it doesn't follow the pattern and the rest of the value setting. It should do it the same way as the others.

@DonLakeFlyer
Copy link
Copy Markdown
Collaborator

FWIW, ArduPilot likes OEM's to use this script.

For what?

@DonLakeFlyer
Copy link
Copy Markdown
Collaborator

Other question: Did the NCELLS param suddenly become ready only. That is part of Power Setup page as something user should set.

@DonLakeFlyer
Copy link
Copy Markdown
Collaborator

Code review notes:

1. onAccepted still fires for read-only parameters (ParameterEditorDialog.qml)

The dialog uses Dialog.Close for read-only facts, but the onAccepted handler doesn't guard against writes. If Close ever triggers accepted, the handler would attempt to write to a read-only fact. Consider adding an early return:

onAccepted: {
    if (fact.readOnly) return
    ...
}

2. _hideReadOnlyChanged duplicates _buildLists teardown logic (ParameterEditorController.cc)

The _hideReadOnlyChanged method manually clears _currentCategory, _currentGroup, _parameters, _mapCategoryName2Category, and calls _categories.clearAndDeleteContents() — this is the same teardown that _buildLists() already does internally. This duplication is fragile; if _buildLists teardown changes, this code won't stay in sync. Consider extracting a _clearLists() helper, or just calling _buildLists() directly since it already handles the full rebuild.

@julianoes
Copy link
Copy Markdown
Contributor Author

Other question: Did the NCELLS param suddenly become ready only. That is part of Power Setup page as something user should set.

No, that's just one that I tried. No param is read only by default. It's just a feature that a drone manufacturer can use.

@DonLakeFlyer
Copy link
Copy Markdown
Collaborator

ArduPilot has read only support in their json metadata. This should be carried through to updating the COMPONENT_INFORMATION spec to add read only metadata as well.

@hamishwillee
Copy link
Copy Markdown
Collaborator

ArduPilot has read only support in their json metadata. This should be carried through to updating the COMPONENT_INFORMATION spec to add read only metadata as well.

I think so too, but we do need to be careful in case read-only might potentially be a dynamic field, or depend on "who you are".

@DonLakeFlyer
Copy link
Copy Markdown
Collaborator

mavlink/mavlink#2449

@julianoes
Copy link
Copy Markdown
Contributor Author

Alright I've just merged the MAVLink PR, thanks for that @DonLakeFlyer. And I've also rebased the PX4 commit, so that it can hopefully go in soon.

And I've just rebased this and addressed the review comments.

This is how it looks like, and the lock is only displayed for readOnly params, but not for volatile ones:

image

@DonLakeFlyer
Copy link
Copy Markdown
Collaborator

My thinking is that the lock icon should go after the param name not before. I think this makes things more readable when you scan down the names.

julianoes and others added 6 commits April 15, 2026 09:59
This parses the readonly flag for a parameter, so that it's clear in the
UI when a param can't be set.
Show a lock icon next to read-only parameter names in the parameter
list and add a "Hide read-only" checkbox to filter them out entirely.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Add readOnly guard in ParameterEditorDialog onAccepted handler
- Make readOnly JSON parsing follow the same pattern as other bools
- Parse readOnly before volatile so setVolatileValue can enforce readOnly
- Move teardown logic into _buildLists to avoid duplication in _hideReadOnlyChanged
- Remove _readOnly = true from setVolatileValue() since volatile params
  (e.g. calibration offsets) should still be user-writable
- Fix description column width being cut off due to wrong implicitWidth
- Fix QML warning about assigning Fact to QString in name column
@julianoes julianoes force-pushed the pr-readonly-params branch from 4c9826d to e81d5fa Compare April 14, 2026 22:17
@julianoes
Copy link
Copy Markdown
Contributor Author

Ok, changed:

image

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.

5 participants