Skip to content

fix(MavlinkLogTest): remove QSKIP and fix _startLogging race after vehicle disconnect #14507

Description

@DonLakeFlyer

Background

MavlinkLogTest::_connectLogArm_test is currently QSKIPped because MAVLinkProtocol::_startLogging() contains an early return for all unit tests:

void MAVLinkProtocol::_startLogging()
{
    if (QGC::runningUnitTests()) {
        return;
    }
    ...

This means no temp log file is ever opened during tests, so _stopLogging() never saves anything, and the condition savedTelemetryLogCount() == initialSavedLogCount + 1 can never be true. The test burned 5 s on a guaranteed timeout and then passed by asserting the opposite of what it was supposed to verify.

Root Cause

The runningUnitTests() guard was added to prevent a real race condition: after vehicleRemoved fires _stopLogging(), late-arriving MAVLink bytes (still in the event queue from settleEventLoopForCleanup) can re-trigger _startLogging() and open an orphan temp file that _stopLogging() will never close. This race exists in production too — orphan files are only recovered by checkForLostLogFiles() on the next app start.

What Needs to Be Done

  1. Fix the production race in _startLogging() — guard against opening a new log when vehicle count is already 0, or track a "logging enabled" flag that is cleared before the event loop settles:
    if (MultiVehicleManager::instance()->vehicles()->count() == 0) {
        return;
    }
  2. Remove the runningUnitTests() guard from _startLogging().
  3. Remove the QSKIP from _connectLogArm_test and verify it passes.
  4. Ensure _connectLogNoArm_test still passes (no orphan temp files).

Affected Files

  • src/Comms/MAVLinkProtocol.cc — production fix
  • test/AnalyzeView/MavlinkLogTest.cc — remove QSKIP

Metadata

Metadata

Assignees

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions