Skip to content

fix(Vehicle): remove initial connect outer timeouts causing slow-link failures - #15020

Merged
DonLakeFlyer merged 1 commit into
mavlink:masterfrom
DonLakeFlyer:RemoveInitialConnectTimeouts
Sep 2, 2026
Merged

fix(Vehicle): remove initial connect outer timeouts causing slow-link failures#15020
DonLakeFlyer merged 1 commit into
mavlink:masterfrom
DonLakeFlyer:RemoveInitialConnectTimeouts

Conversation

@DonLakeFlyer

@DonLakeFlyer DonLakeFlyer commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Problem

Users reported sporadic initial-connect failures over slow links (e.g. SiK radios). The cause is the fixed outer timeouts on each InitialConnectStateMachine state (params 60s, comp info 30s, mission 30s, etc.). On slow links these fire even though the transfer is progressing normally. The timeout retry path then force-advanced the machine, leaving the vehicle half-initialized (missing params, modes, or plan).

Fix

Remove all outer timeouts and the retry-transition wiring from InitialConnectStateMachine. Every underlying subsystem already handles its own per-step timeouts/retries and always signals completion:

  • ParametersParameterManager retries per-param and signals parametersReadyChanged. A new terminal initialParametersRequestFailed signal covers the case where the vehicle never answers PARAM_REQUEST_LIST, so the state advances without parameters instead of relying on an outer timeout.
  • Mission/GeoFence/RallyPlanManager has per-message timeouts/retries.
  • Component informationComponentInformationManager's nested state machines have per-state timeouts on every step. A new requestAllComplete signal replaces the callback plumbing for completion.
  • Standard modes — the protocol handles timeouts internally and always signals requestCompleted.
  • AUTOPILOT_VERSION — the one request with no lower-level protocol timeout keeps its per-attempt timeout/retry inside RetryableRequestMessageState.

Additional fix: GimbalController request collision

GimbalController sent MAV_CMD_REQUEST_MESSAGE (GIMBAL_MANAGER_INFORMATION) raw via sendMavCommand. The command queue's duplicate-command check ignores param1 (the requested message id), so this flakily collided with RequestMessageCoordinator users targeting the same component (e.g. the AVAILABLE_MODES fetch during initial connect), failing them as duplicates. Now routed through Vehicle::requestMessage() with an in-flight guard so it serializes with other request-message users.

Supporting changes

  • State machine wait timeouts and retries now log at warning level (visible without logging filters) including the elapsed timeout value, so genuine stalls are diagnosable in the field.
  • StandardModes failure logs the MAV_RESULT name and failure code and is promoted to warning.
  • ParameterManager logging cleanup (cache write log, clearer timeout/cache messages, per-param remap log moved to verbose).
  • QGCFileDownload logs URLs with user info/query stripped.
  • MockLink: AVAILABLE_MODES now served on high-latency links; the automatic modes-monitor sequence bump is replaced with an explicit test API (bumpAvailableModesMonitorSequence) exercised by a new StandardModesTest.
  • Tests: fixture-wide log ignores replaced with per-test scoped ignores documenting the expected cause; obsolete outer-timeout tests reworked to verify subsystem failures fall through to initial-connect completion.

Copilot AI balanced review requested due to automatic review settings September 1, 2026 17:18
@DonLakeFlyer DonLakeFlyer added this to the Release V5.1 milestone Sep 1, 2026

This comment was marked as resolved.

@github-actions

github-actions Bot commented Sep 1, 2026

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: 2 passed, 76 failed, 7 skipped.

Test Results

linux-coverage-integration: 52 passed, 0 skipped
linux-coverage-unit: 184 passed, 0 skipped
linux-sanitizers-integration: 31 passed, 0 skipped
linux-sanitizers-unit: 184 passed, 0 skipped
Total: 451 passed, 0 skipped

Code Coverage

Coverage: 70.6%

No baseline available for comparison

Artifact Sizes

Artifact Size
QGroundControl 253.09 MB
QGroundControl-aarch64 177.30 MB
QGroundControl-installer-AMD64 128.42 MB
QGroundControl-installer-AMD64-ARM64 70.82 MB
QGroundControl-installer-ARM64 99.55 MB
QGroundControl-linux 84.76 MB
QGroundControl-mac 84.76 MB
QGroundControl-x86_64 189.57 MB
No baseline available for comparison

Updated: 2026-09-02 03:11:41 UTC • Commit: 85a1c6b • Triggered by: Linux

This comment was marked as resolved.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Gimbal/GimbalController.cc:279

  • The in-flight guard returns without sending, but both callers still decrement their retry counters unconditionally (_handleHeartbeat and _checkComplete). While one request is queued or awaiting retries, repeated heartbeat/status messages can therefore exhaust every retry—and with multiple managers, a manager can exhaust its retries without ever issuing a request. Return whether a request was actually submitted and decrement only on success.
    if (_pendingInformationRequestCompId != -1) {
        qCDebug(GimbalControllerLog) << "_requestGimbalInformation: request already in flight for compid" << _pendingInformationRequestCompId;
        return;

@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 41.42857% with 41 lines in your changes missing coverage. Please review.
✅ Project coverage is 34.50%. Comparing base (f29efd3) to head (85a1c6b).
⚠️ Report is 352 commits behind head on master.

Files with missing lines Patch % Lines
src/Vehicle/RequestMessageCoordinator.cc 20.00% 10 Missing and 2 partials ⚠️
src/FactSystem/ParameterManager.cc 20.00% 0 Missing and 8 partials ⚠️
src/Utilities/Network/QGCFileDownload.cc 0.00% 4 Missing and 1 partial ⚠️
src/Vehicle/InitialConnectStateMachine.cc 66.66% 2 Missing and 3 partials ⚠️
src/Gimbal/GimbalController.cc 66.66% 1 Missing and 3 partials ⚠️
...nentInformation/RequestMetaDataTypeStateMachine.cc 0.00% 1 Missing and 1 partial ⚠️
src/Comms/MockLink/MockLink.cc 50.00% 0 Missing and 1 partial ⚠️
src/Utilities/StateMachine/States/WaitStateBase.cc 0.00% 0 Missing and 1 partial ⚠️
...lities/StateMachine/Transitions/RetryTransition.cc 0.00% 0 Missing and 1 partial ⚠️
src/Vehicle/StandardModes.cc 66.66% 0 Missing and 1 partial ⚠️
... and 1 more
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #15020      +/-   ##
==========================================
+ Coverage   25.47%   34.50%   +9.03%     
==========================================
  Files         769      945     +176     
  Lines       65912    85606   +19694     
  Branches    30495    39453    +8958     
==========================================
+ Hits        16788    29538   +12750     
+ Misses      37285    36428     -857     
- Partials    11839    19640    +7801     
Flag Coverage Δ
unittests 34.50% <41.42%> (+9.03%) ⬆️

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

Files with missing lines Coverage Δ
src/Comms/MockLink/MockLink.h 64.58% <100.00%> (-4.87%) ⬇️
src/FactSystem/ParameterManager.h 83.33% <ø> (ø)
src/Gimbal/GimbalController.h 50.00% <ø> (+14.28%) ⬆️
...omponentInformation/ComponentInformationManager.cc 62.02% <100.00%> (-4.22%) ⬇️
...ComponentInformation/ComponentInformationManager.h 50.00% <ø> (ø)
src/Comms/MockLink/MockLink.cc 57.34% <50.00%> (+6.46%) ⬆️
src/Utilities/StateMachine/States/WaitStateBase.cc 72.05% <0.00%> (-5.89%) ⬇️
...lities/StateMachine/Transitions/RetryTransition.cc 52.38% <0.00%> (ø)
src/Vehicle/StandardModes.cc 42.85% <66.66%> (-7.70%) ⬇️
src/Vehicle/Vehicle.cc 31.05% <50.00%> (+9.64%) ⬆️
... and 6 more

... and 650 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@DonLakeFlyer
DonLakeFlyer force-pushed the RemoveInitialConnectTimeouts branch from 0ec0001 to 28948c1 Compare September 1, 2026 21:29
Comment thread src/Gimbal/GimbalController.cc Dismissed
… failures

The fixed outer timeouts on each InitialConnectStateMachine state could fire on
slow links (e.g. SiK radios) even though the underlying protocols (ParameterManager,
PlanManager, ComponentInformationManager, StandardModes) handle their own
timeouts/retries and always signal completion. Force-advancing on timeout left the
vehicle half-initialized.

ParameterManager gains a terminal initialParametersRequestFailed signal so the
Parameters state can advance when the vehicle never answers PARAM_REQUEST_LIST.

Also fixes GimbalController sending MAV_CMD_REQUEST_MESSAGE raw, which collided
with RequestMessageCoordinator users in the command queue duplicate check and
flakily failed the AVAILABLE_MODES fetch.

Also improves timeout/failure logging visibility and sanitizes downloaded URL
logging.
@DonLakeFlyer
DonLakeFlyer force-pushed the RemoveInitialConnectTimeouts branch from 28948c1 to 85a1c6b Compare September 2, 2026 01:56
@DonLakeFlyer
DonLakeFlyer merged commit c279a29 into mavlink:master Sep 2, 2026
48 checks passed
@DonLakeFlyer
DonLakeFlyer deleted the RemoveInitialConnectTimeouts branch September 2, 2026 03:36
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.

3 participants