vertical unit setting fact bug fix#14539
Conversation
|
Thanks for your first pull request! 🎉 A maintainer will review this soon. Please ensure:
We appreciate your contribution to QGroundControl! |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR appears to align altitude-related behavior across UI and data-model layers by switching some bindings to rawValue, updating the “in control” system id source, and changing altitude unit strings.
Changes:
- Update QML bindings to use
rawValuefor altitude-related properties/ranges. - Switch control status tracking from
gcs_maintosysid_in_control. - Change altitude-related “units” strings from
"m"to"vertical m"in JSON fact/settings definitions.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Viewer3D/Viewer3DQml/Drones/DroneModelDjiF450.qml | Uses altitudeRelative.rawValue for 3D model Z positioning |
| src/Vehicle/Vehicle.cc | Uses sysid_in_control for _gcsMain updates |
| src/Vehicle/FactGroups/VehicleFact.json | Updates altitude fact unit strings to "vertical m" |
| src/Settings/FlyView.SettingsGroup.json | Updates guided altitude setting unit strings to "vertical m" |
| src/FirmwarePlugin/PX4/PX4FlightModeIndicator.qml | Uses guidedMaximumAltitude.rawValue for slider max |
| "type": "double", | ||
| "decimalPlaces": 1, | ||
| "units": "m" | ||
| "units": "vertical m" |
| "type": "double", | ||
| "decimalPlaces": 1, | ||
| "units": "m" | ||
| "units": "vertical m" |
| "type": "double", | ||
| "decimalPlaces": 1, | ||
| "units": "m" | ||
| "units": "vertical m" |
| "shortDesc": "Minimum altitude allowed for guided mode actions like takeoff and altitude changes.", | ||
| "type": "double", | ||
| "units": "m", | ||
| "units": "vertical m", |
| "shortDesc": "Maximum altitude allowed for guided mode actions like takeoff and altitude changes.", | ||
| "type": "double", | ||
| "units": "m", | ||
| "units": "vertical m", |
| Layout.fillWidth: true | ||
| fact: controller.getParameterFact(-1, "GF_MAX_VER_DIST") | ||
| to: flyViewSettings.guidedMaximumAltitude.value | ||
| to: flyViewSettings.guidedMaximumAltitude.rawValue |
| if (_gcsMain != controlStatus.sysid_in_control) { | ||
| _gcsMain = controlStatus.sysid_in_control; |
|
See the Build Results workflow run for details. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #14539 +/- ##
==========================================
+ Coverage 25.47% 30.20% +4.73%
==========================================
Files 769 772 +3
Lines 65912 66943 +1031
Branches 30495 31090 +595
==========================================
+ Hits 16788 20220 +3432
+ Misses 37285 32990 -4295
- Partials 11839 13733 +1894
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
n-sleeman
force-pushed
the
alt-unit-bug
branch
7 times, most recently
from
86237e8 to
f92bc59
Compare
Description
Bug: vertical units handled by horizontal unit setting fact.
Fix: switched to use "vertical m" FactMetaData for Vertical unit setting fact
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).