Skip to content

feat(FactSystem): add BulkRefreshJob / bulkRefresh() to replace scatter-shot refreshParameter calls#14413

Merged
DonLakeFlyer merged 2 commits into
mavlink:masterfrom
DonLakeFlyer:feat/bulk-refresh-parameter-api
May 19, 2026
Merged

feat(FactSystem): add BulkRefreshJob / bulkRefresh() to replace scatter-shot refreshParameter calls#14413
DonLakeFlyer merged 2 commits into
mavlink:masterfrom
DonLakeFlyer:feat/bulk-refresh-parameter-api

Conversation

@DonLakeFlyer
Copy link
Copy Markdown
Collaborator

Summary

Adds a proper batch-parameter-refresh API to ParameterManager and uses it
to eliminate the post-compass-calibration popup spam described in #14347 /
#14346.

Problem

_refreshParams() in both the APM and PX4 sensor controllers called
refreshParameter() and refreshParametersPrefix() individually. The FC
is often slow to respond mid-cancel; every timed-out request fired a modal
"Parameter read failed" via QGC::showAppMessage, stacking dozens of
dialogs and freezing the UI.

Solution

BulkRefreshJob (new src/FactSystem/BulkRefreshJob.{h,cc}):
A QObject-child of ParameterManager that fires a batch of
PARAM_REQUEST_READ messages and retries (exponential back-off, up to
kMaxRetryRounds = 3 rounds) any that time out. notifyFailure = false
suppresses the user-visible modal while still logging and emitting
_paramRequestReadFailure for internal use. Self-deletes on completion.

ParameterManager::bulkRefresh() (new public API):
Accepts a list of exact names and/or wildcard prefixes (e.g. "COMPASS_*"),
resolves them against the known parameter set, deduplicates, and delegates to
BulkRefreshJob.

Sensor controllers updated to use a single bulkRefresh() call with
notifyFailure = false, replacing the old scattered individual-refresh loops.

QML (APMSensorsComponent.qml): signal handlers converted to arrow-function
syntax to fix qmllint warnings.

Test-speed note

_waitForParamValueAckMs and BulkRefreshJob::_retryBaseDelayMs are set to
50 ms under QGC::runningUnitTests() so the new retry tests complete in ~1 s
instead of ~19 s.

Test plan

  • ctest -L Unit -R ParameterManager — all 19 tests pass (5 new tests added,
    including retry-succeeds and all-rounds-exhausted paths)
  • Start a compass cal on ArduPilot, cancel mid-way → no flood of
    "Parameter read failed" modals; UI stays responsive
  • Normal compass cal to completion on ArduPilot and PX4 → params repopulate,
    no popup spam

Addresses the compass-cal refresh spam described in #14347.

Add a BulkRefreshJob class that fires a batch of PARAM_REQUEST_READs with
exponential-backoff retry. Owned by ParameterManager as a QObject child;
self-deletes on completion.

Add ParameterManager::bulkRefresh() which resolves a mix of exact names
and wildcard prefixes (e.g. "COMPASS_*") against the known parameter set,
deduplicates, and delegates to BulkRefreshJob.

Timeouts (_waitForParamValueAckMs, BulkRefreshJob::_retryBaseDelayMs) are
reduced to 50 ms under QGC::runningUnitTests() so the test suite stays fast.

Add five unit tests covering: exact names, prefix expansion, unknown name
skipped, round-0-failure-then-retry-succeeds, and all-rounds-exhausted.
@DonLakeFlyer DonLakeFlyer force-pushed the feat/bulk-refresh-parameter-api branch from a62226d to 2830339 Compare May 19, 2026 20:41
@DonLakeFlyer DonLakeFlyer marked this pull request as ready for review May 19, 2026 20:42
@DonLakeFlyer DonLakeFlyer requested a review from HTRamsey as a code owner May 19, 2026 20:42
Copilot AI review requested due to automatic review settings May 19, 2026 20:42

This comment was marked as resolved.

…pups

After compass calibration stops/cancels the FC is often slow to answer.
The old code called refreshParameter + refreshParametersPrefix individually,
causing a modal "Parameter read failed" popup for every timed-out request
which could stack dozens of dialogs and freeze the UI (mavlink#14346).

Replace _refreshParams() in both APM and PX4 sensor controllers with a
single bulkRefresh() call (notifyFailure=false). Retry logic and logging
are preserved inside BulkRefreshJob; only the user-visible modal is suppressed.

Also fix QML signal handler syntax (arrow-function form) in
APMSensorsComponent.qml to silence qmllint warnings.

Addresses the compass-cal refresh spam described in mavlink#14347.
@DonLakeFlyer DonLakeFlyer force-pushed the feat/bulk-refresh-parameter-api branch from 2830339 to 6b5d3e2 Compare May 19, 2026 20:56
@codecov
Copy link
Copy Markdown

codecov Bot commented May 19, 2026

Codecov Report

❌ Patch coverage is 41.90476% with 61 lines in your changes missing coverage. Please review.
✅ Project coverage is 25.57%. Comparing base (f29efd3) to head (6b5d3e2).
⚠️ Report is 23 commits behind head on master.

Files with missing lines Patch % Lines
src/FactSystem/BulkRefreshJob.cc 54.23% 8 Missing and 19 partials ⚠️
src/FactSystem/ParameterManager.cc 32.43% 5 Missing and 20 partials ⚠️
...AutoPilotPlugins/PX4/SensorsComponentController.cc 0.00% 5 Missing ⚠️
...oPilotPlugins/APM/APMSensorsComponentController.cc 0.00% 4 Missing ⚠️

❌ Your project check has failed because the head coverage (25.57%) is below the target coverage (30.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14413      +/-   ##
==========================================
+ Coverage   25.47%   25.57%   +0.10%     
==========================================
  Files         769      767       -2     
  Lines       65912    66281     +369     
  Branches    30495    30663     +168     
==========================================
+ Hits        16788    16954     +166     
+ Misses      37285    37279       -6     
- Partials    11839    12048     +209     
Flag Coverage Δ
unittests 25.57% <41.90%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/FactSystem/ParameterManager.h 83.33% <ø> (ø)
...oPilotPlugins/APM/APMSensorsComponentController.cc 0.00% <0.00%> (ø)
...AutoPilotPlugins/PX4/SensorsComponentController.cc 0.00% <0.00%> (ø)
src/FactSystem/ParameterManager.cc 36.78% <32.43%> (-0.02%) ⬇️
src/FactSystem/BulkRefreshJob.cc 54.23% <54.23%> (ø)

... and 30 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Copy Markdown
Contributor

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, 45 failed, 7 skipped.

Test Results

linux-coverage: 87 passed, 0 skipped
Total: 87 passed, 0 skipped

Code Coverage

Coverage: 59.4%

No baseline available for comparison

Artifact Sizes

Artifact Size
QGroundControl 216.77 MB
QGroundControl-aarch64 176.67 MB
QGroundControl-installer-AMD64 134.68 MB
QGroundControl-installer-AMD64-ARM64 77.29 MB
QGroundControl-installer-ARM64 106.01 MB
QGroundControl-linux 334.68 MB
QGroundControl-mac 186.86 MB
QGroundControl-windows 186.87 MB
QGroundControl-x86_64 171.88 MB
No baseline available for comparison

Updated: 2026-05-19 21:45:35 UTC • Triggered by: Android

@DonLakeFlyer DonLakeFlyer merged commit 483aae4 into mavlink:master May 19, 2026
32 of 33 checks passed
@DonLakeFlyer DonLakeFlyer deleted the feat/bulk-refresh-parameter-api branch May 19, 2026 21:53
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.

2 participants