From 59566202f4c0bc072f2088567b4f4e43c8c69437 Mon Sep 17 00:00:00 2001 From: Don Gagne Date: Mon, 20 Jul 2026 19:13:44 -0700 Subject: [PATCH] fix(Vehicle): clear communicationLost as soon as any link regains comms Previously recovery from total communication loss required all links to regain comms before communicationLost was cleared. This is asymmetric with how the state is set (all links must be lost) and left the vehicle reported as comm lost even though a working link was available. Fixes #14666 --- src/Vehicle/VehicleLinkManager.cc | 20 +++------ test/Vehicle/VehicleLinkManagerTest.cc | 60 ++++++++++++++++++++++++++ test/Vehicle/VehicleLinkManagerTest.h | 1 + 3 files changed, 67 insertions(+), 14 deletions(-) diff --git a/src/Vehicle/VehicleLinkManager.cc b/src/Vehicle/VehicleLinkManager.cc index d3eabbd6a4bb..e53c114a7b91 100644 --- a/src/Vehicle/VehicleLinkManager.cc +++ b/src/Vehicle/VehicleLinkManager.cc @@ -9,6 +9,8 @@ #endif #include "QGCLoggingCategory.h" +#include + QGC_LOGGING_CATEGORY(VehicleLinkManagerLog, "Vehicle.VehicleLinkManager") VehicleLinkManager::VehicleLinkManager(Vehicle *vehicle) @@ -86,20 +88,10 @@ void VehicleLinkManager::_commRegainedOnLink(LinkInterface *link) emit linkStatusesChanged(); - // Check recovery from total communication loss - if (!_communicationLost) { - return; - } - - bool noCommunicationLoss = true; - for (const LinkInfo_t &linkInfo: _rgLinkInfo) { - if (linkInfo.commLost) { - noCommunicationLoss = false; - break; - } - } - - if (noCommunicationLoss) { + // Any single regained link ends total communication loss + if (_communicationLost && + std::any_of(_rgLinkInfo.cbegin(), _rgLinkInfo.cend(), + [](const LinkInfo_t &info) { return !info.commLost; })) { _communicationLost = false; emit communicationLostChanged(_communicationLost); } diff --git a/test/Vehicle/VehicleLinkManagerTest.cc b/test/Vehicle/VehicleLinkManagerTest.cc index fa229b3470d3..010269244d72 100644 --- a/test/Vehicle/VehicleLinkManagerTest.cc +++ b/test/Vehicle/VehicleLinkManagerTest.cc @@ -71,6 +71,7 @@ void VehicleLinkManagerTest::_simpleCommLossTest() SharedLinkInterfacePtr mockLink; _startMockLink(1, false /*highLatency*/, true /*incrementVehicleId*/, mockConfig, mockLink); MockLink* const pMockLink = qobject_cast(mockLink.get()); + QVERIFY(pMockLink); Vehicle* const vehicle = waitForVehicleConnect(TestTimeout::shortMs()); QVERIFY(vehicle); @@ -131,6 +132,8 @@ void VehicleLinkManagerTest::_multiLinkSingleVehicleTest() MockLink* pMockLink1 = qobject_cast(mockLink1.get()); MockLink* pMockLink2 = qobject_cast(mockLink2.get()); + QVERIFY(pMockLink1); + QVERIFY(pMockLink2); if (primaryLink == mockLink2) { std::swap(pMockLink1, pMockLink2); } @@ -205,6 +208,60 @@ void VehicleLinkManagerTest::_multiLinkSingleVehicleTest() multiSpy.clearAllSignals(); } +void VehicleLinkManagerTest::_multiLinkTotalCommLossRecoveryTest() +{ + // Comm loss with pending vehicle commands causes MavCommandQueue to give up. + ignoreLogMessage("Vehicle.MavCommandQueue", QtWarningMsg, + QRegularExpression("Giving up sending command after max retries:")); + // Primary link switchover produces a showAppMessage debug log. + ignoreLogMessage("API.QGCApplication.AppMessage", QtDebugMsg, + QRegularExpression("Switching communication to")); + + SharedLinkConfigurationPtr mockConfig1; + SharedLinkInterfacePtr mockLink1; + SharedLinkConfigurationPtr mockConfig2; + SharedLinkInterfacePtr mockLink2; + _startMockLink(1, false /*highLatency*/, false /*incrementVehicleId*/, mockConfig1, mockLink1); + _startMockLink(2, false /*highLatency*/, false /*incrementVehicleId*/, mockConfig2, mockLink2); + + Vehicle* const vehicle = waitForVehicleConnect(TestTimeout::shortMs()); + QVERIFY(vehicle); + QVERIFY_TRUE_WAIT(MultiVehicleManager::instance()->vehicles()->count() == 1, TestTimeout::mediumMs()); + + VehicleLinkManager* const vehicleLinkManager = vehicle->vehicleLinkManager(); + QVERIFY(vehicleLinkManager); + + QSignalSpy spyVehicleInitialConnectComplete(vehicle, &Vehicle::initialConnectComplete); + QVERIFY_TRUE_WAIT(spyVehicleInitialConnectComplete.count() > 0 || vehicle->isInitialConnectComplete(), + TestTimeout::mediumMs()); + + MockLink* const pMockLink1 = qobject_cast(mockLink1.get()); + MockLink* const pMockLink2 = qobject_cast(mockLink2.get()); + QVERIFY(pMockLink1); + QVERIFY(pMockLink2); + + QSignalSpy spyCommLostChanged(vehicleLinkManager, &VehicleLinkManager::communicationLostChanged); + + // Lose both links to reach total communication loss + pMockLink1->setCommLost(true); + pMockLink2->setCommLost(true); + QVERIFY_SIGNAL_WAIT(spyCommLostChanged, VehicleLinkManager::kTestCommLostDetectionTimeoutMs); + QCOMPARE(spyCommLostChanged.count(), 1); + QCOMPARE(spyCommLostChanged[0][0].toBool(), true); + spyCommLostChanged.clear(); + + // Regaining a single link should end total communication loss + pMockLink1->setCommLost(false); + QVERIFY_SIGNAL_WAIT(spyCommLostChanged, VehicleLinkManager::kTestCommLostDetectionTimeoutMs); + QCOMPARE(spyCommLostChanged.count(), 1); + QCOMPARE(spyCommLostChanged[0][0].toBool(), false); + + // Exactly one link should still be comm lost (link ordering is scheduling dependent) + const QStringList rgStatus = vehicleLinkManager->linkStatuses(); + QCOMPARE(rgStatus.count(), 2); + QCOMPARE(rgStatus.count(QString()), 1); +} + void VehicleLinkManagerTest::_connectionRemovedTest() { // Connection removal makes MavCommandQueue give up pending commands, same as the comm-loss tests. @@ -215,6 +272,7 @@ void VehicleLinkManagerTest::_connectionRemovedTest() SharedLinkInterfacePtr mockLink; _startMockLink(1, false /*highLatency*/, true /*incrementVehicleId*/, mockConfig, mockLink); MockLink* const pMockLink = qobject_cast(mockLink.get()); + QVERIFY(pMockLink); Vehicle* const vehicle = waitForVehicleConnect(TestTimeout::mediumMs()); QVERIFY(vehicle); @@ -249,6 +307,7 @@ void VehicleLinkManagerTest::_highLatencyLinkTest() SharedLinkInterfacePtr mockLink2; _startMockLink(1, true /*highLatency*/, false /*incrementVehicleId*/, mockConfig1, mockLink1); MockLink* const pMockLink1 = qobject_cast(mockLink1.get()); + QVERIFY(pMockLink1); Vehicle* const vehicle = waitForVehicleConnect(TestTimeout::mediumMs()); QVERIFY(vehicle); @@ -268,6 +327,7 @@ void VehicleLinkManagerTest::_highLatencyLinkTest() _startMockLink(2, false /*highLatency*/, false /*incrementVehicleId*/, mockConfig2, mockLink2); MockLink* pMockLink2 = qobject_cast(mockLink2.get()); + QVERIFY(pMockLink2); QCOMPARE(multiSpyVLM.waitForSignal(_primaryLinkChangedSignalName, TestTimeout::shortMs()), true); QCOMPARE(pMockLink2, vehicleLinkManager->primaryLink().lock().get()); diff --git a/test/Vehicle/VehicleLinkManagerTest.h b/test/Vehicle/VehicleLinkManagerTest.h index 0dc480a47287..231f11a7fe03 100644 --- a/test/Vehicle/VehicleLinkManagerTest.h +++ b/test/Vehicle/VehicleLinkManagerTest.h @@ -13,6 +13,7 @@ private slots: void _simpleLinkTest(); void _simpleCommLossTest(); void _multiLinkSingleVehicleTest(); + void _multiLinkTotalCommLossRecoveryTest(); void _connectionRemovedTest(); void _highLatencyLinkTest();