Fix parameter file diff loader and compass-cal refresh popup spam#14347
Fix parameter file diff loader and compass-cal refresh popup spam#14347HTRamsey wants to merge 2 commits into
Conversation
…ff loader buildDiffFromFile only split lines on \t, so files with whitespace-aligned columns (Mission Planner / ArduPilot tooling exports like `1 1 SERVO_BLH_AUTO 1 2`) and simple `name,value` CSVs produced zero parsed entries. The diff dialog then reported "There are no differences between the file loaded and the current settings on the Vehicle" even when values clearly differed. Split on `[\s,]+` after trimming, accept both the existing 5-token `vehicleId componentId name value mavType` format and a new 2-token `name value` form. For 2-token rows the component is resolved by walking the vehicle's component ids and the type is derived from the live Fact, so we can still build a typed diff without the extra columns. Fixes mavlink#14346
…efresh After a compass calibration stops (especially on cancel) the FC is often slow to answer the bulk `COMPASS_*` / `INS_*` (or `CAL_*` / `SENS_*` on PX4) refresh that `_refreshParams` issues. Each timeout went through `_mavlinkParamRequestRead` with `notifyFailure=true`, firing a modal "Parameter read failed: …" via `QGC::showAppMessage` for every parameter. Dozens of modals stack on the QML root and freeze the UI. Add a `notifyFailure` parameter (defaults to true) to `ParameterManager::refreshParameter` and `refreshParametersPrefix` and pass `false` from the post-calibration refresh paths in both the APM and PX4 sensors controllers. The state machine still logs failures and emits `_paramRequestReadFailure`; only the user-facing modal is suppressed for the batch refresh.
|
@DonLakeFlyer this is a quick sample to show you what fixed some of the issues, in case you want to make your own PR based on this |
There was a problem hiding this comment.
Pull request overview
Fixes two parameter-system UX issues: parameter file diff import now correctly parses Mission Planner/ArduPilot-style whitespace-separated and simple name,value files, and post-compass-calibration parameter refreshes no longer spam modal “Parameter read failed” popups that can freeze the UI.
Changes:
- Broadened
ParameterEditorController::buildDiffFromFileparsing to accept whitespace/comma separators and both 5-column and 2-column formats. - Added a
notifyFailureflag toParameterManager::refreshParameter/refreshParametersPrefixto optionally suppress user-facing failure popups while still logging/emitting failure signals. - Updated PX4/APM sensor controllers to disable failure popups during bulk post-calibration refreshes.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/QmlControls/ParameterEditorController.cc | Makes the diff loader accept whitespace/comma-separated parameter files and 2-token name value lines. |
| src/FactSystem/ParameterManager.h | Extends refresh APIs with an optional notifyFailure parameter (defaulting to current behavior). |
| src/FactSystem/ParameterManager.cc | Wires notifyFailure through to the PARAM_REQUEST_READ state machine failure path. |
| src/AutoPilotPlugins/PX4/SensorsComponentController.cc | Suppresses per-parameter failure popups during bulk post-cal refresh. |
| src/AutoPilotPlugins/APM/APMSensorsComponentController.cc | Suppresses per-parameter failure popups during bulk post-cal refresh. |
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (2.56%) is below the target coverage (30.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #14347 +/- ##
=========================================
Coverage ? 25.29%
=========================================
Files ? 764
Lines ? 65561
Branches ? 30314
=========================================
Hits ? 16585
Misses ? 37316
Partials ? 11660
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Build ResultsPlatform Status
Some builds failed. Pre-commit
Pre-commit hooks: 4 passed, 44 failed, 7 skipped. Test Resultslinux-coverage: 90 passed, 0 skipped Code CoverageCoverage: 59.1% No baseline available for comparison Artifact Sizes
Updated: 2026-05-09 16:22:52 UTC • Triggered by: Android |
|
Ok, thanks. I'll take a look... |
Summary
Two parameter-system UX bugs surfaced together in #14346.
ParameterEditorController::buildDiffFromFileonly split on\t, so files with whitespace-aligned columns (Mission Planner / ArduPilot tooling, e.g.1 1 SERVO_BLH_AUTO 1 2) and simplename,valueCSVs produced zero parsed entries and the dialog reported "There are no differences between the file loaded and the current settings on the Vehicle" even when values clearly differed. Now splits on[\s,]+and accepts both the 5-column and a new 2-column format._refreshParamsrequests a bulk refresh ofCOMPASS_*/INS_*(APM) orCAL_*/SENS_*(PX4). The FC is often slow to answer mid-cancel, every timeout fired a modal "Parameter read failed" viaQGC::showAppMessage, and dozens of modals stacked on the QML root and froze the UI. Added anotifyFailureflag (default true, preserves existing call sites) toParameterManager::refreshParameter/refreshParametersPrefixand passfalsefrom the post-cal refresh paths in both sensors controllers. The state machine still logs and emits_paramRequestReadFailure; only the modal is suppressed for the batch refresh.Fixes #14346.
Test plan
.paramfile with at least one differing value → diff dialog lists differences and applies them on OK.name,valueCSV → same.ctest -L Unit -R Parameterpasses.