From b8a6f02c8dcf813358c4422483599d928ec61565 Mon Sep 17 00:00:00 2001 From: davidsastresas Date: Thu, 11 Jun 2026 20:47:34 +0200 Subject: [PATCH 01/23] Vehicle, Toolbar: update operator control to final MAVLink #2313 spec - Send the actual GCS sysid in param4 of MAV_CMD_REQUEST_OPERATOR_CONTROL (spec changed from #2158 to #2313; autopilot rejects param4=0), with optional param4/param5 range computed from configured secondary GCS ids - Gate MANUAL_CONTROL and RC override sends on operator control: block joystick output while another GCS is in control - Parse full CONTROL_STATUS (gcs_main + gcs_secondary[10]) and expose secondary GCS list to QML - Add release control support and secondary GCS range configuration UI to GCSControlIndicator - New persisted setting operatorControlSecondaryGCS (comma-separated secondary GCS sysids) --- src/Settings/FlyView.SettingsGroup.json | 7 ++ src/Settings/FlyViewSettings.cc | 1 + src/Settings/FlyViewSettings.h | 1 + src/Toolbar/GCSControlIndicator.qml | 79 +++++++++++++++++++- src/Vehicle/Vehicle.cc | 95 +++++++++++++++++++++---- src/Vehicle/Vehicle.h | 56 ++++++++------- 6 files changed, 199 insertions(+), 40 deletions(-) diff --git a/src/Settings/FlyView.SettingsGroup.json b/src/Settings/FlyView.SettingsGroup.json index fba0b87ef456..36855b64850c 100644 --- a/src/Settings/FlyView.SettingsGroup.json +++ b/src/Settings/FlyView.SettingsGroup.json @@ -136,6 +136,13 @@ "default": true, "label": "Enable automatic mission start/resume popups", "keywords": "mission popup" + }, + { + "name": "operatorControlSecondaryGCS", + "shortDesc": "Comma-separated list of GCS system IDs to include in the control range when requesting operator control. Empty means single-GCS mode.", + "type": "string", + "default": "", + "label": "Secondary GCS system IDs to include in control range" } ] } diff --git a/src/Settings/FlyViewSettings.cc b/src/Settings/FlyViewSettings.cc index f4f57c55fef9..c894109d29af 100644 --- a/src/Settings/FlyViewSettings.cc +++ b/src/Settings/FlyViewSettings.cc @@ -20,3 +20,4 @@ DECLARE_SETTINGSFACT(FlyViewSettings, instrumentQmlFile2) DECLARE_SETTINGSFACT(FlyViewSettings, requestControlAllowTakeover) DECLARE_SETTINGSFACT(FlyViewSettings, requestControlTimeout) DECLARE_SETTINGSFACT(FlyViewSettings, enableAutomaticMissionPopups) +DECLARE_SETTINGSFACT(FlyViewSettings, operatorControlSecondaryGCS) diff --git a/src/Settings/FlyViewSettings.h b/src/Settings/FlyViewSettings.h index fd6e02154fa1..2d38924bf92c 100644 --- a/src/Settings/FlyViewSettings.h +++ b/src/Settings/FlyViewSettings.h @@ -30,4 +30,5 @@ class FlyViewSettings : public SettingsGroup DEFINE_SETTINGFACT(requestControlAllowTakeover) DEFINE_SETTINGFACT(requestControlTimeout) DEFINE_SETTINGFACT(enableAutomaticMissionPopups) + DEFINE_SETTINGFACT(operatorControlSecondaryGCS) }; diff --git a/src/Toolbar/GCSControlIndicator.qml b/src/Toolbar/GCSControlIndicator.qml index 2a4be5cb1787..42bc9241f4a3 100644 --- a/src/Toolbar/GCSControlIndicator.qml +++ b/src/Toolbar/GCSControlIndicator.qml @@ -13,7 +13,8 @@ Item { property var activeVehicle: QGroundControl.multiVehicleManager.activeVehicle property bool showIndicator: activeVehicle && activeVehicle.firstControlStatusReceived - property var gcsMain: activeVehicle ? activeVehicle.gcsMain : 0 + property var sysidInControl: activeVehicle ? activeVehicle.sysidInControl : 0 + property var secondaryGCSList: activeVehicle ? activeVehicle.secondaryGCSList : [] property bool gcsControlStatusFlags_SystemManager: activeVehicle ? activeVehicle.gcsControlStatusFlags_SystemManager : false property bool gcsControlStatusFlags_TakeoverAllowed: activeVehicle ? activeVehicle.gcsControlStatusFlags_TakeoverAllowed : false property Fact requestControlAllowTakeoverFact: QGroundControl.settingsManager.flyViewSettings.requestControlAllowTakeover @@ -21,6 +22,10 @@ Item { property bool isThisGCSinControl: gcsMain == QGroundControl.settingsManager.mavlinkSettings.gcsMavlinkSystemID.rawValue property bool sendControlRequestAllowed: activeVehicle ? activeVehicle.sendControlRequestAllowed : false + property Fact secondaryGCSSettingFact: QGroundControl.settingsManager.flyViewSettings.operatorControlSecondaryGCS + property string secondaryGCSSetting: secondaryGCSSettingFact.rawValue + property bool hasConfiguredSecondaryGCS: secondaryGCSSetting.length > 0 + property var margins: ScreenTools.defaultFontPixelWidth property var panelRadius: ScreenTools.defaultFontPixelWidth * 0.5 property var buttonHeight: height * 1.6 @@ -250,6 +255,11 @@ Item { horizontalAlignment: Text.AlignRight color: gcsControlStatusFlags_TakeoverAllowed ? qgcPal.colorGreen : qgcPal.text } + QGCLabel { + text: qsTr("Secondary GCS: ") + secondaryGCSList.join(", ") + Layout.columnSpan: 2 + visible: secondaryGCSList.length > 0 + } // Separator Rectangle { Layout.columnSpan: 2 @@ -280,12 +290,11 @@ Item { enabled: gcsControlStatusFlags_TakeoverAllowed || isThisGCSinControl } QGCButton { - text: gcsControlStatusFlags_TakeoverAllowed ? qsTr("Adquire Control") : qsTr("Send Request") + text: gcsControlStatusFlags_TakeoverAllowed ? qsTr("Acquire Control") : qsTr("Send Request") onClicked: { var timeout = gcsControlStatusFlags_TakeoverAllowed ? 0 : QGroundControl.settingsManager.flyViewSettings.requestControlTimeout.rawValue control.activeVehicle.requestOperatorControl(requestControlAllowTakeoverFact.rawValue, timeout) if (timeout > 0) { - // Start UI timeout animation startProgressTracker(timeout) } } @@ -310,6 +319,15 @@ Item { Layout.alignment: Qt.AlignRight enabled: gcsControlStatusFlags_TakeoverAllowed != requestControlAllowTakeoverFact.rawValue } + QGCButton { + text: qsTr("Release Control") + onClicked: { + control.activeVehicle.releaseOperatorControl() + } + Layout.columnSpan: 2 + Layout.alignment: Qt.AlignRight + visible: isThisGCSinControl + } // Separator Rectangle { Layout.columnSpan: 2 @@ -324,6 +342,61 @@ Item { label: qsTr("This GCS Mavlink System ID: ") fact: QGroundControl.settingsManager.mavlinkSettings.gcsMavlinkSystemID } + // Separator + Rectangle { + Layout.columnSpan: 2 + Layout.preferredWidth: parent.width + Layout.alignment: Qt.AlignHCenter + color: qgcPal.windowShade + height: outdoorPalette ? 1 : 2 + } + // Compact hint when secondary GCS are configured (collapsed) + QGCLabel { + property bool expanded: false + id: rangeSettingsToggle + text: hasConfiguredSecondaryGCS + ? qsTr("Control range configured (click to edit)") + : qsTr("Configure secondary GCS range") + Layout.columnSpan: 2 + color: hasConfiguredSecondaryGCS ? qgcPal.buttonHighlight : qgcPal.text + font.underline: true + MouseArea { + anchors.fill: parent + cursorShape: Qt.PointingHandCursor + onClicked: rangeSettingsToggle.expanded = !rangeSettingsToggle.expanded + } + } + // Expanded settings + QGCLabel { + text: qsTr("Secondary GCS IDs (comma-separated):") + Layout.columnSpan: 2 + visible: rangeSettingsToggle.expanded + } + FactTextField { + fact: secondaryGCSSettingFact + Layout.columnSpan: 2 + Layout.fillWidth: true + visible: rangeSettingsToggle.expanded + } + QGCLabel { + visible: rangeSettingsToggle.expanded && hasConfiguredSecondaryGCS + Layout.columnSpan: 2 + color: qgcPal.buttonHighlight + text: { + var myId = QGroundControl.settingsManager.mavlinkSettings.gcsMavlinkSystemID.rawValue + var parts = secondaryGCSSetting.split(",") + var lo = myId + var hi = myId + for (var i = 0; i < parts.length; i++) { + var val = parseInt(parts[i].trim()) + if (!isNaN(val) && val >= 1 && val <= 255) { + if (val < lo) lo = val + if (val > hi) hi = val + } + } + return qsTr("Request range: ") + lo + " - " + hi + } + } } } } diff --git a/src/Vehicle/Vehicle.cc b/src/Vehicle/Vehicle.cc index c19094d300b4..a4467ee8dbea 100644 --- a/src/Vehicle/Vehicle.cc +++ b/src/Vehicle/Vehicle.cc @@ -2977,6 +2977,10 @@ void Vehicle::clearAllParamMapRC(void) void Vehicle::sendJoystickDataThreadSafe(float roll, float pitch, float yaw, float thrust, quint16 buttons, quint16 buttons2, float pitchExtension, float rollExtension, float aux1, float aux2, float aux3, float aux4, float aux5, float aux6) { + if (!_joystickSendAllowed.load(std::memory_order_relaxed)) { + return; + } + SharedLinkInterfacePtr sharedLink = vehicleLinkManager()->primaryLink().lock(); if (!sharedLink) { qCDebug(VehicleLog)<< "sendJoystickDataThreadSafe: primary link gone!"; @@ -3037,6 +3041,10 @@ void Vehicle::sendJoystickDataThreadSafe(float roll, float pitch, float yaw, flo // Channels 1–4 (attitude axes) always carry UINT16_MAX (ignore) and channels 11–18 are unused. void Vehicle::sendJoystickAuxRcOverrideThreadSafe(const std::array &channelValues, const std::array &channelEnabled, bool useRcOverride) { + if (!_joystickSendAllowed.load(std::memory_order_relaxed)) { + return; + } + SharedLinkInterfacePtr sharedLink = vehicleLinkManager()->primaryLink().lock(); if (!sharedLink) { qCDebug(VehicleLog) << "sendJoystickAuxRcOverrideThreadSafe: primary link gone!"; @@ -3189,6 +3197,34 @@ void Vehicle::startTimerRevertAllowTakeover() _timerRevertAllowTakeover.start(); } +void Vehicle::_computeOperatorControlRange(uint8_t &rangeLow, uint8_t &rangeHigh) const +{ + const uint8_t myId = static_cast(MAVLinkProtocol::instance()->getSystemId()); + const QString secondaryStr = SettingsManager::instance()->flyViewSettings()->operatorControlSecondaryGCS()->rawValue().toString().trimmed(); + + if (secondaryStr.isEmpty()) { + rangeLow = myId; + rangeHigh = 0; + return; + } + + uint8_t lo = myId; + uint8_t hi = myId; + const QStringList parts = secondaryStr.split(',', Qt::SkipEmptyParts); + for (const QString &part : parts) { + bool ok = false; + const int val = part.trimmed().toInt(&ok); + if (ok && val >= 1 && val <= 255) { + const uint8_t id = static_cast(val); + if (id < lo) lo = id; + if (id > hi) hi = id; + } + } + + rangeLow = lo; + rangeHigh = (hi > lo) ? hi : 0; +} + void Vehicle::requestOperatorControl(bool allowOverride, int requestTimeoutSecs) { int safeRequestTimeoutSecs; @@ -3197,27 +3233,44 @@ void Vehicle::requestOperatorControl(bool allowOverride, int requestTimeoutSecs) if (requestTimeoutSecs >= requestTimeoutSecsMin && requestTimeoutSecs <= requestTimeoutSecsMax) { safeRequestTimeoutSecs = requestTimeoutSecs; } else { - // If out of limits use default value safeRequestTimeoutSecs = SettingsManager::instance()->flyViewSettings()->requestControlTimeout()->cookedDefaultValue().toInt(); } + uint8_t rangeLow, rangeHigh; + _computeOperatorControlRange(rangeLow, rangeHigh); + const MavCmdAckHandlerInfo_t handlerInfo = {&Vehicle::_requestOperatorControlAckHandler, this, nullptr, nullptr}; sendMavCommandWithHandler( &handlerInfo, _defaultComponentId, MAV_CMD_REQUEST_OPERATOR_CONTROL, - 0, // System ID of GCS requesting control, 0 if it is this GCS - 1, // Action - 0: Release control, 1: Request control. - allowOverride ? 1 : 0, // Allow takeover - Enable automatic granting of ownership on request. 0: Ask current owner and reject request, 1: Allow automatic takeover. - safeRequestTimeoutSecs // Timeout in seconds before a request to a GCS to allow takeover is assumed to be rejected. This is used to display the timeout graphically on requestor and GCS in control. + 1, // param1: Action - 1: Request control + allowOverride ? 1.0f : 0.0f, // param2: Allow takeover + static_cast(safeRequestTimeoutSecs), // param3: Timeout in seconds + static_cast(rangeLow), // param4: GCS sysid (range low) + static_cast(rangeHigh) // param5: GCS sysid upper range (0 = single GCS) ); - // If this is a request we sent to other GCS, start timer so User can not keep sending requests until the current timeout expires if (requestTimeoutSecs > 0) { requestOperatorControlStartTimer(requestTimeoutSecs * 1000); } } +void Vehicle::releaseOperatorControl() +{ + const MavCmdAckHandlerInfo_t handlerInfo = {&Vehicle::_requestOperatorControlAckHandler, this, nullptr, nullptr}; + sendMavCommandWithHandler( + &handlerInfo, + _defaultComponentId, + MAV_CMD_REQUEST_OPERATOR_CONTROL, + 0, // param1: Action - 0: Release control + 0, // param2: Allow takeover (irrelevant for release) + 0, // param3: Timeout (irrelevant for release) + static_cast(MAVLinkProtocol::instance()->getSystemId()), // param4: GCS sysid + 0 // param5: GCS sysid upper range (0 = single GCS) + ); +} + void Vehicle::_requestOperatorControlAckHandler(void* resultHandlerData, int compId, const mavlink_command_ack_t& ack, MavCmdResultFailureCode_t failureCode) { // For the moment, this will always come from an autopilot, compid 1 @@ -3278,8 +3331,22 @@ void Vehicle::_handleControlStatus(const mavlink_message_t& message) updateControlStatusSignals = true; } - if (_gcsMain != controlStatus.gcs_main) { - _gcsMain = controlStatus.gcs_main; + if (_sysid_in_control != controlStatus.gcs_main) { + _sysid_in_control = controlStatus.gcs_main; + const uint8_t myId = static_cast(MAVLinkProtocol::instance()->getSystemId()); + _joystickSendAllowed.store(_sysid_in_control == 0 || _sysid_in_control == myId, + std::memory_order_relaxed); + updateControlStatusSignals = true; + } + + QList newSecondaryList; + for (int i = 0; i < 10; i++) { + if (controlStatus.gcs_secondary[i] != 0) { + newSecondaryList.append(controlStatus.gcs_secondary[i]); + } + } + if (_secondaryGCSList != newSecondaryList) { + _secondaryGCSList = newSecondaryList; updateControlStatusSignals = true; } @@ -3292,9 +3359,9 @@ void Vehicle::_handleControlStatus(const mavlink_message_t& message) emit gcsControlStatusChanged(); } - // If we were waiting for a request to be accepted and now it was accepted, adjust flags accordingly so - // UI unlocks the request/take control button - if (!sendControlRequestAllowed() && _gcsControlStatusFlags_TakeoverAllowed) { + const uint8_t myId = static_cast(MAVLinkProtocol::instance()->getSystemId()); + if (!sendControlRequestAllowed() && (_sysid_in_control == myId || _gcsControlStatusFlags_TakeoverAllowed)) { + _timerRequestOperatorControl.stop(); disconnect(&_timerRequestOperatorControl, &QTimer::timeout, nullptr, nullptr); _sendControlRequestAllowed = true; emit sendControlRequestAllowedChanged(true); @@ -3303,7 +3370,11 @@ void Vehicle::_handleControlStatus(const mavlink_message_t& message) void Vehicle::_handleCommandRequestOperatorControl(const mavlink_command_long_t commandLong) { - emit requestOperatorControlReceived(commandLong.param1, commandLong.param3, commandLong.param4); + emit requestOperatorControlReceived( + static_cast(commandLong.param4), // GCS sysid requesting control + static_cast(commandLong.param2), // Allow takeover + static_cast(commandLong.param3) // Request timeout in seconds + ); } void Vehicle::_handleCommandLong(const mavlink_message_t& message) diff --git a/src/Vehicle/Vehicle.h b/src/Vehicle/Vehicle.h index 2986f7f9ebf7..ee2c8d149df0 100644 --- a/src/Vehicle/Vehicle.h +++ b/src/Vehicle/Vehicle.h @@ -947,6 +947,7 @@ private slots: bool _allSensorsHealthy = true; VehicleSigningController* _signingController = nullptr; std::atomic _joystickAuxRcOverrideActive = false; + std::atomic _joystickSendAllowed = true; std::unique_ptr _sysStatusSensorInfo; @@ -1151,37 +1152,42 @@ private slots: public: Q_INVOKABLE void startTimerRevertAllowTakeover(); Q_INVOKABLE void requestOperatorControl(bool allowOverride, int requestTimeoutSecs = 0); + Q_INVOKABLE void releaseOperatorControl(); private: void _handleControlStatus(const mavlink_message_t& message); void _handleCommandRequestOperatorControl(const mavlink_command_long_t commandLong); static void _requestOperatorControlAckHandler(void* resultHandlerData, int compId, const mavlink_command_ack_t& ack, MavCmdResultFailureCode_t failureCode); - Q_PROPERTY(uint8_t gcsMain READ gcsMain NOTIFY gcsControlStatusChanged) - Q_PROPERTY(bool gcsControlStatusFlags_SystemManager READ gcsControlStatusFlags_SystemManager NOTIFY gcsControlStatusChanged) - Q_PROPERTY(bool gcsControlStatusFlags_TakeoverAllowed READ gcsControlStatusFlags_TakeoverAllowed NOTIFY gcsControlStatusChanged) - Q_PROPERTY(bool firstControlStatusReceived READ firstControlStatusReceived NOTIFY gcsControlStatusChanged) - Q_PROPERTY(int operatorControlTakeoverTimeoutMsecs READ operatorControlTakeoverTimeoutMsecs CONSTANT) - Q_PROPERTY(int requestOperatorControlRemainingMsecs READ requestOperatorControlRemainingMsecs CONSTANT) - Q_PROPERTY(bool sendControlRequestAllowed READ sendControlRequestAllowed NOTIFY sendControlRequestAllowedChanged) - - uint8_t gcsMain() const { return _gcsMain; } - bool gcsControlStatusFlags_SystemManager() const { return _gcsControlStatusFlags_SystemManager; } - bool gcsControlStatusFlags_TakeoverAllowed() const { return _gcsControlStatusFlags_TakeoverAllowed; } - bool firstControlStatusReceived() const { return _firstControlStatusReceived; } - int operatorControlTakeoverTimeoutMsecs() const; - int requestOperatorControlRemainingMsecs() const { return _timerRequestOperatorControl.remainingTime(); } - bool sendControlRequestAllowed() const { return _sendControlRequestAllowed; } - void requestOperatorControlStartTimer(int requestTimeoutMsecs); - - uint8_t _gcsMain = 0; - uint8_t _gcsControlStatusFlags = 0; - bool _gcsControlStatusFlags_SystemManager = 0; - bool _gcsControlStatusFlags_TakeoverAllowed = 0; - bool _firstControlStatusReceived = false; - QTimer _timerRevertAllowTakeover; - QTimer _timerRequestOperatorControl; - bool _sendControlRequestAllowed = true; + Q_PROPERTY(uint8_t sysidInControl READ sysidInControl NOTIFY gcsControlStatusChanged) + Q_PROPERTY(QList secondaryGCSList READ secondaryGCSList NOTIFY gcsControlStatusChanged) + Q_PROPERTY(bool gcsControlStatusFlags_SystemManager READ gcsControlStatusFlags_SystemManager NOTIFY gcsControlStatusChanged) + Q_PROPERTY(bool gcsControlStatusFlags_TakeoverAllowed READ gcsControlStatusFlags_TakeoverAllowed NOTIFY gcsControlStatusChanged) + Q_PROPERTY(bool firstControlStatusReceived READ firstControlStatusReceived NOTIFY gcsControlStatusChanged) + Q_PROPERTY(int operatorControlTakeoverTimeoutMsecs READ operatorControlTakeoverTimeoutMsecs CONSTANT) + Q_PROPERTY(int requestOperatorControlRemainingMsecs READ requestOperatorControlRemainingMsecs CONSTANT) + Q_PROPERTY(bool sendControlRequestAllowed READ sendControlRequestAllowed NOTIFY sendControlRequestAllowedChanged) + + uint8_t sysidInControl() const { return _sysid_in_control; } + QList secondaryGCSList() const { return _secondaryGCSList; } + bool gcsControlStatusFlags_SystemManager() const { return _gcsControlStatusFlags_SystemManager; } + bool gcsControlStatusFlags_TakeoverAllowed() const { return _gcsControlStatusFlags_TakeoverAllowed; } + bool firstControlStatusReceived() const { return _firstControlStatusReceived; } + int operatorControlTakeoverTimeoutMsecs() const; + int requestOperatorControlRemainingMsecs() const { return _timerRequestOperatorControl.remainingTime(); } + bool sendControlRequestAllowed() const { return _sendControlRequestAllowed; } + void requestOperatorControlStartTimer(int requestTimeoutMsecs); + void _computeOperatorControlRange(uint8_t &rangeLow, uint8_t &rangeHigh) const; + + uint8_t _sysid_in_control = 0; + QList _secondaryGCSList; + uint8_t _gcsControlStatusFlags = 0; + bool _gcsControlStatusFlags_SystemManager = 0; + bool _gcsControlStatusFlags_TakeoverAllowed = 0; + bool _firstControlStatusReceived = false; + QTimer _timerRevertAllowTakeover; + QTimer _timerRequestOperatorControl; + bool _sendControlRequestAllowed = true; signals: void gcsControlStatusChanged(); From 71b405ff3a6d7166360263a68609fb69da7dc68b Mon Sep 17 00:00:00 2001 From: davidsastresas Date: Thu, 11 Jun 2026 20:48:42 +0200 Subject: [PATCH 02/23] Vehicle, Toolbar: fix operator control request countdown lifecycle The request countdown could keep running (or restart from a stale value) after the request was already resolved: - The stop check in _handleControlStatus() missed the uncontrolled case: when the owning GCS releases control, CONTROL_STATUS arrives with gcs_main=0 and takeover not allowed, so neither stop condition matched. Add _sysid_in_control == 0 to the check. - The QML progress tracker only stopped on the takeover-allowed transition. Add watchers so it also stops when this GCS gains control or when the C++ side re-allows sending requests. - requestOperatorControlRemainingMsecs was declared CONSTANT but wraps QTimer::remainingTime(), so reopening the popup restored the countdown from a stale cached value. Tie it to sendControlRequestAllowedChanged instead. - releaseOperatorControl() left the request and takeover-revert timers running, firing stale signals after release. Stop them first. - A pending takeover revert also kept running when control moved to another GCS. Stop it on control loss in _handleControlStatus(). --- src/Toolbar/GCSControlIndicator.qml | 14 ++++++++++++++ src/Vehicle/Vehicle.cc | 15 ++++++++++++++- src/Vehicle/Vehicle.h | 2 +- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/Toolbar/GCSControlIndicator.qml b/src/Toolbar/GCSControlIndicator.qml index 42bc9241f4a3..8a4b29b19520 100644 --- a/src/Toolbar/GCSControlIndicator.qml +++ b/src/Toolbar/GCSControlIndicator.qml @@ -216,6 +216,20 @@ Item { sendRequestProgressTracker.stop() } } + // Also stop it if we gained control, or the vehicle became uncontrolled/the request expired, + // which the C++ side reports through sendControlRequestAllowed + property bool isThisGCSinControlLocal: control.isThisGCSinControl + onIsThisGCSinControlLocalChanged: { + if (isThisGCSinControlLocal && sendRequestProgressTracker.running) { + sendRequestProgressTracker.stop() + } + } + property bool sendControlRequestAllowedLocal: control.sendControlRequestAllowed + onSendControlRequestAllowedLocalChanged: { + if (sendControlRequestAllowedLocal && sendRequestProgressTracker.running) { + sendRequestProgressTracker.stop() + } + } Component.onCompleted: { // If send control request is not allowed it means we recently sent a request, closed the popup, and opened again diff --git a/src/Vehicle/Vehicle.cc b/src/Vehicle/Vehicle.cc index a4467ee8dbea..8e2eb0541aff 100644 --- a/src/Vehicle/Vehicle.cc +++ b/src/Vehicle/Vehicle.cc @@ -3258,6 +3258,15 @@ void Vehicle::requestOperatorControl(bool allowOverride, int requestTimeoutSecs) void Vehicle::releaseOperatorControl() { + // Releasing control makes any pending request countdown or takeover revert meaningless + _timerRevertAllowTakeover.stop(); + _timerRequestOperatorControl.stop(); + disconnect(&_timerRequestOperatorControl, &QTimer::timeout, nullptr, nullptr); + if (!_sendControlRequestAllowed) { + _sendControlRequestAllowed = true; + emit sendControlRequestAllowedChanged(true); + } + const MavCmdAckHandlerInfo_t handlerInfo = {&Vehicle::_requestOperatorControlAckHandler, this, nullptr, nullptr}; sendMavCommandWithHandler( &handlerInfo, @@ -3336,6 +3345,10 @@ void Vehicle::_handleControlStatus(const mavlink_message_t& message) const uint8_t myId = static_cast(MAVLinkProtocol::instance()->getSystemId()); _joystickSendAllowed.store(_sysid_in_control == 0 || _sysid_in_control == myId, std::memory_order_relaxed); + if (_sysid_in_control != myId) { + // Control moved away from this GCS, so a pending revert to takeover not allowed no longer applies + _timerRevertAllowTakeover.stop(); + } updateControlStatusSignals = true; } @@ -3360,7 +3373,7 @@ void Vehicle::_handleControlStatus(const mavlink_message_t& message) } const uint8_t myId = static_cast(MAVLinkProtocol::instance()->getSystemId()); - if (!sendControlRequestAllowed() && (_sysid_in_control == myId || _gcsControlStatusFlags_TakeoverAllowed)) { + if (!sendControlRequestAllowed() && (_sysid_in_control == myId || _sysid_in_control == 0 || _gcsControlStatusFlags_TakeoverAllowed)) { _timerRequestOperatorControl.stop(); disconnect(&_timerRequestOperatorControl, &QTimer::timeout, nullptr, nullptr); _sendControlRequestAllowed = true; diff --git a/src/Vehicle/Vehicle.h b/src/Vehicle/Vehicle.h index ee2c8d149df0..b6d514d6715e 100644 --- a/src/Vehicle/Vehicle.h +++ b/src/Vehicle/Vehicle.h @@ -1165,7 +1165,7 @@ private slots: Q_PROPERTY(bool gcsControlStatusFlags_TakeoverAllowed READ gcsControlStatusFlags_TakeoverAllowed NOTIFY gcsControlStatusChanged) Q_PROPERTY(bool firstControlStatusReceived READ firstControlStatusReceived NOTIFY gcsControlStatusChanged) Q_PROPERTY(int operatorControlTakeoverTimeoutMsecs READ operatorControlTakeoverTimeoutMsecs CONSTANT) - Q_PROPERTY(int requestOperatorControlRemainingMsecs READ requestOperatorControlRemainingMsecs CONSTANT) + Q_PROPERTY(int requestOperatorControlRemainingMsecs READ requestOperatorControlRemainingMsecs NOTIFY sendControlRequestAllowedChanged) Q_PROPERTY(bool sendControlRequestAllowed READ sendControlRequestAllowed NOTIFY sendControlRequestAllowedChanged) uint8_t sysidInControl() const { return _sysid_in_control; } From 3b0ac5d6369549822c37dfdf9b5be8069790f95f Mon Sep 17 00:00:00 2001 From: davidsastresas Date: Thu, 11 Jun 2026 20:49:17 +0200 Subject: [PATCH 03/23] Vehicle: ack operator control takeover notification When takeover is not allowed, the autopilot forwards MAV_CMD_REQUEST_OPERATOR_CONTROL to the GCS in control as a notification. QGC showed the popup but never sent COMMAND_ACK, leaving the autopilot notification pending and the command unacknowledged at the MAVLink level. Reply with MAV_RESULT_ACCEPTED to the sender. --- src/Vehicle/Vehicle.cc | 22 ++++++++++++++++++++-- src/Vehicle/Vehicle.h | 2 +- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/Vehicle/Vehicle.cc b/src/Vehicle/Vehicle.cc index 8e2eb0541aff..e4f09b357ea5 100644 --- a/src/Vehicle/Vehicle.cc +++ b/src/Vehicle/Vehicle.cc @@ -3381,8 +3381,26 @@ void Vehicle::_handleControlStatus(const mavlink_message_t& message) } } -void Vehicle::_handleCommandRequestOperatorControl(const mavlink_command_long_t commandLong) +void Vehicle::_handleCommandRequestOperatorControl(const mavlink_message_t& message, const mavlink_command_long_t& commandLong) { + // Acknowledge the takeover notification so the autopilot can clear its pending notification state + SharedLinkInterfacePtr sharedLink = vehicleLinkManager()->primaryLink().lock(); + if (sharedLink) { + mavlink_message_t ackMessage{}; + (void) mavlink_msg_command_ack_pack_chan( + MAVLinkProtocol::instance()->getSystemId(), + MAVLinkProtocol::getComponentId(), + sharedLink->mavlinkChannel(), + &ackMessage, + MAV_CMD_REQUEST_OPERATOR_CONTROL, + MAV_RESULT_ACCEPTED, + 0, // progress + 0, // result_param2 + message.sysid, + message.compid); + (void) sendMessageOnLinkThreadSafe(sharedLink.get(), ackMessage); + } + emit requestOperatorControlReceived( static_cast(commandLong.param4), // GCS sysid requesting control static_cast(commandLong.param2), // Allow takeover @@ -3399,7 +3417,7 @@ void Vehicle::_handleCommandLong(const mavlink_message_t& message) return; } if (commandLong.command == MAV_CMD_REQUEST_OPERATOR_CONTROL) { - _handleCommandRequestOperatorControl(commandLong); + _handleCommandRequestOperatorControl(message, commandLong); } } diff --git a/src/Vehicle/Vehicle.h b/src/Vehicle/Vehicle.h index b6d514d6715e..03500d5080c1 100644 --- a/src/Vehicle/Vehicle.h +++ b/src/Vehicle/Vehicle.h @@ -1156,7 +1156,7 @@ private slots: private: void _handleControlStatus(const mavlink_message_t& message); - void _handleCommandRequestOperatorControl(const mavlink_command_long_t commandLong); + void _handleCommandRequestOperatorControl(const mavlink_message_t& message, const mavlink_command_long_t& commandLong); static void _requestOperatorControlAckHandler(void* resultHandlerData, int compId, const mavlink_command_ack_t& ack, MavCmdResultFailureCode_t failureCode); Q_PROPERTY(uint8_t sysidInControl READ sysidInControl NOTIFY gcsControlStatusChanged) From 1ef74aab5654d85686494bdccac620b83060156b Mon Sep 17 00:00:00 2001 From: davidsastresas Date: Fri, 12 Jun 2026 18:13:03 +0200 Subject: [PATCH 04/23] Vehicle: address operator control commands to the system manager component The spec says a GCS should monitor for CONTROL_STATUS with the GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER flag and address MAV_CMD_REQUEST_OPERATOR_CONTROL to that component, which is not necessarily the autopilot. Learn the component id from CONTROL_STATUS and use it as the command target, falling back to the default component id until the first CONTROL_STATUS is seen. --- src/Vehicle/Vehicle.cc | 10 ++++++++-- src/Vehicle/Vehicle.h | 1 + 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/Vehicle/Vehicle.cc b/src/Vehicle/Vehicle.cc index e4f09b357ea5..927ddc61ad04 100644 --- a/src/Vehicle/Vehicle.cc +++ b/src/Vehicle/Vehicle.cc @@ -3242,7 +3242,7 @@ void Vehicle::requestOperatorControl(bool allowOverride, int requestTimeoutSecs) const MavCmdAckHandlerInfo_t handlerInfo = {&Vehicle::_requestOperatorControlAckHandler, this, nullptr, nullptr}; sendMavCommandWithHandler( &handlerInfo, - _defaultComponentId, + (_operatorControlCompId != 0) ? _operatorControlCompId : _defaultComponentId, MAV_CMD_REQUEST_OPERATOR_CONTROL, 1, // param1: Action - 1: Request control allowOverride ? 1.0f : 0.0f, // param2: Allow takeover @@ -3270,7 +3270,7 @@ void Vehicle::releaseOperatorControl() const MavCmdAckHandlerInfo_t handlerInfo = {&Vehicle::_requestOperatorControlAckHandler, this, nullptr, nullptr}; sendMavCommandWithHandler( &handlerInfo, - _defaultComponentId, + (_operatorControlCompId != 0) ? _operatorControlCompId : _defaultComponentId, MAV_CMD_REQUEST_OPERATOR_CONTROL, 0, // param1: Action - 0: Release control 0, // param2: Allow takeover (irrelevant for release) @@ -3332,6 +3332,12 @@ void Vehicle::_handleControlStatus(const mavlink_message_t& message) mavlink_control_status_t controlStatus; mavlink_msg_control_status_decode(&message, &controlStatus); + if (controlStatus.flags & GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER) { + // This component manages GCS control of the whole system. Operator control + // commands must be addressed to it, which is not necessarily the autopilot + _operatorControlCompId = message.compid; + } + bool updateControlStatusSignals = false; if (_gcsControlStatusFlags != controlStatus.flags) { _gcsControlStatusFlags = controlStatus.flags; diff --git a/src/Vehicle/Vehicle.h b/src/Vehicle/Vehicle.h index 03500d5080c1..5f4fae9b02d2 100644 --- a/src/Vehicle/Vehicle.h +++ b/src/Vehicle/Vehicle.h @@ -1180,6 +1180,7 @@ private slots: void _computeOperatorControlRange(uint8_t &rangeLow, uint8_t &rangeHigh) const; uint8_t _sysid_in_control = 0; + uint8_t _operatorControlCompId = 0; // compid of the system manager component, learned from CONTROL_STATUS QList _secondaryGCSList; uint8_t _gcsControlStatusFlags = 0; bool _gcsControlStatusFlags_SystemManager = 0; From b2bc953edfe289564245b99225e4cd8c99945c3a Mon Sep 17 00:00:00 2001 From: davidsastresas Date: Fri, 12 Jun 2026 18:13:26 +0200 Subject: [PATCH 05/23] Vehicle: use acquire/release ordering for joystick operator control gate _joystickSendAllowed was read and written with memory_order_relaxed. On weakly ordered architectures the store could in principle be observed by the joystick thread after other effects of the CONTROL_STATUS handling. Use release on the store and acquire on the loads so the gate change is properly ordered with the control status update that caused it. --- src/Vehicle/Vehicle.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Vehicle/Vehicle.cc b/src/Vehicle/Vehicle.cc index 927ddc61ad04..aee09a917858 100644 --- a/src/Vehicle/Vehicle.cc +++ b/src/Vehicle/Vehicle.cc @@ -2977,7 +2977,7 @@ void Vehicle::clearAllParamMapRC(void) void Vehicle::sendJoystickDataThreadSafe(float roll, float pitch, float yaw, float thrust, quint16 buttons, quint16 buttons2, float pitchExtension, float rollExtension, float aux1, float aux2, float aux3, float aux4, float aux5, float aux6) { - if (!_joystickSendAllowed.load(std::memory_order_relaxed)) { + if (!_joystickSendAllowed.load(std::memory_order_acquire)) { return; } @@ -3041,7 +3041,7 @@ void Vehicle::sendJoystickDataThreadSafe(float roll, float pitch, float yaw, flo // Channels 1–4 (attitude axes) always carry UINT16_MAX (ignore) and channels 11–18 are unused. void Vehicle::sendJoystickAuxRcOverrideThreadSafe(const std::array &channelValues, const std::array &channelEnabled, bool useRcOverride) { - if (!_joystickSendAllowed.load(std::memory_order_relaxed)) { + if (!_joystickSendAllowed.load(std::memory_order_acquire)) { return; } @@ -3350,7 +3350,7 @@ void Vehicle::_handleControlStatus(const mavlink_message_t& message) _sysid_in_control = controlStatus.gcs_main; const uint8_t myId = static_cast(MAVLinkProtocol::instance()->getSystemId()); _joystickSendAllowed.store(_sysid_in_control == 0 || _sysid_in_control == myId, - std::memory_order_relaxed); + std::memory_order_release); if (_sysid_in_control != myId) { // Control moved away from this GCS, so a pending revert to takeover not allowed no longer applies _timerRevertAllowTakeover.stop(); From 20ba03e2489285abe4d44870143e5bd9733e5375 Mon Sep 17 00:00:00 2001 From: davidsastresas Date: Fri, 12 Jun 2026 18:13:40 +0200 Subject: [PATCH 06/23] Toolbar: warn when secondary GCS range covers unconfigured ids The operator control request encodes secondary GCS as a contiguous sysid range, so non-contiguous configured ids (e.g. 200, 254) silently grant control to every id in between. Show a warning in the range configuration panel with the number of unconfigured ids the computed range would accept. --- src/Toolbar/GCSControlIndicator.qml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/Toolbar/GCSControlIndicator.qml b/src/Toolbar/GCSControlIndicator.qml index 8a4b29b19520..3ede3aab330b 100644 --- a/src/Toolbar/GCSControlIndicator.qml +++ b/src/Toolbar/GCSControlIndicator.qml @@ -393,12 +393,17 @@ Item { visible: rangeSettingsToggle.expanded } QGCLabel { + id: rangeSummaryLabel visible: rangeSettingsToggle.expanded && hasConfiguredSecondaryGCS Layout.columnSpan: 2 color: qgcPal.buttonHighlight + // Number of sysids inside the computed range which are neither this GCS nor a configured secondary. + // The protocol encodes the request as a contiguous range, so these would be granted control too + property int unconfiguredIdsInRange: 0 text: { var myId = QGroundControl.settingsManager.mavlinkSettings.gcsMavlinkSystemID.rawValue var parts = secondaryGCSSetting.split(",") + var ids = [ myId ] var lo = myId var hi = myId for (var i = 0; i < parts.length; i++) { @@ -406,11 +411,21 @@ Item { if (!isNaN(val) && val >= 1 && val <= 255) { if (val < lo) lo = val if (val > hi) hi = val + if (ids.indexOf(val) < 0) ids.push(val) } } + rangeSummaryLabel.unconfiguredIdsInRange = (hi - lo + 1) - ids.length return qsTr("Request range: ") + lo + " - " + hi } } + QGCLabel { + visible: rangeSummaryLabel.visible && rangeSummaryLabel.unconfiguredIdsInRange > 0 + Layout.columnSpan: 2 + Layout.fillWidth: true + wrapMode: Text.WordWrap + color: qgcPal.colorOrange + text: qsTr("Warning: %1 other GCS id(s) inside this range will also be accepted as operators").arg(rangeSummaryLabel.unconfiguredIdsInRange) + } } } } From 65f500e9fa1a9936d41a1edb82b52766d033e2aa Mon Sep 17 00:00:00 2001 From: davidsastresas Date: Sun, 14 Jun 2026 13:45:14 +0200 Subject: [PATCH 07/23] GCSControlIndicator.qml: fix labels when no one in control and takeover is permited --- src/Toolbar/GCSControlIndicator.qml | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/Toolbar/GCSControlIndicator.qml b/src/Toolbar/GCSControlIndicator.qml index 3ede3aab330b..fe355d7325c9 100644 --- a/src/Toolbar/GCSControlIndicator.qml +++ b/src/Toolbar/GCSControlIndicator.qml @@ -21,8 +21,11 @@ Item { property bool requestControlAllowTakeover: requestControlAllowTakeoverFact.rawValue property bool isThisGCSinControl: gcsMain == QGroundControl.settingsManager.mavlinkSettings.gcsMavlinkSystemID.rawValue property bool sendControlRequestAllowed: activeVehicle ? activeVehicle.sendControlRequestAllowed : false + // When nobody is in control (uncontrolled) or takeover is allowed, the autopilot grants control + // immediately, so there is no owner to ask and no request countdown + property bool controlGrantedImmediately: sysidInControl == 0 || gcsControlStatusFlags_TakeoverAllowed - property Fact secondaryGCSSettingFact: QGroundControl.settingsManager.flyViewSettings.operatorControlSecondaryGCS + property Fact secondaryGCSSettingFact: QGroundControl.settingsManager.flyViewSettings.operatorControlSecondaryGCS property string secondaryGCSSetting: secondaryGCSSettingFact.rawValue property bool hasConfiguredSecondaryGCS: secondaryGCSSetting.length > 0 @@ -304,9 +307,9 @@ Item { enabled: gcsControlStatusFlags_TakeoverAllowed || isThisGCSinControl } QGCButton { - text: gcsControlStatusFlags_TakeoverAllowed ? qsTr("Acquire Control") : qsTr("Send Request") + text: controlGrantedImmediately ? qsTr("Acquire Control") : qsTr("Send Request") onClicked: { - var timeout = gcsControlStatusFlags_TakeoverAllowed ? 0 : QGroundControl.settingsManager.flyViewSettings.requestControlTimeout.rawValue + var timeout = controlGrantedImmediately ? 0 : QGroundControl.settingsManager.flyViewSettings.requestControlTimeout.rawValue control.activeVehicle.requestOperatorControl(requestControlAllowTakeoverFact.rawValue, timeout) if (timeout > 0) { startProgressTracker(timeout) @@ -318,11 +321,11 @@ Item { } QGCLabel { text: qsTr("Request Timeout (sec):") - visible: !isThisGCSinControl && !gcsControlStatusFlags_TakeoverAllowed + visible: !isThisGCSinControl && !controlGrantedImmediately } FactTextField { fact: QGroundControl.settingsManager.flyViewSettings.requestControlTimeout - visible: !isThisGCSinControl && !gcsControlStatusFlags_TakeoverAllowed + visible: !isThisGCSinControl && !controlGrantedImmediately Layout.alignment: Qt.AlignRight Layout.preferredWidth: ScreenTools.defaultFontPixelWidth * 7 } From 62aaea045ee6eadb9adb5df252046d9642927376 Mon Sep 17 00:00:00 2001 From: davidsastresas Date: Sun, 14 Jun 2026 13:51:49 +0200 Subject: [PATCH 08/23] GCSControlIndicator.qml: fix width for aditional GCS in range label --- src/Toolbar/GCSControlIndicator.qml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Toolbar/GCSControlIndicator.qml b/src/Toolbar/GCSControlIndicator.qml index fe355d7325c9..af2f426f2cd5 100644 --- a/src/Toolbar/GCSControlIndicator.qml +++ b/src/Toolbar/GCSControlIndicator.qml @@ -424,7 +424,11 @@ Item { QGCLabel { visible: rangeSummaryLabel.visible && rangeSummaryLabel.unconfiguredIdsInRange > 0 Layout.columnSpan: 2 + // preferredWidth 0 keeps the long warning text from inflating the + // panel's natural width; fillWidth then stretches it to whatever + // width the other panel elements already establish, wrapping inside it Layout.fillWidth: true + Layout.preferredWidth: 0 wrapMode: Text.WordWrap color: qgcPal.colorOrange text: qsTr("Warning: %1 other GCS id(s) inside this range will also be accepted as operators").arg(rangeSummaryLabel.unconfiguredIdsInRange) From 9fef2cde80cc71402c48f59f019f37de43ef8757 Mon Sep 17 00:00:00 2001 From: davidsastresas Date: Wed, 17 Jun 2026 15:51:22 +0200 Subject: [PATCH 09/23] GCSControlIndicator.qml: accept space separated too for secondary GCS range --- src/Settings/FlyView.SettingsGroup.json | 2 +- src/Toolbar/GCSControlIndicator.qml | 7 ++++--- src/Vehicle/Vehicle.cc | 6 ++++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/Settings/FlyView.SettingsGroup.json b/src/Settings/FlyView.SettingsGroup.json index 36855b64850c..49934ec7e687 100644 --- a/src/Settings/FlyView.SettingsGroup.json +++ b/src/Settings/FlyView.SettingsGroup.json @@ -139,7 +139,7 @@ }, { "name": "operatorControlSecondaryGCS", - "shortDesc": "Comma-separated list of GCS system IDs to include in the control range when requesting operator control. Empty means single-GCS mode.", + "shortDesc": "List of GCS system IDs to include in the control range when requesting operator control, separated by commas or spaces. Empty means single-GCS mode.", "type": "string", "default": "", "label": "Secondary GCS system IDs to include in control range" diff --git a/src/Toolbar/GCSControlIndicator.qml b/src/Toolbar/GCSControlIndicator.qml index af2f426f2cd5..571b10013690 100644 --- a/src/Toolbar/GCSControlIndicator.qml +++ b/src/Toolbar/GCSControlIndicator.qml @@ -385,7 +385,7 @@ Item { } // Expanded settings QGCLabel { - text: qsTr("Secondary GCS IDs (comma-separated):") + text: qsTr("Secondary GCS IDs (comma or space separated):") Layout.columnSpan: 2 visible: rangeSettingsToggle.expanded } @@ -405,12 +405,13 @@ Item { property int unconfiguredIdsInRange: 0 text: { var myId = QGroundControl.settingsManager.mavlinkSettings.gcsMavlinkSystemID.rawValue - var parts = secondaryGCSSetting.split(",") + // Accept any non-digit separator (commas, spaces, or a mix), matching _computeOperatorControlRange + var parts = secondaryGCSSetting.match(/\d+/g) || [] var ids = [ myId ] var lo = myId var hi = myId for (var i = 0; i < parts.length; i++) { - var val = parseInt(parts[i].trim()) + var val = parseInt(parts[i]) if (!isNaN(val) && val >= 1 && val <= 255) { if (val < lo) lo = val if (val > hi) hi = val diff --git a/src/Vehicle/Vehicle.cc b/src/Vehicle/Vehicle.cc index aee09a917858..761cd8f05abd 100644 --- a/src/Vehicle/Vehicle.cc +++ b/src/Vehicle/Vehicle.cc @@ -84,6 +84,7 @@ #endif #include +#include QGC_LOGGING_CATEGORY(VehicleLog, "Vehicle.Vehicle") @@ -3210,10 +3211,11 @@ void Vehicle::_computeOperatorControlRange(uint8_t &rangeLow, uint8_t &rangeHigh uint8_t lo = myId; uint8_t hi = myId; - const QStringList parts = secondaryStr.split(',', Qt::SkipEmptyParts); + // Accept any non-digit separator (commas, spaces, or a mix) so the field is forgiving + const QStringList parts = secondaryStr.split(QRegularExpression(QStringLiteral("[^0-9]+")), Qt::SkipEmptyParts); for (const QString &part : parts) { bool ok = false; - const int val = part.trimmed().toInt(&ok); + const int val = part.toInt(&ok); if (ok && val >= 1 && val <= 255) { const uint8_t id = static_cast(val); if (id < lo) lo = id; From d4a345e5780cf9d9c22e4984acea2495a73d0044 Mon Sep 17 00:00:00 2001 From: davidsastresas Date: Thu, 18 Jun 2026 11:06:01 +0200 Subject: [PATCH 10/23] Toolbar: redesign GCS control indicator with role-based icons Replace the line/device/gcs icon set with a state-driven composition using new artwork (aircraft, line, solid/outlined device, open/closed lock): - aircraft (top-left): green when any GCS controls the vehicle - line (bottom-left): green only when this GCS has an operator role (its own control link) - role glyph (right), always white: solid device = primary, outlined device = secondary, lock when this GCS has no role - open when control is acquirable now (uncontrolled or takeover allowed), closed when a request is required - PRIM/SEC label shown in green when this GCS is primary/secondary The lock open/closed condition reuses controlGrantedImmediately, so it always agrees with the request button (Take Control / Acquire / Send Request). > Co-authored-by: alexdelatorre --- ...ntrol_device.svg => multigcs_aircraft.svg} | 6 +-- ...gcscontrol_gcs.svg => multigcs_device.svg} | 4 +- .../multigcs_device_alt.svg | 24 +++++++++ ...{gcscontrol_line.svg => multigcs_line.svg} | 4 +- .../multigcs_lock_closed.svg | 20 ++++++++ .../multigcs_lock_open.svg | 20 ++++++++ src/Toolbar/CMakeLists.txt | 9 ++-- src/Toolbar/GCSControlIndicator.qml | 49 +++++++++++++------ 8 files changed, 111 insertions(+), 25 deletions(-) rename resources/gcscontrolIndicator/{gcscontrol_device.svg => multigcs_aircraft.svg} (55%) rename resources/gcscontrolIndicator/{gcscontrol_gcs.svg => multigcs_device.svg} (58%) create mode 100644 resources/gcscontrolIndicator/multigcs_device_alt.svg rename resources/gcscontrolIndicator/{gcscontrol_line.svg => multigcs_line.svg} (63%) create mode 100644 resources/gcscontrolIndicator/multigcs_lock_closed.svg create mode 100644 resources/gcscontrolIndicator/multigcs_lock_open.svg diff --git a/resources/gcscontrolIndicator/gcscontrol_device.svg b/resources/gcscontrolIndicator/multigcs_aircraft.svg similarity index 55% rename from resources/gcscontrolIndicator/gcscontrol_device.svg rename to resources/gcscontrolIndicator/multigcs_aircraft.svg index fb267ffe5074..85a841045772 100644 --- a/resources/gcscontrolIndicator/gcscontrol_device.svg +++ b/resources/gcscontrolIndicator/multigcs_aircraft.svg @@ -11,10 +11,10 @@ } - + - - + + \ No newline at end of file diff --git a/resources/gcscontrolIndicator/gcscontrol_gcs.svg b/resources/gcscontrolIndicator/multigcs_device.svg similarity index 58% rename from resources/gcscontrolIndicator/gcscontrol_gcs.svg rename to resources/gcscontrolIndicator/multigcs_device.svg index af965e771f2d..1e7a475c049f 100644 --- a/resources/gcscontrolIndicator/gcscontrol_gcs.svg +++ b/resources/gcscontrolIndicator/multigcs_device.svg @@ -11,10 +11,10 @@ } - + - + \ No newline at end of file diff --git a/resources/gcscontrolIndicator/multigcs_device_alt.svg b/resources/gcscontrolIndicator/multigcs_device_alt.svg new file mode 100644 index 000000000000..6aafe1969d6d --- /dev/null +++ b/resources/gcscontrolIndicator/multigcs_device_alt.svg @@ -0,0 +1,24 @@ + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/resources/gcscontrolIndicator/gcscontrol_line.svg b/resources/gcscontrolIndicator/multigcs_line.svg similarity index 63% rename from resources/gcscontrolIndicator/gcscontrol_line.svg rename to resources/gcscontrolIndicator/multigcs_line.svg index 8e695693efd9..9b1529f5fc81 100644 --- a/resources/gcscontrolIndicator/gcscontrol_line.svg +++ b/resources/gcscontrolIndicator/multigcs_line.svg @@ -11,10 +11,10 @@ } - + - + \ No newline at end of file diff --git a/resources/gcscontrolIndicator/multigcs_lock_closed.svg b/resources/gcscontrolIndicator/multigcs_lock_closed.svg new file mode 100644 index 000000000000..f2377409b20f --- /dev/null +++ b/resources/gcscontrolIndicator/multigcs_lock_closed.svg @@ -0,0 +1,20 @@ + + + + + + + + + + + + \ No newline at end of file diff --git a/resources/gcscontrolIndicator/multigcs_lock_open.svg b/resources/gcscontrolIndicator/multigcs_lock_open.svg new file mode 100644 index 000000000000..449fbc740bdc --- /dev/null +++ b/resources/gcscontrolIndicator/multigcs_lock_open.svg @@ -0,0 +1,20 @@ + + + + + + + + + + + + \ No newline at end of file diff --git a/src/Toolbar/CMakeLists.txt b/src/Toolbar/CMakeLists.txt index 6a2fd68dac4f..feb21e5df5d4 100644 --- a/src/Toolbar/CMakeLists.txt +++ b/src/Toolbar/CMakeLists.txt @@ -101,7 +101,10 @@ qt_add_resources(${CMAKE_PROJECT_NAME} toolbar_gcscontrol_resources PREFIX "/gcscontrolIndicator" BASE "${CMAKE_SOURCE_DIR}/resources/gcscontrolIndicator" FILES - "${CMAKE_SOURCE_DIR}/resources/gcscontrolIndicator/gcscontrol_device.svg" - "${CMAKE_SOURCE_DIR}/resources/gcscontrolIndicator/gcscontrol_gcs.svg" - "${CMAKE_SOURCE_DIR}/resources/gcscontrolIndicator/gcscontrol_line.svg" + "${CMAKE_SOURCE_DIR}/resources/gcscontrolIndicator/multigcs_aircraft.svg" + "${CMAKE_SOURCE_DIR}/resources/gcscontrolIndicator/multigcs_line.svg" + "${CMAKE_SOURCE_DIR}/resources/gcscontrolIndicator/multigcs_device.svg" + "${CMAKE_SOURCE_DIR}/resources/gcscontrolIndicator/multigcs_device_alt.svg" + "${CMAKE_SOURCE_DIR}/resources/gcscontrolIndicator/multigcs_lock_open.svg" + "${CMAKE_SOURCE_DIR}/resources/gcscontrolIndicator/multigcs_lock_closed.svg" ) diff --git a/src/Toolbar/GCSControlIndicator.qml b/src/Toolbar/GCSControlIndicator.qml index 571b10013690..fa56aa809e0e 100644 --- a/src/Toolbar/GCSControlIndicator.qml +++ b/src/Toolbar/GCSControlIndicator.qml @@ -7,7 +7,7 @@ import QGroundControl.FactControls Item { id: control - width: controlIndicatorIconGCS.width * 1.1 + width: controlIndicatorIconRole.width * 1.1 anchors.top: parent.top anchors.bottom: parent.bottom @@ -24,6 +24,14 @@ Item { // When nobody is in control (uncontrolled) or takeover is allowed, the autopilot grants control // immediately, so there is no owner to ask and no request countdown property bool controlGrantedImmediately: sysidInControl == 0 || gcsControlStatusFlags_TakeoverAllowed + // Someone (anyone) holds control of the vehicle + property bool someoneInControl: sysidInControl != 0 + // This GCS is a recognized secondary operator: another GCS is primary and the vehicle lists us + // in its secondary range. (When uncontrolled the vehicle lists every recognized GCS, so scope to someoneInControl.) + property bool isThisGCSsecondary: someoneInControl && !isThisGCSinControl && + secondaryGCSList.indexOf(Number(QGroundControl.settingsManager.mavlinkSettings.gcsMavlinkSystemID.rawValue)) >= 0 + // This GCS has an operator role (primary or secondary) on the vehicle + property bool isThisGCSoperator: isThisGCSinControl || isThisGCSsecondary property Fact secondaryGCSSettingFact: QGroundControl.settingsManager.flyViewSettings.operatorControlSecondaryGCS property string secondaryGCSSetting: secondaryGCSSettingFact.rawValue @@ -438,44 +446,55 @@ Item { } } - // Actual top toolbar indicator + // Actual top toolbar indicator. Three stacked layers occupying different quadrants: + // aircraft (top-left) - green when any GCS controls the vehicle + // line (bottom-left) - green only when THIS GCS has an operator role (its control link) + // role glyph (right) - solid device = primary, outlined device = secondary, + // lock open/closed = no role (open when control is acquirable now). + // Always white: it's your station; green is reserved for the control + // relationship (aircraft + line + PRIM/SEC label). QGCColoredImage { - id: controlIndicatorIconLine + id: controlIndicatorIconAircraft width: height anchors.top: parent.top anchors.bottom: parent.bottom - source: "/gcscontrolIndicator/gcscontrol_line.svg" + source: "/gcscontrolIndicator/multigcs_aircraft.svg" fillMode: Image.PreserveAspectFit sourceSize.height: height - color: isThisGCSinControl ? qgcPal.colorGreen : qgcPal.text + color: someoneInControl ? qgcPal.colorGreen : qgcPal.text } QGCColoredImage { - id: controlIndicatorIconAircraft + id: controlIndicatorIconLine width: height anchors.top: parent.top anchors.bottom: parent.bottom - source: "/gcscontrolIndicator/gcscontrol_device.svg" + source: "/gcscontrolIndicator/multigcs_line.svg" fillMode: Image.PreserveAspectFit sourceSize.height: height - color: (isThisGCSinControl || gcsControlStatusFlags_TakeoverAllowed) ? qgcPal.colorGreen : qgcPal.text + color: isThisGCSoperator ? qgcPal.colorGreen : qgcPal.text } QGCColoredImage { - id: controlIndicatorIconGCS + id: controlIndicatorIconRole width: height anchors.top: parent.top anchors.bottom: parent.bottom - source: "/gcscontrolIndicator/gcscontrol_gcs.svg" + source: isThisGCSoperator + ? (isThisGCSinControl ? "/gcscontrolIndicator/multigcs_device.svg" + : "/gcscontrolIndicator/multigcs_device_alt.svg") + : (controlGrantedImmediately ? "/gcscontrolIndicator/multigcs_lock_open.svg" + : "/gcscontrolIndicator/multigcs_lock_closed.svg") fillMode: Image.PreserveAspectFit sourceSize.height: height color: qgcPal.text - // Current GCS in control indicator + // PRIM/SEC role label, only shown when this GCS has an operator role QGCLabel { - id: gcsInControlIndicator - text: gcsMain + id: roleLabel + text: isThisGCSinControl ? qsTr("PRIM") : qsTr("SEC") + visible: isThisGCSoperator font.bold: true - font.pointSize: ScreenTools.smallFontPointSize * 1.1 - color: isThisGCSinControl ? qgcPal.colorGreen : qgcPal.text + font.pointSize: ScreenTools.smallFontPointSize + color: qgcPal.colorGreen anchors.bottom: parent.bottom anchors.bottomMargin: -margins * 0.7 anchors.right: parent.right From 2f2d4cc8a1a4a0939494959b476d8666c4410917 Mon Sep 17 00:00:00 2001 From: davidsastresas Date: Thu, 18 Jun 2026 17:33:44 +0200 Subject: [PATCH 11/23] GCSControlIndicator.qml: rework GCS control panel: Replace the flat label list with a labelled status roster (Control status / Main GCS / Secondary GCS / Takeover), mark this GCS in each row, and move editable settings behind a More options expander with a control group toggle. Recognize a secondary's role even when the vehicle is uncontrolled (gcs_main == 0). --- src/Toolbar/GCSControlIndicator.qml | 197 ++++++++++++++++++---------- 1 file changed, 127 insertions(+), 70 deletions(-) diff --git a/src/Toolbar/GCSControlIndicator.qml b/src/Toolbar/GCSControlIndicator.qml index fa56aa809e0e..2d754de55c6b 100644 --- a/src/Toolbar/GCSControlIndicator.qml +++ b/src/Toolbar/GCSControlIndicator.qml @@ -26,9 +26,10 @@ Item { property bool controlGrantedImmediately: sysidInControl == 0 || gcsControlStatusFlags_TakeoverAllowed // Someone (anyone) holds control of the vehicle property bool someoneInControl: sysidInControl != 0 - // This GCS is a recognized secondary operator: another GCS is primary and the vehicle lists us - // in its secondary range. (When uncontrolled the vehicle lists every recognized GCS, so scope to someoneInControl.) - property bool isThisGCSsecondary: someoneInControl && !isThisGCSinControl && + // This GCS is a recognized secondary operator: the vehicle lists us in its secondary range. + // This holds even when uncontrolled (gcs_main == 0): a GCS within the recognized range is an + // owner that can command the vehicle, just not the one holding manual control. + property bool isThisGCSsecondary: !isThisGCSinControl && secondaryGCSList.indexOf(Number(QGroundControl.settingsManager.mavlinkSettings.gcsMavlinkSystemID.rawValue)) >= 0 // This GCS has an operator role (primary or secondary) on the vehicle property bool isThisGCSoperator: isThisGCSinControl || isThisGCSsecondary @@ -260,49 +261,76 @@ Item { id: mainLayout columns: 2 + // --- Status (read-only) --- + // 1. My situation: in control (main or secondary), else whether control is acquirable QGCLabel { - text: qsTr("System in control: ") + text: qsTr("Control status:") font.bold: true } QGCLabel { - text: isThisGCSinControl ? (qsTr("This GCS") + " (" + gcsMain + ")" ) : gcsMain - font.bold: isThisGCSinControl - color: isThisGCSinControl ? qgcPal.colorGreen : qgcPal.text + text: isThisGCSinControl ? qsTr("In control, full") + : (isThisGCSsecondary ? qsTr("In control, commands only") + : (controlGrantedImmediately ? qsTr("Unlocked") : qsTr("Request needed"))) + font.bold: isThisGCSoperator + color: isThisGCSoperator ? qgcPal.colorGreen : qgcPal.text Layout.alignment: Qt.AlignRight Layout.fillWidth: true horizontalAlignment: Text.AlignRight } + // 2. Takeover permission QGCLabel { - text: gcsControlStatusFlags_TakeoverAllowed ? qsTr("Takeover allowed") : qsTr("Takeover NOT allowed") + text: controlGrantedImmediately ? qsTr("Takeover allowed") : qsTr("Takeover not allowed") Layout.columnSpan: 2 + } + // 3. Ownership roster: the main GCS and any secondaries, each labelled (this GCS marked) + QGCLabel { + text: qsTr("Main GCS:") + font.bold: true + } + QGCLabel { + text: isThisGCSinControl ? (sysidInControl + qsTr(" (This GCS)")) + : (someoneInControl ? ("" + sysidInControl) : qsTr("Nobody")) + font.bold: isThisGCSinControl + color: isThisGCSinControl ? qgcPal.colorGreen : qgcPal.text Layout.alignment: Qt.AlignRight Layout.fillWidth: true horizontalAlignment: Text.AlignRight - color: gcsControlStatusFlags_TakeoverAllowed ? qgcPal.colorGreen : qgcPal.text } QGCLabel { - text: qsTr("Secondary GCS: ") + secondaryGCSList.join(", ") - Layout.columnSpan: 2 + text: qsTr("Secondary GCS:") + font.bold: true + visible: secondaryGCSList.length > 0 + } + QGCLabel { visible: secondaryGCSList.length > 0 + color: qgcPal.text + textFormat: Text.StyledText + Layout.alignment: Qt.AlignRight + Layout.fillWidth: true + horizontalAlignment: Text.AlignRight + // List secondaries; colour only the entry that is this GCS green + text: { + var myId = Number(QGroundControl.settingsManager.mavlinkSettings.gcsMavlinkSystemID.rawValue) + var out = [] + for (var i = 0; i < secondaryGCSList.length; i++) { + var id = secondaryGCSList[i] + if (id === myId) { + out.push('' + id + qsTr(" (This GCS)") + '') + } else { + out.push("" + id) + } + } + return out.join(", ") + } } // Separator Rectangle { Layout.columnSpan: 2 - Layout.preferredWidth: parent.width - Layout.alignment: Qt.AlignHCenter + Layout.fillWidth: true color: qgcPal.windowShade height: outdoorPalette ? 1 : 2 } - QGCLabel { - text: qsTr("Send Control Request:") - Layout.columnSpan: 2 - visible: !isThisGCSinControl - } - QGCLabel { - text: qsTr("Change takeover condition:") - Layout.columnSpan: 2 - visible: isThisGCSinControl - } + // --- Actions --- QGCLabel { id: requestSentTimeoutLabel text: qsTr("Request sent: ") + sendRequestProgressTracker.progressLabel @@ -315,7 +343,9 @@ Item { enabled: gcsControlStatusFlags_TakeoverAllowed || isThisGCSinControl } QGCButton { - text: controlGrantedImmediately ? qsTr("Acquire Control") : qsTr("Send Request") + // Requesting always targets the main (full) role, so a secondary that already has + // command authority sees that this upgrades it to full/manual control + text: controlGrantedImmediately ? qsTr("Take full control") : qsTr("Request full control") onClicked: { var timeout = controlGrantedImmediately ? 0 : QGroundControl.settingsManager.flyViewSettings.requestControlTimeout.rawValue control.activeVehicle.requestOperatorControl(requestControlAllowTakeoverFact.rawValue, timeout) @@ -327,22 +357,14 @@ Item { visible: !isThisGCSinControl enabled: !sendRequestProgressTracker.running } - QGCLabel { - text: qsTr("Request Timeout (sec):") - visible: !isThisGCSinControl && !controlGrantedImmediately - } - FactTextField { - fact: QGroundControl.settingsManager.flyViewSettings.requestControlTimeout - visible: !isThisGCSinControl && !controlGrantedImmediately - Layout.alignment: Qt.AlignRight - Layout.preferredWidth: ScreenTools.defaultFontPixelWidth * 7 - } QGCButton { text: qsTr("Change") onClicked: control.activeVehicle.requestOperatorControl(requestControlAllowTakeoverFact.rawValue) visible: isThisGCSinControl Layout.alignment: Qt.AlignRight enabled: gcsControlStatusFlags_TakeoverAllowed != requestControlAllowTakeoverFact.rawValue + // padding to the right, otherwise the panel will get too narrow and the UI will look inconsistent when only this button is present. + Layout.leftMargin: ScreenTools.defaultFontPixelWidth * 5 } QGCButton { text: qsTr("Release Control") @@ -356,57 +378,90 @@ Item { // Separator Rectangle { Layout.columnSpan: 2 - Layout.preferredWidth: parent.width - Layout.alignment: Qt.AlignHCenter + Layout.fillWidth: true color: qgcPal.windowShade height: outdoorPalette ? 1 : 2 } - LabelledFactTextField { - Layout.fillWidth: true + // More options expander (editable settings). A full-width button is a reliable + // touch target on small screens, unlike a bare clickable label. + QGCButton { + id: moreOptionsToggle + property bool expanded: false + text: (expanded ? qsTr("▾ ") : qsTr("▸ ")) + qsTr("More options") Layout.columnSpan: 2 - label: qsTr("This GCS Mavlink System ID: ") - fact: QGroundControl.settingsManager.mavlinkSettings.gcsMavlinkSystemID + Layout.fillWidth: true + onClicked: expanded = !expanded } - // Separator - Rectangle { - Layout.columnSpan: 2 - Layout.preferredWidth: parent.width - Layout.alignment: Qt.AlignHCenter - color: qgcPal.windowShade - height: outdoorPalette ? 1 : 2 + // This GCS system ID setting. Label on its own wrapping row so the long text doesn't + // drive the panel width; small field below. + QGCLabel { + text: qsTr("This GCS System ID:") + Layout.fillWidth: true + Layout.preferredWidth: 0 + wrapMode: Text.WordWrap + visible: moreOptionsToggle.expanded } - // Compact hint when secondary GCS are configured (collapsed) + FactTextField { + fact: QGroundControl.settingsManager.mavlinkSettings.gcsMavlinkSystemID + Layout.preferredWidth: ScreenTools.defaultFontPixelWidth * 8 + visible: moreOptionsToggle.expanded + } + // Request timeout setting (same wrapping-label treatment) QGCLabel { - property bool expanded: false - id: rangeSettingsToggle - text: hasConfiguredSecondaryGCS - ? qsTr("Control range configured (click to edit)") - : qsTr("Configure secondary GCS range") + text: qsTr("Takeover request timeout (s):") + Layout.fillWidth: true + Layout.preferredWidth: 0 + wrapMode: Text.WordWrap + visible: moreOptionsToggle.expanded + } + FactTextField { + fact: QGroundControl.settingsManager.flyViewSettings.requestControlTimeout + Layout.preferredWidth: ScreenTools.defaultFontPixelWidth * 8 + visible: moreOptionsToggle.expanded + } + // Multi-owner toggle: reveals the secondary GCS field. The label is a separate wrapping, + // clickable QGCLabel (QGCCheckBox text can't wrap) so the long text doesn't drive panel + // width. Initialised from whether secondaries are configured; unticking clears the list. + RowLayout { Layout.columnSpan: 2 - color: hasConfiguredSecondaryGCS ? qgcPal.buttonHighlight : qgcPal.text - font.underline: true - MouseArea { - anchors.fill: parent - cursorShape: Qt.PointingHandCursor - onClicked: rangeSettingsToggle.expanded = !rangeSettingsToggle.expanded + Layout.fillWidth: true + visible: moreOptionsToggle.expanded + spacing: ScreenTools.defaultFontPixelWidth + QGCCheckBox { + id: multiGcsCheckBox + checked: hasConfiguredSecondaryGCS + onCheckedChanged: if (!checked) secondaryGCSSettingFact.rawValue = "" + } + QGCLabel { + text: qsTr("This GCS is part of a control group") + Layout.fillWidth: true + Layout.preferredWidth: 0 + wrapMode: Text.WordWrap + MouseArea { + anchors.fill: parent + onClicked: multiGcsCheckBox.checked = !multiGcsCheckBox.checked + } } } - // Expanded settings + // Secondary GCS list setting. This label is unusually long, so let it wrap instead of + // inflating the panel width: preferredWidth 0 + fillWidth makes it take the width the + // rest of the panel already establishes and wrap within it. QGCLabel { text: qsTr("Secondary GCS IDs (comma or space separated):") Layout.columnSpan: 2 - visible: rangeSettingsToggle.expanded + Layout.fillWidth: true + Layout.preferredWidth: 0 + wrapMode: Text.WordWrap + visible: moreOptionsToggle.expanded && multiGcsCheckBox.checked } FactTextField { fact: secondaryGCSSettingFact - Layout.columnSpan: 2 Layout.fillWidth: true - visible: rangeSettingsToggle.expanded + visible: moreOptionsToggle.expanded && multiGcsCheckBox.checked } QGCLabel { id: rangeSummaryLabel - visible: rangeSettingsToggle.expanded && hasConfiguredSecondaryGCS - Layout.columnSpan: 2 + visible: moreOptionsToggle.expanded && multiGcsCheckBox.checked && hasConfiguredSecondaryGCS color: qgcPal.buttonHighlight // Number of sysids inside the computed range which are neither this GCS nor a configured secondary. // The protocol encodes the request as a contiguous range, so these would be granted control too @@ -447,12 +502,12 @@ Item { } // Actual top toolbar indicator. Three stacked layers occupying different quadrants: - // aircraft (top-left) - green when any GCS controls the vehicle + // aircraft (top-left) - green when the vehicle has a controller or this GCS is an operator // line (bottom-left) - green only when THIS GCS has an operator role (its control link) // role glyph (right) - solid device = primary, outlined device = secondary, // lock open/closed = no role (open when control is acquirable now). // Always white: it's your station; green is reserved for the control - // relationship (aircraft + line + PRIM/SEC label). + // relationship (aircraft + line + MAIN/SEC label). QGCColoredImage { id: controlIndicatorIconAircraft width: height @@ -461,7 +516,9 @@ Item { source: "/gcscontrolIndicator/multigcs_aircraft.svg" fillMode: Image.PreserveAspectFit sourceSize.height: height - color: someoneInControl ? qgcPal.colorGreen : qgcPal.text + // Green when the vehicle has a controller (a main) or when this GCS is itself an operator + // (covers the gcs_main==0 case where this GCS is a recognized secondary) + color: (someoneInControl || isThisGCSoperator) ? qgcPal.colorGreen : qgcPal.text } QGCColoredImage { id: controlIndicatorIconLine @@ -487,10 +544,10 @@ Item { sourceSize.height: height color: qgcPal.text - // PRIM/SEC role label, only shown when this GCS has an operator role + // MAIN/SEC role label, only shown when this GCS has an operator role QGCLabel { id: roleLabel - text: isThisGCSinControl ? qsTr("PRIM") : qsTr("SEC") + text: isThisGCSinControl ? qsTr("MAIN") : qsTr("SEC") visible: isThisGCSoperator font.bold: true font.pointSize: ScreenTools.smallFontPointSize From 9b5cbd481a34fb86d17cb175e86e929d89de7894 Mon Sep 17 00:00:00 2001 From: davidsastresas Date: Thu, 18 Jun 2026 18:52:52 +0200 Subject: [PATCH 12/23] GCSControlIndicator.qml: use ToolIndicatorPage expandedComponent --- src/Toolbar/GCSControlIndicator.qml | 234 ++++++++++++++-------------- 1 file changed, 121 insertions(+), 113 deletions(-) diff --git a/src/Toolbar/GCSControlIndicator.qml b/src/Toolbar/GCSControlIndicator.qml index 2d754de55c6b..503f679b8b06 100644 --- a/src/Toolbar/GCSControlIndicator.qml +++ b/src/Toolbar/GCSControlIndicator.qml @@ -216,6 +216,7 @@ Item { id: controlPopup ToolIndicatorPage { + showExpand: true TimedProgressTracker { id: sendRequestProgressTracker @@ -375,127 +376,134 @@ Item { Layout.alignment: Qt.AlignRight visible: isThisGCSinControl } - // Separator - Rectangle { - Layout.columnSpan: 2 - Layout.fillWidth: true - color: qgcPal.windowShade - height: outdoorPalette ? 1 : 2 - } - // More options expander (editable settings). A full-width button is a reliable - // touch target on small screens, unlike a bare clickable label. - QGCButton { - id: moreOptionsToggle - property bool expanded: false - text: (expanded ? qsTr("▾ ") : qsTr("▸ ")) + qsTr("More options") - Layout.columnSpan: 2 - Layout.fillWidth: true - onClicked: expanded = !expanded - } + } + + // Editable settings, shown to the right of the status/actions panel via the standard + // ToolIndicatorPage expand button, matching the rest of the toolbar indicators. + // Wrapped in an Item with an explicit implicitWidth: the grid is loaded inside a Loader, + // so Layout.* on it is ignored. Without a width source the wrapping labels below + // (preferredWidth 0 + fillWidth) collapse and pile up; the fixed-width wrapper gives the + // anchored grid a stable width to wrap within regardless of which rows are visible. + expandedComponent: Item { + id: settingsRoot + implicitWidth: ScreenTools.defaultFontPixelWidth * 30 + implicitHeight: settingsLayout.implicitHeight + + GridLayout { + id: settingsLayout + anchors.left: parent.left + anchors.right: parent.right + anchors.top: parent.top + columns: 2 + // This GCS system ID setting. Label on its own wrapping row so the long text doesn't - // drive the panel width; small field below. - QGCLabel { - text: qsTr("This GCS System ID:") - Layout.fillWidth: true - Layout.preferredWidth: 0 - wrapMode: Text.WordWrap - visible: moreOptionsToggle.expanded - } - FactTextField { - fact: QGroundControl.settingsManager.mavlinkSettings.gcsMavlinkSystemID - Layout.preferredWidth: ScreenTools.defaultFontPixelWidth * 8 - visible: moreOptionsToggle.expanded - } - // Request timeout setting (same wrapping-label treatment) - QGCLabel { - text: qsTr("Takeover request timeout (s):") - Layout.fillWidth: true - Layout.preferredWidth: 0 - wrapMode: Text.WordWrap - visible: moreOptionsToggle.expanded - } - FactTextField { - fact: QGroundControl.settingsManager.flyViewSettings.requestControlTimeout - Layout.preferredWidth: ScreenTools.defaultFontPixelWidth * 8 - visible: moreOptionsToggle.expanded - } - // Multi-owner toggle: reveals the secondary GCS field. The label is a separate wrapping, - // clickable QGCLabel (QGCCheckBox text can't wrap) so the long text doesn't drive panel - // width. Initialised from whether secondaries are configured; unticking clears the list. - RowLayout { - Layout.columnSpan: 2 - Layout.fillWidth: true - visible: moreOptionsToggle.expanded - spacing: ScreenTools.defaultFontPixelWidth - QGCCheckBox { - id: multiGcsCheckBox - checked: hasConfiguredSecondaryGCS - onCheckedChanged: if (!checked) secondaryGCSSettingFact.rawValue = "" + // drive the panel width; small field below. + QGCLabel { + text: qsTr("This GCS System ID:") + Layout.fillWidth: true + Layout.preferredWidth: 0 + wrapMode: Text.WordWrap } + FactTextField { + fact: QGroundControl.settingsManager.mavlinkSettings.gcsMavlinkSystemID + Layout.preferredWidth: ScreenTools.defaultFontPixelWidth * 8 + Layout.alignment: Qt.AlignRight + } + // Request timeout setting (same wrapping-label treatment) QGCLabel { - text: qsTr("This GCS is part of a control group") - Layout.fillWidth: true - Layout.preferredWidth: 0 - wrapMode: Text.WordWrap - MouseArea { - anchors.fill: parent - onClicked: multiGcsCheckBox.checked = !multiGcsCheckBox.checked + text: qsTr("Takeover request timeout (s):") + Layout.fillWidth: true + Layout.preferredWidth: 0 + wrapMode: Text.WordWrap + } + FactTextField { + fact: QGroundControl.settingsManager.flyViewSettings.requestControlTimeout + Layout.preferredWidth: ScreenTools.defaultFontPixelWidth * 8 + Layout.alignment: Qt.AlignRight + } + // Multi-owner toggle: reveals the secondary GCS field. The label is a separate wrapping, + // clickable QGCLabel (QGCCheckBox text can't wrap) so the long text doesn't drive panel + // width. Initialised from whether secondaries are configured; unticking clears the list. + RowLayout { + Layout.columnSpan: 2 + Layout.fillWidth: true + spacing: ScreenTools.defaultFontPixelWidth + QGCCheckBox { + id: multiGcsCheckBox + checked: hasConfiguredSecondaryGCS + onCheckedChanged: if (!checked) secondaryGCSSettingFact.rawValue = "" + } + QGCLabel { + text: qsTr("This GCS is part of a control group") + Layout.fillWidth: true + Layout.preferredWidth: 0 + wrapMode: Text.WordWrap + MouseArea { + anchors.fill: parent + onClicked: multiGcsCheckBox.checked = !multiGcsCheckBox.checked + } } } - } - // Secondary GCS list setting. This label is unusually long, so let it wrap instead of - // inflating the panel width: preferredWidth 0 + fillWidth makes it take the width the - // rest of the panel already establishes and wrap within it. - QGCLabel { - text: qsTr("Secondary GCS IDs (comma or space separated):") - Layout.columnSpan: 2 - Layout.fillWidth: true - Layout.preferredWidth: 0 - wrapMode: Text.WordWrap - visible: moreOptionsToggle.expanded && multiGcsCheckBox.checked - } - FactTextField { - fact: secondaryGCSSettingFact - Layout.fillWidth: true - visible: moreOptionsToggle.expanded && multiGcsCheckBox.checked - } - QGCLabel { - id: rangeSummaryLabel - visible: moreOptionsToggle.expanded && multiGcsCheckBox.checked && hasConfiguredSecondaryGCS - color: qgcPal.buttonHighlight - // Number of sysids inside the computed range which are neither this GCS nor a configured secondary. - // The protocol encodes the request as a contiguous range, so these would be granted control too - property int unconfiguredIdsInRange: 0 - text: { - var myId = QGroundControl.settingsManager.mavlinkSettings.gcsMavlinkSystemID.rawValue - // Accept any non-digit separator (commas, spaces, or a mix), matching _computeOperatorControlRange - var parts = secondaryGCSSetting.match(/\d+/g) || [] - var ids = [ myId ] - var lo = myId - var hi = myId - for (var i = 0; i < parts.length; i++) { - var val = parseInt(parts[i]) - if (!isNaN(val) && val >= 1 && val <= 255) { - if (val < lo) lo = val - if (val > hi) hi = val - if (ids.indexOf(val) < 0) ids.push(val) + // Secondary GCS list setting. This label is unusually long, so let it wrap instead of + // inflating the panel width: preferredWidth 0 + fillWidth makes it take the width the + // rest of the panel already establishes and wrap within it. + QGCLabel { + text: qsTr("Secondary GCS IDs (comma or space separated):") + Layout.columnSpan: 2 + Layout.fillWidth: true + Layout.preferredWidth: 0 + wrapMode: Text.WordWrap + visible: multiGcsCheckBox.checked + } + FactTextField { + fact: secondaryGCSSettingFact + // Full-width own row with preferredWidth 0: keeps the typed list from widening a + // shared column (which would shove the right-justified fields on the rows above). + Layout.columnSpan: 2 + Layout.fillWidth: true + Layout.preferredWidth: 0 + visible: multiGcsCheckBox.checked + } + QGCLabel { + id: rangeSummaryLabel + // Full-width own row so it never lands in the field column above and inflate it + Layout.columnSpan: 2 + visible: multiGcsCheckBox.checked && hasConfiguredSecondaryGCS + color: qgcPal.buttonHighlight + // Number of sysids inside the computed range which are neither this GCS nor a configured secondary. + // The protocol encodes the request as a contiguous range, so these would be granted control too + property int unconfiguredIdsInRange: 0 + text: { + var myId = QGroundControl.settingsManager.mavlinkSettings.gcsMavlinkSystemID.rawValue + // Accept any non-digit separator (commas, spaces, or a mix), matching _computeOperatorControlRange + var parts = secondaryGCSSetting.match(/\d+/g) || [] + var ids = [ myId ] + var lo = myId + var hi = myId + for (var i = 0; i < parts.length; i++) { + var val = parseInt(parts[i]) + if (!isNaN(val) && val >= 1 && val <= 255) { + if (val < lo) lo = val + if (val > hi) hi = val + if (ids.indexOf(val) < 0) ids.push(val) + } } + rangeSummaryLabel.unconfiguredIdsInRange = (hi - lo + 1) - ids.length + return qsTr("Request range: ") + lo + " - " + hi } - rangeSummaryLabel.unconfiguredIdsInRange = (hi - lo + 1) - ids.length - return qsTr("Request range: ") + lo + " - " + hi } - } - QGCLabel { - visible: rangeSummaryLabel.visible && rangeSummaryLabel.unconfiguredIdsInRange > 0 - Layout.columnSpan: 2 - // preferredWidth 0 keeps the long warning text from inflating the - // panel's natural width; fillWidth then stretches it to whatever - // width the other panel elements already establish, wrapping inside it - Layout.fillWidth: true - Layout.preferredWidth: 0 - wrapMode: Text.WordWrap - color: qgcPal.colorOrange - text: qsTr("Warning: %1 other GCS id(s) inside this range will also be accepted as operators").arg(rangeSummaryLabel.unconfiguredIdsInRange) + QGCLabel { + visible: rangeSummaryLabel.visible && rangeSummaryLabel.unconfiguredIdsInRange > 0 + Layout.columnSpan: 2 + // preferredWidth 0 keeps the long warning text from inflating the + // panel's natural width; fillWidth then stretches it to whatever + // width the other panel elements already establish, wrapping inside it + Layout.fillWidth: true + Layout.preferredWidth: 0 + wrapMode: Text.WordWrap + color: qgcPal.colorOrange + text: qsTr("Warning: %1 other GCS id(s) inside this range will also be accepted as operators").arg(rangeSummaryLabel.unconfiguredIdsInRange) + } } } } From a7c59b369cddd5dafde45ad329f131410d633c4e Mon Sep 17 00:00:00 2001 From: davidsastresas Date: Thu, 18 Jun 2026 19:12:08 +0200 Subject: [PATCH 13/23] Move Multi-GCS code from vehicle to dedicated GCSControlManager --- src/Toolbar/GCSControlIndicator.qml | 31 ++-- src/Vehicle/CMakeLists.txt | 2 + src/Vehicle/GCSControlManager.cc | 258 ++++++++++++++++++++++++++++ src/Vehicle/GCSControlManager.h | 77 +++++++++ src/Vehicle/Vehicle.cc | 249 +-------------------------- src/Vehicle/Vehicle.h | 55 ++---- 6 files changed, 368 insertions(+), 304 deletions(-) create mode 100644 src/Vehicle/GCSControlManager.cc create mode 100644 src/Vehicle/GCSControlManager.h diff --git a/src/Toolbar/GCSControlIndicator.qml b/src/Toolbar/GCSControlIndicator.qml index 503f679b8b06..256e096a57b0 100644 --- a/src/Toolbar/GCSControlIndicator.qml +++ b/src/Toolbar/GCSControlIndicator.qml @@ -12,15 +12,16 @@ Item { anchors.bottom: parent.bottom property var activeVehicle: QGroundControl.multiVehicleManager.activeVehicle - property bool showIndicator: activeVehicle && activeVehicle.firstControlStatusReceived - property var sysidInControl: activeVehicle ? activeVehicle.sysidInControl : 0 - property var secondaryGCSList: activeVehicle ? activeVehicle.secondaryGCSList : [] - property bool gcsControlStatusFlags_SystemManager: activeVehicle ? activeVehicle.gcsControlStatusFlags_SystemManager : false - property bool gcsControlStatusFlags_TakeoverAllowed: activeVehicle ? activeVehicle.gcsControlStatusFlags_TakeoverAllowed : false + property var gcsControlManager: activeVehicle ? activeVehicle.gcsControlManager : null + property bool showIndicator: gcsControlManager && gcsControlManager.firstControlStatusReceived + property var sysidInControl: gcsControlManager ? gcsControlManager.sysidInControl : 0 + property var secondaryGCSList: gcsControlManager ? gcsControlManager.secondaryGCSList : [] + property bool gcsControlStatusFlags_SystemManager: gcsControlManager ? gcsControlManager.gcsControlStatusFlags_SystemManager : false + property bool gcsControlStatusFlags_TakeoverAllowed: gcsControlManager ? gcsControlManager.gcsControlStatusFlags_TakeoverAllowed : false property Fact requestControlAllowTakeoverFact: QGroundControl.settingsManager.flyViewSettings.requestControlAllowTakeover property bool requestControlAllowTakeover: requestControlAllowTakeoverFact.rawValue - property bool isThisGCSinControl: gcsMain == QGroundControl.settingsManager.mavlinkSettings.gcsMavlinkSystemID.rawValue - property bool sendControlRequestAllowed: activeVehicle ? activeVehicle.sendControlRequestAllowed : false + property bool isThisGCSinControl: sysidInControl == QGroundControl.settingsManager.mavlinkSettings.gcsMavlinkSystemID.rawValue + property bool sendControlRequestAllowed: gcsControlManager ? gcsControlManager.sendControlRequestAllowed : false // When nobody is in control (uncontrolled) or takeover is allowed, the autopilot grants control // immediately, so there is no owner to ask and no request countdown property bool controlGrantedImmediately: sysidInControl == 0 || gcsControlStatusFlags_TakeoverAllowed @@ -54,7 +55,7 @@ Item { signal triggerAnimations // Used to trigger animation inside the popup component Connections { - target: activeVehicle + target: gcsControlManager // Popup prompting user to accept control from other GCS function onRequestOperatorControlReceived(sysIdRequestingControl, allowTakeover, requestTimeoutSecs) { // If we don't have the indicator visible ( not receiving CONTROL_STATUS ) don't proceed @@ -131,11 +132,11 @@ Item { Layout.alignment: Qt.AlignBottom Layout.fillHeight: true onClicked: { - control.activeVehicle.requestOperatorControl(true) // Allow takeover + control.gcsControlManager.requestOperatorControl(true) // Allow takeover mainWindow.closeIndicatorDrawer() // After allowing takeover, if other GCS does not take control within 10 seconds // takeover will be set to not allowed again. Notify user about this - control.activeVehicle.startTimerRevertAllowTakeover() + control.gcsControlManager.startTimerRevertAllowTakeover() mainWindow.showIndicatorDrawer(allowTakeoverExpirationPopup, control) } } @@ -172,7 +173,7 @@ Item { // that after vehicle::REQUEST_OPERATOR_CONTROL_ALLOW_TAKEOVER_TIMEOUT_MSECS seconds, this GCS will change back to takeover not allowed, as per mavlink specs TimedProgressTracker { id: revertTakeoverProgressTracker - timeoutSeconds: control.activeVehicle.operatorControlTakeoverTimeoutMsecs * 0.001 + timeoutSeconds: control.gcsControlManager.operatorControlTakeoverTimeoutMsecs * 0.001 onTimeout: { mainWindow.closeIndicatorDrawer() } @@ -249,7 +250,7 @@ Item { // before the other request timeout expired. This way we can keep track of the time remaining and update UI accordingly if (!sendControlRequestAllowed) { // vehicle.requestOperatorControlRemainingMsecs holds the time remaining for the current request - startProgressTracker(control.activeVehicle.requestOperatorControlRemainingMsecs * 0.001) + startProgressTracker(control.gcsControlManager.requestOperatorControlRemainingMsecs * 0.001) } } @@ -349,7 +350,7 @@ Item { text: controlGrantedImmediately ? qsTr("Take full control") : qsTr("Request full control") onClicked: { var timeout = controlGrantedImmediately ? 0 : QGroundControl.settingsManager.flyViewSettings.requestControlTimeout.rawValue - control.activeVehicle.requestOperatorControl(requestControlAllowTakeoverFact.rawValue, timeout) + control.gcsControlManager.requestOperatorControl(requestControlAllowTakeoverFact.rawValue, timeout) if (timeout > 0) { startProgressTracker(timeout) } @@ -360,7 +361,7 @@ Item { } QGCButton { text: qsTr("Change") - onClicked: control.activeVehicle.requestOperatorControl(requestControlAllowTakeoverFact.rawValue) + onClicked: control.gcsControlManager.requestOperatorControl(requestControlAllowTakeoverFact.rawValue) visible: isThisGCSinControl Layout.alignment: Qt.AlignRight enabled: gcsControlStatusFlags_TakeoverAllowed != requestControlAllowTakeoverFact.rawValue @@ -370,7 +371,7 @@ Item { QGCButton { text: qsTr("Release Control") onClicked: { - control.activeVehicle.releaseOperatorControl() + control.gcsControlManager.releaseOperatorControl() } Layout.columnSpan: 2 Layout.alignment: Qt.AlignRight diff --git a/src/Vehicle/CMakeLists.txt b/src/Vehicle/CMakeLists.txt index 6384f82f9754..49f13edc8cc7 100644 --- a/src/Vehicle/CMakeLists.txt +++ b/src/Vehicle/CMakeLists.txt @@ -11,6 +11,8 @@ target_sources(${CMAKE_PROJECT_NAME} FTPController.h FTPManager.cc FTPManager.h + GCSControlManager.cc + GCSControlManager.h InitialConnectStateMachine.cc InitialConnectStateMachine.h MavCommandQueue.cc diff --git a/src/Vehicle/GCSControlManager.cc b/src/Vehicle/GCSControlManager.cc new file mode 100644 index 000000000000..84db02f70a5b --- /dev/null +++ b/src/Vehicle/GCSControlManager.cc @@ -0,0 +1,258 @@ +#include "GCSControlManager.h" +#include "Vehicle.h" +#include "VehicleLinkManager.h" +#include "MAVLinkProtocol.h" +#include "SettingsManager.h" +#include "FlyViewSettings.h" +#include "AppMessages.h" +#include "QGCLoggingCategory.h" + +#include + +QGC_LOGGING_CATEGORY(GCSControlManagerLog, "Vehicle.GCSControlManager") + +#define REQUEST_OPERATOR_CONTROL_ALLOW_TAKEOVER_TIMEOUT_MSECS 10000 + +GCSControlManager::GCSControlManager(Vehicle* vehicle) + : QObject(vehicle) + , _vehicle(vehicle) +{ +} + +int GCSControlManager::operatorControlTakeoverTimeoutMsecs() const +{ + return REQUEST_OPERATOR_CONTROL_ALLOW_TAKEOVER_TIMEOUT_MSECS; +} + +void GCSControlManager::startTimerRevertAllowTakeover() +{ + _timerRevertAllowTakeover.stop(); + _timerRevertAllowTakeover.setSingleShot(true); + _timerRevertAllowTakeover.setInterval(operatorControlTakeoverTimeoutMsecs()); + // Disconnect any previous connections to avoid multiple handlers + disconnect(&_timerRevertAllowTakeover, &QTimer::timeout, nullptr, nullptr); + + connect(&_timerRevertAllowTakeover, &QTimer::timeout, this, [this](){ + if (MAVLinkProtocol::instance()->getSystemId() == _sysid_in_control) { + this->requestOperatorControl(false); + } + }); + _timerRevertAllowTakeover.start(); +} + +void GCSControlManager::_computeOperatorControlRange(uint8_t &rangeLow, uint8_t &rangeHigh) const +{ + const uint8_t myId = static_cast(MAVLinkProtocol::instance()->getSystemId()); + const QString secondaryStr = SettingsManager::instance()->flyViewSettings()->operatorControlSecondaryGCS()->rawValue().toString().trimmed(); + + if (secondaryStr.isEmpty()) { + rangeLow = myId; + rangeHigh = 0; + return; + } + + uint8_t lo = myId; + uint8_t hi = myId; + // Accept any non-digit separator (commas, spaces, or a mix) so the field is forgiving + const QStringList parts = secondaryStr.split(QRegularExpression(QStringLiteral("[^0-9]+")), Qt::SkipEmptyParts); + for (const QString &part : parts) { + bool ok = false; + const int val = part.toInt(&ok); + if (ok && val >= 1 && val <= 255) { + const uint8_t id = static_cast(val); + if (id < lo) lo = id; + if (id > hi) hi = id; + } + } + + rangeLow = lo; + rangeHigh = (hi > lo) ? hi : 0; +} + +void GCSControlManager::requestOperatorControl(bool allowOverride, int requestTimeoutSecs) +{ + int safeRequestTimeoutSecs; + int requestTimeoutSecsMin = SettingsManager::instance()->flyViewSettings()->requestControlTimeout()->cookedMin().toInt(); + int requestTimeoutSecsMax = SettingsManager::instance()->flyViewSettings()->requestControlTimeout()->cookedMax().toInt(); + if (requestTimeoutSecs >= requestTimeoutSecsMin && requestTimeoutSecs <= requestTimeoutSecsMax) { + safeRequestTimeoutSecs = requestTimeoutSecs; + } else { + safeRequestTimeoutSecs = SettingsManager::instance()->flyViewSettings()->requestControlTimeout()->cookedDefaultValue().toInt(); + } + + uint8_t rangeLow, rangeHigh; + _computeOperatorControlRange(rangeLow, rangeHigh); + + const VehicleTypes::MavCmdAckHandlerInfo_t handlerInfo = {&GCSControlManager::_requestOperatorControlAckHandler, this, nullptr, nullptr}; + _vehicle->sendMavCommandWithHandler( + &handlerInfo, + (_operatorControlCompId != 0) ? _operatorControlCompId : _vehicle->defaultComponentId(), + MAV_CMD_REQUEST_OPERATOR_CONTROL, + 1, // param1: Action - 1: Request control + allowOverride ? 1.0f : 0.0f, // param2: Allow takeover + static_cast(safeRequestTimeoutSecs), // param3: Timeout in seconds + static_cast(rangeLow), // param4: GCS sysid (range low) + static_cast(rangeHigh) // param5: GCS sysid upper range (0 = single GCS) + ); + + if (requestTimeoutSecs > 0) { + _requestOperatorControlStartTimer(requestTimeoutSecs * 1000); + } +} + +void GCSControlManager::releaseOperatorControl() +{ + // Releasing control makes any pending request countdown or takeover revert meaningless + _timerRevertAllowTakeover.stop(); + _timerRequestOperatorControl.stop(); + disconnect(&_timerRequestOperatorControl, &QTimer::timeout, nullptr, nullptr); + if (!_sendControlRequestAllowed) { + _sendControlRequestAllowed = true; + emit sendControlRequestAllowedChanged(true); + } + + const VehicleTypes::MavCmdAckHandlerInfo_t handlerInfo = {&GCSControlManager::_requestOperatorControlAckHandler, this, nullptr, nullptr}; + _vehicle->sendMavCommandWithHandler( + &handlerInfo, + (_operatorControlCompId != 0) ? _operatorControlCompId : _vehicle->defaultComponentId(), + MAV_CMD_REQUEST_OPERATOR_CONTROL, + 0, // param1: Action - 0: Release control + 0, // param2: Allow takeover (irrelevant for release) + 0, // param3: Timeout (irrelevant for release) + static_cast(MAVLinkProtocol::instance()->getSystemId()), // param4: GCS sysid + 0 // param5: GCS sysid upper range (0 = single GCS) + ); +} + +void GCSControlManager::_requestOperatorControlAckHandler(void* resultHandlerData, int compId, const mavlink_command_ack_t& ack, VehicleTypes::MavCmdResultFailureCode_t failureCode) +{ + // For the moment, this will always come from an autopilot, compid 1 + Q_UNUSED(compId); + + // If duplicated or no response, show popup to user. Otherwise only log it. + switch (failureCode) { + case VehicleTypes::MavCmdResultFailureDuplicateCommand: + QGC::showAppMessage(tr("Waiting for previous operator control request")); + return; + case VehicleTypes::MavCmdResultFailureNoResponseToCommand: + QGC::showAppMessage(tr("No response to operator control request")); + return; + default: + break; + } + + GCSControlManager* manager = static_cast(resultHandlerData); + if (!manager) { + return; + } + + if (ack.result == MAV_RESULT_ACCEPTED) { + qCDebug(GCSControlManagerLog) << "Operator control request accepted"; + } else { + qCDebug(GCSControlManagerLog) << "Operator control request rejected"; + } +} + +void GCSControlManager::_requestOperatorControlStartTimer(int requestTimeoutMsecs) +{ + // First flag requests not allowed + _sendControlRequestAllowed = false; + emit sendControlRequestAllowedChanged(false); + // Setup timer to re enable it again after timeout + _timerRequestOperatorControl.stop(); + _timerRequestOperatorControl.setSingleShot(true); + _timerRequestOperatorControl.setInterval(requestTimeoutMsecs); + // Disconnect any previous connections to avoid multiple handlers + disconnect(&_timerRequestOperatorControl, &QTimer::timeout, nullptr, nullptr); + connect(&_timerRequestOperatorControl, &QTimer::timeout, this, [this](){ + _sendControlRequestAllowed = true; + emit sendControlRequestAllowedChanged(true); + }); + _timerRequestOperatorControl.start(); +} + +void GCSControlManager::handleControlStatus(const mavlink_message_t& message) +{ + mavlink_control_status_t controlStatus; + mavlink_msg_control_status_decode(&message, &controlStatus); + + if (controlStatus.flags & GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER) { + // This component manages GCS control of the whole system. Operator control + // commands must be addressed to it, which is not necessarily the autopilot + _operatorControlCompId = message.compid; + } + + bool updateControlStatusSignals = false; + if (_gcsControlStatusFlags != controlStatus.flags) { + _gcsControlStatusFlags = controlStatus.flags; + _gcsControlStatusFlags_SystemManager = controlStatus.flags & GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER; + _gcsControlStatusFlags_TakeoverAllowed = controlStatus.flags & GCS_CONTROL_STATUS_FLAGS_TAKEOVER_ALLOWED; + updateControlStatusSignals = true; + } + + if (_sysid_in_control != controlStatus.gcs_main) { + _sysid_in_control = controlStatus.gcs_main; + const uint8_t myId = static_cast(MAVLinkProtocol::instance()->getSystemId()); + _vehicle->setJoystickSendAllowed(_sysid_in_control == 0 || _sysid_in_control == myId); + if (_sysid_in_control != myId) { + // Control moved away from this GCS, so a pending revert to takeover not allowed no longer applies + _timerRevertAllowTakeover.stop(); + } + updateControlStatusSignals = true; + } + + QList newSecondaryList; + for (int i = 0; i < 10; i++) { + if (controlStatus.gcs_secondary[i] != 0) { + newSecondaryList.append(controlStatus.gcs_secondary[i]); + } + } + if (_secondaryGCSList != newSecondaryList) { + _secondaryGCSList = newSecondaryList; + updateControlStatusSignals = true; + } + + if (!_firstControlStatusReceived) { + _firstControlStatusReceived = true; + updateControlStatusSignals = true; + } + + if (updateControlStatusSignals) { + emit gcsControlStatusChanged(); + } + + const uint8_t myId = static_cast(MAVLinkProtocol::instance()->getSystemId()); + if (!sendControlRequestAllowed() && (_sysid_in_control == myId || _sysid_in_control == 0 || _gcsControlStatusFlags_TakeoverAllowed)) { + _timerRequestOperatorControl.stop(); + disconnect(&_timerRequestOperatorControl, &QTimer::timeout, nullptr, nullptr); + _sendControlRequestAllowed = true; + emit sendControlRequestAllowedChanged(true); + } +} + +void GCSControlManager::handleCommandRequestOperatorControl(const mavlink_message_t& message, const mavlink_command_long_t& commandLong) +{ + // Acknowledge the takeover notification so the autopilot can clear its pending notification state + SharedLinkInterfacePtr sharedLink = _vehicle->vehicleLinkManager()->primaryLink().lock(); + if (sharedLink) { + mavlink_message_t ackMessage{}; + (void) mavlink_msg_command_ack_pack_chan( + MAVLinkProtocol::instance()->getSystemId(), + MAVLinkProtocol::getComponentId(), + sharedLink->mavlinkChannel(), + &ackMessage, + MAV_CMD_REQUEST_OPERATOR_CONTROL, + MAV_RESULT_ACCEPTED, + 0, // progress + 0, // result_param2 + message.sysid, + message.compid); + (void) _vehicle->sendMessageOnLinkThreadSafe(sharedLink.get(), ackMessage); + } + + emit requestOperatorControlReceived( + static_cast(commandLong.param4), // GCS sysid requesting control + static_cast(commandLong.param2), // Allow takeover + static_cast(commandLong.param3) // Request timeout in seconds + ); +} diff --git a/src/Vehicle/GCSControlManager.h b/src/Vehicle/GCSControlManager.h new file mode 100644 index 000000000000..e630c5d17160 --- /dev/null +++ b/src/Vehicle/GCSControlManager.h @@ -0,0 +1,77 @@ +#pragma once + +#include +#include +#include +#include + +#include "MAVLinkMessageType.h" +#include "VehicleTypes.h" + +class Vehicle; + +/// Manages GCS / operator control of a single Vehicle (MAVLink #2313). +/// +/// Tracks which GCS currently holds control, the configured secondary GCS list +/// and the request / release / takeover lifecycle, and gates joystick output on +/// the owning Vehicle accordingly. Owned by and parented to that Vehicle, which +/// routes the relevant MAVLink messages here. +class GCSControlManager : public QObject +{ + Q_OBJECT + QML_ELEMENT + QML_UNCREATABLE("") + +public: + GCSControlManager(Vehicle* vehicle); + + Q_PROPERTY(uint8_t sysidInControl READ sysidInControl NOTIFY gcsControlStatusChanged) + Q_PROPERTY(QList secondaryGCSList READ secondaryGCSList NOTIFY gcsControlStatusChanged) + Q_PROPERTY(bool gcsControlStatusFlags_SystemManager READ gcsControlStatusFlags_SystemManager NOTIFY gcsControlStatusChanged) + Q_PROPERTY(bool gcsControlStatusFlags_TakeoverAllowed READ gcsControlStatusFlags_TakeoverAllowed NOTIFY gcsControlStatusChanged) + Q_PROPERTY(bool firstControlStatusReceived READ firstControlStatusReceived NOTIFY gcsControlStatusChanged) + Q_PROPERTY(int operatorControlTakeoverTimeoutMsecs READ operatorControlTakeoverTimeoutMsecs CONSTANT) + Q_PROPERTY(int requestOperatorControlRemainingMsecs READ requestOperatorControlRemainingMsecs NOTIFY sendControlRequestAllowedChanged) + Q_PROPERTY(bool sendControlRequestAllowed READ sendControlRequestAllowed NOTIFY sendControlRequestAllowedChanged) + + Q_INVOKABLE void startTimerRevertAllowTakeover(); + Q_INVOKABLE void requestOperatorControl(bool allowOverride, int requestTimeoutSecs = 0); + Q_INVOKABLE void releaseOperatorControl(); + + uint8_t sysidInControl() const { return _sysid_in_control; } + QList secondaryGCSList() const { return _secondaryGCSList; } + bool gcsControlStatusFlags_SystemManager() const { return _gcsControlStatusFlags_SystemManager; } + bool gcsControlStatusFlags_TakeoverAllowed() const { return _gcsControlStatusFlags_TakeoverAllowed; } + bool firstControlStatusReceived() const { return _firstControlStatusReceived; } + int operatorControlTakeoverTimeoutMsecs() const; + int requestOperatorControlRemainingMsecs() const { return _timerRequestOperatorControl.remainingTime(); } + bool sendControlRequestAllowed() const { return _sendControlRequestAllowed; } + + /// Handle an incoming CONTROL_STATUS message. Called by Vehicle message routing. + void handleControlStatus(const mavlink_message_t& message); + /// Handle an incoming MAV_CMD_REQUEST_OPERATOR_CONTROL command. Called by Vehicle message routing. + void handleCommandRequestOperatorControl(const mavlink_message_t& message, const mavlink_command_long_t& commandLong); + +signals: + void gcsControlStatusChanged(); + void requestOperatorControlReceived(int sysIdRequestingControl, int allowTakeover, int requestTimeoutSecs); + void sendControlRequestAllowedChanged(bool sendControlRequestAllowed); + +private: + static void _requestOperatorControlAckHandler(void* resultHandlerData, int compId, const mavlink_command_ack_t& ack, VehicleTypes::MavCmdResultFailureCode_t failureCode); + void _requestOperatorControlStartTimer(int requestTimeoutMsecs); + void _computeOperatorControlRange(uint8_t &rangeLow, uint8_t &rangeHigh) const; + + Vehicle* _vehicle = nullptr; + + uint8_t _sysid_in_control = 0; + uint8_t _operatorControlCompId = 0; // compid of the system manager component, learned from CONTROL_STATUS + QList _secondaryGCSList; + uint8_t _gcsControlStatusFlags = 0; + bool _gcsControlStatusFlags_SystemManager = false; + bool _gcsControlStatusFlags_TakeoverAllowed = false; + bool _firstControlStatusReceived = false; + QTimer _timerRevertAllowTakeover; + QTimer _timerRequestOperatorControl; + bool _sendControlRequestAllowed = true; +}; diff --git a/src/Vehicle/Vehicle.cc b/src/Vehicle/Vehicle.cc index 761cd8f05abd..9741eca99be7 100644 --- a/src/Vehicle/Vehicle.cc +++ b/src/Vehicle/Vehicle.cc @@ -29,6 +29,7 @@ #include "FirmwarePlugin.h" #include "FirmwarePluginManager.h" #include "FTPManager.h" +#include "GCSControlManager.h" #include "GeoFenceManager.h" #include "ImageProtocolManager.h" #include "InitialConnectStateMachine.h" @@ -92,10 +93,6 @@ QGC_LOGGING_CATEGORY(VehicleLog, "Vehicle.Vehicle") #define DEFAULT_LAT 38.965767f #define DEFAULT_LON -120.083923f -// After a second GCS has requested control and we have given it permission to takeover, we will remove takeover permission automatically after this timeout -// If the second GCS didn't get control -#define REQUEST_OPERATOR_CONTROL_ALLOW_TAKEOVER_TIMEOUT_MSECS 10000 - const QString guided_mode_not_supported_by_vehicle = QObject::tr("Guided mode not supported by Vehicle."); // Standard connected vehicle @@ -305,6 +302,7 @@ void Vehicle::_commonInit(LinkInterface* link) // Remote ID manager might want to acces parameters so make sure to create it after _remoteIDManager = new RemoteIDManager(this); + _gcsControlManager = new GCSControlManager(this); // Flight modes can differ based on advanced mode connect(QGCCorePlugin::instance(), &QGCCorePlugin::showAdvancedUIChanged, this, &Vehicle::flightModesChanged); @@ -742,7 +740,7 @@ void Vehicle::_mavlinkMessageReceived(LinkInterface* link, mavlink_message_t mes break; } case MAVLINK_MSG_ID_CONTROL_STATUS: - _handleControlStatus(message); + _gcsControlManager->handleControlStatus(message); break; case MAVLINK_MSG_ID_COMMAND_LONG: _handleCommandLong(message); @@ -3182,240 +3180,6 @@ void Vehicle::pairRX(int rxType, int rxSubType) rxSubType); } -void Vehicle::startTimerRevertAllowTakeover() -{ - _timerRevertAllowTakeover.stop(); - _timerRevertAllowTakeover.setSingleShot(true); - _timerRevertAllowTakeover.setInterval(operatorControlTakeoverTimeoutMsecs()); - // Disconnect any previous connections to avoid multiple handlers - disconnect(&_timerRevertAllowTakeover, &QTimer::timeout, nullptr, nullptr); - - connect(&_timerRevertAllowTakeover, &QTimer::timeout, this, [this](){ - if (MAVLinkProtocol::instance()->getSystemId() == _gcsMain) { - this->requestOperatorControl(false); - } - }); - _timerRevertAllowTakeover.start(); -} - -void Vehicle::_computeOperatorControlRange(uint8_t &rangeLow, uint8_t &rangeHigh) const -{ - const uint8_t myId = static_cast(MAVLinkProtocol::instance()->getSystemId()); - const QString secondaryStr = SettingsManager::instance()->flyViewSettings()->operatorControlSecondaryGCS()->rawValue().toString().trimmed(); - - if (secondaryStr.isEmpty()) { - rangeLow = myId; - rangeHigh = 0; - return; - } - - uint8_t lo = myId; - uint8_t hi = myId; - // Accept any non-digit separator (commas, spaces, or a mix) so the field is forgiving - const QStringList parts = secondaryStr.split(QRegularExpression(QStringLiteral("[^0-9]+")), Qt::SkipEmptyParts); - for (const QString &part : parts) { - bool ok = false; - const int val = part.toInt(&ok); - if (ok && val >= 1 && val <= 255) { - const uint8_t id = static_cast(val); - if (id < lo) lo = id; - if (id > hi) hi = id; - } - } - - rangeLow = lo; - rangeHigh = (hi > lo) ? hi : 0; -} - -void Vehicle::requestOperatorControl(bool allowOverride, int requestTimeoutSecs) -{ - int safeRequestTimeoutSecs; - int requestTimeoutSecsMin = SettingsManager::instance()->flyViewSettings()->requestControlTimeout()->cookedMin().toInt(); - int requestTimeoutSecsMax = SettingsManager::instance()->flyViewSettings()->requestControlTimeout()->cookedMax().toInt(); - if (requestTimeoutSecs >= requestTimeoutSecsMin && requestTimeoutSecs <= requestTimeoutSecsMax) { - safeRequestTimeoutSecs = requestTimeoutSecs; - } else { - safeRequestTimeoutSecs = SettingsManager::instance()->flyViewSettings()->requestControlTimeout()->cookedDefaultValue().toInt(); - } - - uint8_t rangeLow, rangeHigh; - _computeOperatorControlRange(rangeLow, rangeHigh); - - const MavCmdAckHandlerInfo_t handlerInfo = {&Vehicle::_requestOperatorControlAckHandler, this, nullptr, nullptr}; - sendMavCommandWithHandler( - &handlerInfo, - (_operatorControlCompId != 0) ? _operatorControlCompId : _defaultComponentId, - MAV_CMD_REQUEST_OPERATOR_CONTROL, - 1, // param1: Action - 1: Request control - allowOverride ? 1.0f : 0.0f, // param2: Allow takeover - static_cast(safeRequestTimeoutSecs), // param3: Timeout in seconds - static_cast(rangeLow), // param4: GCS sysid (range low) - static_cast(rangeHigh) // param5: GCS sysid upper range (0 = single GCS) - ); - - if (requestTimeoutSecs > 0) { - requestOperatorControlStartTimer(requestTimeoutSecs * 1000); - } -} - -void Vehicle::releaseOperatorControl() -{ - // Releasing control makes any pending request countdown or takeover revert meaningless - _timerRevertAllowTakeover.stop(); - _timerRequestOperatorControl.stop(); - disconnect(&_timerRequestOperatorControl, &QTimer::timeout, nullptr, nullptr); - if (!_sendControlRequestAllowed) { - _sendControlRequestAllowed = true; - emit sendControlRequestAllowedChanged(true); - } - - const MavCmdAckHandlerInfo_t handlerInfo = {&Vehicle::_requestOperatorControlAckHandler, this, nullptr, nullptr}; - sendMavCommandWithHandler( - &handlerInfo, - (_operatorControlCompId != 0) ? _operatorControlCompId : _defaultComponentId, - MAV_CMD_REQUEST_OPERATOR_CONTROL, - 0, // param1: Action - 0: Release control - 0, // param2: Allow takeover (irrelevant for release) - 0, // param3: Timeout (irrelevant for release) - static_cast(MAVLinkProtocol::instance()->getSystemId()), // param4: GCS sysid - 0 // param5: GCS sysid upper range (0 = single GCS) - ); -} - -void Vehicle::_requestOperatorControlAckHandler(void* resultHandlerData, int compId, const mavlink_command_ack_t& ack, MavCmdResultFailureCode_t failureCode) -{ - // For the moment, this will always come from an autopilot, compid 1 - Q_UNUSED(compId); - - // If duplicated or no response, show popup to user. Otherwise only log it. - switch (failureCode) { - case MavCmdResultFailureDuplicateCommand: - QGC::showAppMessage(tr("Waiting for previous operator control request")); - return; - case MavCmdResultFailureNoResponseToCommand: - QGC::showAppMessage(tr("No response to operator control request")); - return; - default: - break; - } - - Vehicle* vehicle = static_cast(resultHandlerData); - if (!vehicle) { - return; - } - - if (ack.result == MAV_RESULT_ACCEPTED) { - qCDebug(VehicleLog) << "Operator control request accepted"; - } else { - qCDebug(VehicleLog) << "Operator control request rejected"; - } -} - -void Vehicle::requestOperatorControlStartTimer(int requestTimeoutMsecs) -{ - // First flag requests not allowed - _sendControlRequestAllowed = false; - emit sendControlRequestAllowedChanged(false); - // Setup timer to re enable it again after timeout - _timerRequestOperatorControl.stop(); - _timerRequestOperatorControl.setSingleShot(true); - _timerRequestOperatorControl.setInterval(requestTimeoutMsecs); - // Disconnect any previous connections to avoid multiple handlers - disconnect(&_timerRequestOperatorControl, &QTimer::timeout, nullptr, nullptr); - connect(&_timerRequestOperatorControl, &QTimer::timeout, this, [this](){ - _sendControlRequestAllowed = true; - emit sendControlRequestAllowedChanged(true); - }); - _timerRequestOperatorControl.start(); -} - -void Vehicle::_handleControlStatus(const mavlink_message_t& message) -{ - mavlink_control_status_t controlStatus; - mavlink_msg_control_status_decode(&message, &controlStatus); - - if (controlStatus.flags & GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER) { - // This component manages GCS control of the whole system. Operator control - // commands must be addressed to it, which is not necessarily the autopilot - _operatorControlCompId = message.compid; - } - - bool updateControlStatusSignals = false; - if (_gcsControlStatusFlags != controlStatus.flags) { - _gcsControlStatusFlags = controlStatus.flags; - _gcsControlStatusFlags_SystemManager = controlStatus.flags & GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER; - _gcsControlStatusFlags_TakeoverAllowed = controlStatus.flags & GCS_CONTROL_STATUS_FLAGS_TAKEOVER_ALLOWED; - updateControlStatusSignals = true; - } - - if (_sysid_in_control != controlStatus.gcs_main) { - _sysid_in_control = controlStatus.gcs_main; - const uint8_t myId = static_cast(MAVLinkProtocol::instance()->getSystemId()); - _joystickSendAllowed.store(_sysid_in_control == 0 || _sysid_in_control == myId, - std::memory_order_release); - if (_sysid_in_control != myId) { - // Control moved away from this GCS, so a pending revert to takeover not allowed no longer applies - _timerRevertAllowTakeover.stop(); - } - updateControlStatusSignals = true; - } - - QList newSecondaryList; - for (int i = 0; i < 10; i++) { - if (controlStatus.gcs_secondary[i] != 0) { - newSecondaryList.append(controlStatus.gcs_secondary[i]); - } - } - if (_secondaryGCSList != newSecondaryList) { - _secondaryGCSList = newSecondaryList; - updateControlStatusSignals = true; - } - - if (!_firstControlStatusReceived) { - _firstControlStatusReceived = true; - updateControlStatusSignals = true; - } - - if (updateControlStatusSignals) { - emit gcsControlStatusChanged(); - } - - const uint8_t myId = static_cast(MAVLinkProtocol::instance()->getSystemId()); - if (!sendControlRequestAllowed() && (_sysid_in_control == myId || _sysid_in_control == 0 || _gcsControlStatusFlags_TakeoverAllowed)) { - _timerRequestOperatorControl.stop(); - disconnect(&_timerRequestOperatorControl, &QTimer::timeout, nullptr, nullptr); - _sendControlRequestAllowed = true; - emit sendControlRequestAllowedChanged(true); - } -} - -void Vehicle::_handleCommandRequestOperatorControl(const mavlink_message_t& message, const mavlink_command_long_t& commandLong) -{ - // Acknowledge the takeover notification so the autopilot can clear its pending notification state - SharedLinkInterfacePtr sharedLink = vehicleLinkManager()->primaryLink().lock(); - if (sharedLink) { - mavlink_message_t ackMessage{}; - (void) mavlink_msg_command_ack_pack_chan( - MAVLinkProtocol::instance()->getSystemId(), - MAVLinkProtocol::getComponentId(), - sharedLink->mavlinkChannel(), - &ackMessage, - MAV_CMD_REQUEST_OPERATOR_CONTROL, - MAV_RESULT_ACCEPTED, - 0, // progress - 0, // result_param2 - message.sysid, - message.compid); - (void) sendMessageOnLinkThreadSafe(sharedLink.get(), ackMessage); - } - - emit requestOperatorControlReceived( - static_cast(commandLong.param4), // GCS sysid requesting control - static_cast(commandLong.param2), // Allow takeover - static_cast(commandLong.param3) // Request timeout in seconds - ); -} - void Vehicle::_handleCommandLong(const mavlink_message_t& message) { mavlink_command_long_t commandLong; @@ -3425,15 +3189,10 @@ void Vehicle::_handleCommandLong(const mavlink_message_t& message) return; } if (commandLong.command == MAV_CMD_REQUEST_OPERATOR_CONTROL) { - _handleCommandRequestOperatorControl(message, commandLong); + _gcsControlManager->handleCommandRequestOperatorControl(message, commandLong); } } -int Vehicle::operatorControlTakeoverTimeoutMsecs() const -{ - return REQUEST_OPERATOR_CONTROL_ALLOW_TAKEOVER_TIMEOUT_MSECS; -} - int32_t Vehicle::getMessageRate(uint8_t compId, uint16_t msgId) { return _messageIntervalManager->getMessageRate(compId, msgId); diff --git a/src/Vehicle/Vehicle.h b/src/Vehicle/Vehicle.h index 5f4fae9b02d2..7b692756dc78 100644 --- a/src/Vehicle/Vehicle.h +++ b/src/Vehicle/Vehicle.h @@ -70,6 +70,7 @@ class RequestMessageCoordinator; class QGCCameraManager; class RallyPointManager; class RemoteIDManager; +class GCSControlManager; class RequestMessageTest; class RetryableRequestMessageStateTest; class SendMavCommandWithHandlerTest; @@ -97,6 +98,7 @@ class Vehicle : public VehicleFactGroup, public VehicleTypes Q_MOC_INCLUDE("QGCMapCircle.h") Q_MOC_INCLUDE("QmlObjectListModel.h") Q_MOC_INCLUDE("RemoteIDManager.h") + Q_MOC_INCLUDE("GCSControlManager.h") Q_MOC_INCLUDE("TrajectoryPoints.h") Q_MOC_INCLUDE("VehicleLinkManager.h") Q_MOC_INCLUDE("VehicleObjectAvoidance.h") @@ -229,6 +231,7 @@ class Vehicle : public VehicleFactGroup, public VehicleTypes Q_PROPERTY(VehicleObjectAvoidance* objectAvoidance READ objectAvoidance CONSTANT) Q_PROPERTY(Autotune* autotune READ autotune CONSTANT) Q_PROPERTY(RemoteIDManager* remoteIDManager READ remoteIDManager CONSTANT) + Q_PROPERTY(GCSControlManager* gcsControlManager READ gcsControlManager CONSTANT) // FactGroup object model properties @@ -578,6 +581,7 @@ class Vehicle : public VehicleFactGroup, public VehicleTypes VehicleObjectAvoidance* objectAvoidance () { return _objectAvoidance; } Autotune* autotune () const { return _autotune; } RemoteIDManager* remoteIDManager () { return _remoteIDManager; } + GCSControlManager* gcsControlManager () { return _gcsControlManager; } static void showCommandAckError(const mavlink_command_ack_t& ack); @@ -1119,6 +1123,7 @@ private slots: InitialConnectStateMachine* _initialConnectStateMachine = nullptr; Actuators* _actuators = nullptr; RemoteIDManager* _remoteIDManager = nullptr; + GCSControlManager* _gcsControlManager = nullptr; StandardModes* _standardModes = nullptr; // All terrain query workflows (doSetHome, ROI, altAboveTerrain) live in the coordinator. @@ -1150,50 +1155,12 @@ private slots: /* CONTROL STATUS HANDLER */ /*===========================================================================*/ public: - Q_INVOKABLE void startTimerRevertAllowTakeover(); - Q_INVOKABLE void requestOperatorControl(bool allowOverride, int requestTimeoutSecs = 0); - Q_INVOKABLE void releaseOperatorControl(); - -private: - void _handleControlStatus(const mavlink_message_t& message); - void _handleCommandRequestOperatorControl(const mavlink_message_t& message, const mavlink_command_long_t& commandLong); - static void _requestOperatorControlAckHandler(void* resultHandlerData, int compId, const mavlink_command_ack_t& ack, MavCmdResultFailureCode_t failureCode); - - Q_PROPERTY(uint8_t sysidInControl READ sysidInControl NOTIFY gcsControlStatusChanged) - Q_PROPERTY(QList secondaryGCSList READ secondaryGCSList NOTIFY gcsControlStatusChanged) - Q_PROPERTY(bool gcsControlStatusFlags_SystemManager READ gcsControlStatusFlags_SystemManager NOTIFY gcsControlStatusChanged) - Q_PROPERTY(bool gcsControlStatusFlags_TakeoverAllowed READ gcsControlStatusFlags_TakeoverAllowed NOTIFY gcsControlStatusChanged) - Q_PROPERTY(bool firstControlStatusReceived READ firstControlStatusReceived NOTIFY gcsControlStatusChanged) - Q_PROPERTY(int operatorControlTakeoverTimeoutMsecs READ operatorControlTakeoverTimeoutMsecs CONSTANT) - Q_PROPERTY(int requestOperatorControlRemainingMsecs READ requestOperatorControlRemainingMsecs NOTIFY sendControlRequestAllowedChanged) - Q_PROPERTY(bool sendControlRequestAllowed READ sendControlRequestAllowed NOTIFY sendControlRequestAllowedChanged) - - uint8_t sysidInControl() const { return _sysid_in_control; } - QList secondaryGCSList() const { return _secondaryGCSList; } - bool gcsControlStatusFlags_SystemManager() const { return _gcsControlStatusFlags_SystemManager; } - bool gcsControlStatusFlags_TakeoverAllowed() const { return _gcsControlStatusFlags_TakeoverAllowed; } - bool firstControlStatusReceived() const { return _firstControlStatusReceived; } - int operatorControlTakeoverTimeoutMsecs() const; - int requestOperatorControlRemainingMsecs() const { return _timerRequestOperatorControl.remainingTime(); } - bool sendControlRequestAllowed() const { return _sendControlRequestAllowed; } - void requestOperatorControlStartTimer(int requestTimeoutMsecs); - void _computeOperatorControlRange(uint8_t &rangeLow, uint8_t &rangeHigh) const; - - uint8_t _sysid_in_control = 0; - uint8_t _operatorControlCompId = 0; // compid of the system manager component, learned from CONTROL_STATUS - QList _secondaryGCSList; - uint8_t _gcsControlStatusFlags = 0; - bool _gcsControlStatusFlags_SystemManager = 0; - bool _gcsControlStatusFlags_TakeoverAllowed = 0; - bool _firstControlStatusReceived = false; - QTimer _timerRevertAllowTakeover; - QTimer _timerRequestOperatorControl; - bool _sendControlRequestAllowed = true; - -signals: - void gcsControlStatusChanged(); - void requestOperatorControlReceived(int sysIdRequestingControl, int allowTakeover, int requestTimeoutSecs); - void sendControlRequestAllowedChanged(bool sendControlRequestAllowed); + /// Gate for joystick / RC override output. Set by GCSControlManager from the + /// MAVLink receive thread when CONTROL_STATUS changes which GCS is in control; + /// read on the joystick send path. The atomic lives here because it gates the + /// Vehicle's own send methods. Uses release/acquire ordering so the gate change + /// is properly ordered with the control status update that caused it. + void setJoystickSendAllowed(bool allowed) { _joystickSendAllowed.store(allowed, std::memory_order_release); } /*===========================================================================*/ /* STATUS TEXT HANDLER */ From a22d996da11100ce93a7a02196c16a9cce626519 Mon Sep 17 00:00:00 2001 From: davidsastresas Date: Thu, 18 Jun 2026 19:46:53 +0200 Subject: [PATCH 14/23] GCSControlIndicator.qml: Update styling on some labels --- src/Toolbar/GCSControlIndicator.qml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Toolbar/GCSControlIndicator.qml b/src/Toolbar/GCSControlIndicator.qml index 256e096a57b0..3c54ec3c1636 100644 --- a/src/Toolbar/GCSControlIndicator.qml +++ b/src/Toolbar/GCSControlIndicator.qml @@ -270,9 +270,9 @@ Item { font.bold: true } QGCLabel { - text: isThisGCSinControl ? qsTr("In control, full") - : (isThisGCSsecondary ? qsTr("In control, commands only") - : (controlGrantedImmediately ? qsTr("Unlocked") : qsTr("Request needed"))) + text: isThisGCSinControl ? qsTr("In Control, FULL") + : (isThisGCSsecondary ? qsTr("In control, COMMANDS ONLY") + : (controlGrantedImmediately ? qsTr("Unlocked") : qsTr("Request Needed"))) font.bold: isThisGCSoperator color: isThisGCSoperator ? qgcPal.colorGreen : qgcPal.text Layout.alignment: Qt.AlignRight @@ -281,8 +281,9 @@ Item { } // 2. Takeover permission QGCLabel { - text: controlGrantedImmediately ? qsTr("Takeover allowed") : qsTr("Takeover not allowed") + text: controlGrantedImmediately ? qsTr("Takeover ALLOWED") : qsTr("Takeover NOT ALLOWED") Layout.columnSpan: 2 + Layout.alignment: Qt.AlignRight } // 3. Ownership roster: the main GCS and any secondaries, each labelled (this GCS marked) QGCLabel { From d09bb5016171566747ffda3ce2d96eede102a9ec Mon Sep 17 00:00:00 2001 From: David Sastre Date: Thu, 18 Jun 2026 20:53:20 +0200 Subject: [PATCH 15/23] Vehicle.cc: Remove no longer used QtCore/QRegularExpression Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/Vehicle/Vehicle.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Vehicle/Vehicle.cc b/src/Vehicle/Vehicle.cc index 9741eca99be7..6fd70898be99 100644 --- a/src/Vehicle/Vehicle.cc +++ b/src/Vehicle/Vehicle.cc @@ -85,7 +85,6 @@ #endif #include -#include QGC_LOGGING_CATEGORY(VehicleLog, "Vehicle.Vehicle") From 47d0f045671421018b04704844c56300db2dcdc3 Mon Sep 17 00:00:00 2001 From: David Sastre Date: Thu, 18 Jun 2026 20:53:52 +0200 Subject: [PATCH 16/23] GCSControlManager.cc: Fix comment Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/Vehicle/GCSControlManager.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Vehicle/GCSControlManager.cc b/src/Vehicle/GCSControlManager.cc index 84db02f70a5b..41c8b74aa507 100644 --- a/src/Vehicle/GCSControlManager.cc +++ b/src/Vehicle/GCSControlManager.cc @@ -126,7 +126,7 @@ void GCSControlManager::releaseOperatorControl() void GCSControlManager::_requestOperatorControlAckHandler(void* resultHandlerData, int compId, const mavlink_command_ack_t& ack, VehicleTypes::MavCmdResultFailureCode_t failureCode) { - // For the moment, this will always come from an autopilot, compid 1 + // COMMAND_ACK may come from the system-manager component (not necessarily the autopilot) Q_UNUSED(compId); // If duplicated or no response, show popup to user. Otherwise only log it. From eb9614c9dcf740e4e8962d527e1d8d06974f76d7 Mon Sep 17 00:00:00 2001 From: davidsastresas Date: Mon, 22 Jun 2026 13:12:42 +0200 Subject: [PATCH 17/23] FirmwarePlugin: GCSControlIndicator.qml available for non debug builds --- src/FirmwarePlugin/FirmwarePlugin.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/FirmwarePlugin/FirmwarePlugin.cc b/src/FirmwarePlugin/FirmwarePlugin.cc index 5cf1d2d7bb5f..34b842a1597e 100644 --- a/src/FirmwarePlugin/FirmwarePlugin.cc +++ b/src/FirmwarePlugin/FirmwarePlugin.cc @@ -209,10 +209,7 @@ const QVariantList &FirmwarePlugin::toolIndicators(const Vehicle*) QVariant::fromValue(QUrl::fromUserInput("qrc:/qml/QGroundControl/Toolbar/EscIndicator.qml")), QVariant::fromValue(QUrl::fromUserInput("qrc:/qml/QGroundControl/Toolbar/JoystickIndicator.qml")), QVariant::fromValue(QUrl::fromUserInput("qrc:/qml/QGroundControl/Toolbar/MultiVehicleSelector.qml")), -#ifdef QT_DEBUG - // ControlIndicator is only available in debug builds for the moment QVariant::fromValue(QUrl::fromUserInput("qrc:/qml/QGroundControl/Toolbar/GCSControlIndicator.qml")), -#endif }); } From 7e7917a44422eb83a098919ded915dd0c576fd45 Mon Sep 17 00:00:00 2001 From: davidsastresas Date: Mon, 6 Jul 2026 14:41:20 +0200 Subject: [PATCH 18/23] GCSControlManager: stop configuring secondary GCS membership from QGC Secondary/owner membership is the flight stack's configuration (MAV_GCS_SYSID/_HI on ArduPilot), not something a control request may redefine: CONTROL_STATUS specifies gcs_secondary as set by the flight stack, and MAV_CMD_REQUEST_OPERATOR_CONTROL.param5 has been removed from the spec (mavlink/mavlink#2535). Remove the secondary-GCS-IDs setting and its settings-panel UI, and the range computation derived from it. A control request now only asks for the gcs_main role: param4 carries this GCS's system id and param5 is always 0. Secondaries shown in the indicator continue to come from CONTROL_STATUS, which is now their only source. --- src/Settings/FlyView.SettingsGroup.json | 7 -- src/Settings/FlyViewSettings.cc | 1 - src/Settings/FlyViewSettings.h | 1 - src/Toolbar/GCSControlIndicator.qml | 87 ------------------------- src/Vehicle/GCSControlManager.cc | 40 ++---------- src/Vehicle/GCSControlManager.h | 1 - 6 files changed, 4 insertions(+), 133 deletions(-) diff --git a/src/Settings/FlyView.SettingsGroup.json b/src/Settings/FlyView.SettingsGroup.json index 49934ec7e687..fba0b87ef456 100644 --- a/src/Settings/FlyView.SettingsGroup.json +++ b/src/Settings/FlyView.SettingsGroup.json @@ -136,13 +136,6 @@ "default": true, "label": "Enable automatic mission start/resume popups", "keywords": "mission popup" - }, - { - "name": "operatorControlSecondaryGCS", - "shortDesc": "List of GCS system IDs to include in the control range when requesting operator control, separated by commas or spaces. Empty means single-GCS mode.", - "type": "string", - "default": "", - "label": "Secondary GCS system IDs to include in control range" } ] } diff --git a/src/Settings/FlyViewSettings.cc b/src/Settings/FlyViewSettings.cc index c894109d29af..f4f57c55fef9 100644 --- a/src/Settings/FlyViewSettings.cc +++ b/src/Settings/FlyViewSettings.cc @@ -20,4 +20,3 @@ DECLARE_SETTINGSFACT(FlyViewSettings, instrumentQmlFile2) DECLARE_SETTINGSFACT(FlyViewSettings, requestControlAllowTakeover) DECLARE_SETTINGSFACT(FlyViewSettings, requestControlTimeout) DECLARE_SETTINGSFACT(FlyViewSettings, enableAutomaticMissionPopups) -DECLARE_SETTINGSFACT(FlyViewSettings, operatorControlSecondaryGCS) diff --git a/src/Settings/FlyViewSettings.h b/src/Settings/FlyViewSettings.h index 2d38924bf92c..fd6e02154fa1 100644 --- a/src/Settings/FlyViewSettings.h +++ b/src/Settings/FlyViewSettings.h @@ -30,5 +30,4 @@ class FlyViewSettings : public SettingsGroup DEFINE_SETTINGFACT(requestControlAllowTakeover) DEFINE_SETTINGFACT(requestControlTimeout) DEFINE_SETTINGFACT(enableAutomaticMissionPopups) - DEFINE_SETTINGFACT(operatorControlSecondaryGCS) }; diff --git a/src/Toolbar/GCSControlIndicator.qml b/src/Toolbar/GCSControlIndicator.qml index 3c54ec3c1636..7b977ab9f733 100644 --- a/src/Toolbar/GCSControlIndicator.qml +++ b/src/Toolbar/GCSControlIndicator.qml @@ -35,10 +35,6 @@ Item { // This GCS has an operator role (primary or secondary) on the vehicle property bool isThisGCSoperator: isThisGCSinControl || isThisGCSsecondary - property Fact secondaryGCSSettingFact: QGroundControl.settingsManager.flyViewSettings.operatorControlSecondaryGCS - property string secondaryGCSSetting: secondaryGCSSettingFact.rawValue - property bool hasConfiguredSecondaryGCS: secondaryGCSSetting.length > 0 - property var margins: ScreenTools.defaultFontPixelWidth property var panelRadius: ScreenTools.defaultFontPixelWidth * 0.5 property var buttonHeight: height * 1.6 @@ -423,89 +419,6 @@ Item { Layout.preferredWidth: ScreenTools.defaultFontPixelWidth * 8 Layout.alignment: Qt.AlignRight } - // Multi-owner toggle: reveals the secondary GCS field. The label is a separate wrapping, - // clickable QGCLabel (QGCCheckBox text can't wrap) so the long text doesn't drive panel - // width. Initialised from whether secondaries are configured; unticking clears the list. - RowLayout { - Layout.columnSpan: 2 - Layout.fillWidth: true - spacing: ScreenTools.defaultFontPixelWidth - QGCCheckBox { - id: multiGcsCheckBox - checked: hasConfiguredSecondaryGCS - onCheckedChanged: if (!checked) secondaryGCSSettingFact.rawValue = "" - } - QGCLabel { - text: qsTr("This GCS is part of a control group") - Layout.fillWidth: true - Layout.preferredWidth: 0 - wrapMode: Text.WordWrap - MouseArea { - anchors.fill: parent - onClicked: multiGcsCheckBox.checked = !multiGcsCheckBox.checked - } - } - } - // Secondary GCS list setting. This label is unusually long, so let it wrap instead of - // inflating the panel width: preferredWidth 0 + fillWidth makes it take the width the - // rest of the panel already establishes and wrap within it. - QGCLabel { - text: qsTr("Secondary GCS IDs (comma or space separated):") - Layout.columnSpan: 2 - Layout.fillWidth: true - Layout.preferredWidth: 0 - wrapMode: Text.WordWrap - visible: multiGcsCheckBox.checked - } - FactTextField { - fact: secondaryGCSSettingFact - // Full-width own row with preferredWidth 0: keeps the typed list from widening a - // shared column (which would shove the right-justified fields on the rows above). - Layout.columnSpan: 2 - Layout.fillWidth: true - Layout.preferredWidth: 0 - visible: multiGcsCheckBox.checked - } - QGCLabel { - id: rangeSummaryLabel - // Full-width own row so it never lands in the field column above and inflate it - Layout.columnSpan: 2 - visible: multiGcsCheckBox.checked && hasConfiguredSecondaryGCS - color: qgcPal.buttonHighlight - // Number of sysids inside the computed range which are neither this GCS nor a configured secondary. - // The protocol encodes the request as a contiguous range, so these would be granted control too - property int unconfiguredIdsInRange: 0 - text: { - var myId = QGroundControl.settingsManager.mavlinkSettings.gcsMavlinkSystemID.rawValue - // Accept any non-digit separator (commas, spaces, or a mix), matching _computeOperatorControlRange - var parts = secondaryGCSSetting.match(/\d+/g) || [] - var ids = [ myId ] - var lo = myId - var hi = myId - for (var i = 0; i < parts.length; i++) { - var val = parseInt(parts[i]) - if (!isNaN(val) && val >= 1 && val <= 255) { - if (val < lo) lo = val - if (val > hi) hi = val - if (ids.indexOf(val) < 0) ids.push(val) - } - } - rangeSummaryLabel.unconfiguredIdsInRange = (hi - lo + 1) - ids.length - return qsTr("Request range: ") + lo + " - " + hi - } - } - QGCLabel { - visible: rangeSummaryLabel.visible && rangeSummaryLabel.unconfiguredIdsInRange > 0 - Layout.columnSpan: 2 - // preferredWidth 0 keeps the long warning text from inflating the - // panel's natural width; fillWidth then stretches it to whatever - // width the other panel elements already establish, wrapping inside it - Layout.fillWidth: true - Layout.preferredWidth: 0 - wrapMode: Text.WordWrap - color: qgcPal.colorOrange - text: qsTr("Warning: %1 other GCS id(s) inside this range will also be accepted as operators").arg(rangeSummaryLabel.unconfiguredIdsInRange) - } } } } diff --git a/src/Vehicle/GCSControlManager.cc b/src/Vehicle/GCSControlManager.cc index 41c8b74aa507..48441e10bc1e 100644 --- a/src/Vehicle/GCSControlManager.cc +++ b/src/Vehicle/GCSControlManager.cc @@ -7,8 +7,6 @@ #include "AppMessages.h" #include "QGCLoggingCategory.h" -#include - QGC_LOGGING_CATEGORY(GCSControlManagerLog, "Vehicle.GCSControlManager") #define REQUEST_OPERATOR_CONTROL_ALLOW_TAKEOVER_TIMEOUT_MSECS 10000 @@ -40,35 +38,6 @@ void GCSControlManager::startTimerRevertAllowTakeover() _timerRevertAllowTakeover.start(); } -void GCSControlManager::_computeOperatorControlRange(uint8_t &rangeLow, uint8_t &rangeHigh) const -{ - const uint8_t myId = static_cast(MAVLinkProtocol::instance()->getSystemId()); - const QString secondaryStr = SettingsManager::instance()->flyViewSettings()->operatorControlSecondaryGCS()->rawValue().toString().trimmed(); - - if (secondaryStr.isEmpty()) { - rangeLow = myId; - rangeHigh = 0; - return; - } - - uint8_t lo = myId; - uint8_t hi = myId; - // Accept any non-digit separator (commas, spaces, or a mix) so the field is forgiving - const QStringList parts = secondaryStr.split(QRegularExpression(QStringLiteral("[^0-9]+")), Qt::SkipEmptyParts); - for (const QString &part : parts) { - bool ok = false; - const int val = part.toInt(&ok); - if (ok && val >= 1 && val <= 255) { - const uint8_t id = static_cast(val); - if (id < lo) lo = id; - if (id > hi) hi = id; - } - } - - rangeLow = lo; - rangeHigh = (hi > lo) ? hi : 0; -} - void GCSControlManager::requestOperatorControl(bool allowOverride, int requestTimeoutSecs) { int safeRequestTimeoutSecs; @@ -80,9 +49,8 @@ void GCSControlManager::requestOperatorControl(bool allowOverride, int requestTi safeRequestTimeoutSecs = SettingsManager::instance()->flyViewSettings()->requestControlTimeout()->cookedDefaultValue().toInt(); } - uint8_t rangeLow, rangeHigh; - _computeOperatorControlRange(rangeLow, rangeHigh); - + // Secondary/owner membership is the flight stack's configuration: the request + // only asks for the gcs_main role for this GCS, it never carries a range. const VehicleTypes::MavCmdAckHandlerInfo_t handlerInfo = {&GCSControlManager::_requestOperatorControlAckHandler, this, nullptr, nullptr}; _vehicle->sendMavCommandWithHandler( &handlerInfo, @@ -91,8 +59,8 @@ void GCSControlManager::requestOperatorControl(bool allowOverride, int requestTi 1, // param1: Action - 1: Request control allowOverride ? 1.0f : 0.0f, // param2: Allow takeover static_cast(safeRequestTimeoutSecs), // param3: Timeout in seconds - static_cast(rangeLow), // param4: GCS sysid (range low) - static_cast(rangeHigh) // param5: GCS sysid upper range (0 = single GCS) + static_cast(MAVLinkProtocol::instance()->getSystemId()), // param4: GCS sysid requesting control + 0 // param5: unused (removed from spec) ); if (requestTimeoutSecs > 0) { diff --git a/src/Vehicle/GCSControlManager.h b/src/Vehicle/GCSControlManager.h index e630c5d17160..647ed15a2020 100644 --- a/src/Vehicle/GCSControlManager.h +++ b/src/Vehicle/GCSControlManager.h @@ -60,7 +60,6 @@ class GCSControlManager : public QObject private: static void _requestOperatorControlAckHandler(void* resultHandlerData, int compId, const mavlink_command_ack_t& ack, VehicleTypes::MavCmdResultFailureCode_t failureCode); void _requestOperatorControlStartTimer(int requestTimeoutMsecs); - void _computeOperatorControlRange(uint8_t &rangeLow, uint8_t &rangeHigh) const; Vehicle* _vehicle = nullptr; From 71b3612c2f1216026da32634f704f2223dbe2a61 Mon Sep 17 00:00:00 2001 From: davidsastresas Date: Tue, 14 Jul 2026 13:48:20 +0200 Subject: [PATCH 19/23] GCSControlManager: surface rejections, gate on SYSTEM_MANAGER, multi-vehicle notify Address the operator-control review findings: - A request denied by the vehicle (requester not an authorized owner) was only logged while the UI kept a "Request sent" countdown and the request button stayed locked out. Cancel the countdown and show an app message on MAV_RESULT_DENIED and other non-pending rejections; keep the countdown only for FAILED/IN_PROGRESS (owner notified, request genuinely pending). - Per spec, CONTROL_STATUS without GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER describes only the emitting component; stop applying it to system-level control state. - A takeover request arriving on a non-active vehicle was ACKed and silently dropped (the popup is bound to the active vehicle). Show an app-level message directing the operator to that vehicle. Also: ignore forwarded releases (param1 != 1) instead of raising a takeover popup; use the sanitized timeout for the local request lockout; fix the received-request timeout fallback which mixed seconds into a milliseconds property (a param3=0 request would auto-close its popup in ~10 ms); refresh stale param5/comment references and drop an unused QML property binding. --- src/Toolbar/GCSControlIndicator.qml | 9 ++-- src/Vehicle/GCSControlManager.cc | 75 +++++++++++++++++++++++------ src/Vehicle/GCSControlManager.h | 1 + 3 files changed, 63 insertions(+), 22 deletions(-) diff --git a/src/Toolbar/GCSControlIndicator.qml b/src/Toolbar/GCSControlIndicator.qml index 7b977ab9f733..e16a631a9d43 100644 --- a/src/Toolbar/GCSControlIndicator.qml +++ b/src/Toolbar/GCSControlIndicator.qml @@ -16,7 +16,6 @@ Item { property bool showIndicator: gcsControlManager && gcsControlManager.firstControlStatusReceived property var sysidInControl: gcsControlManager ? gcsControlManager.sysidInControl : 0 property var secondaryGCSList: gcsControlManager ? gcsControlManager.secondaryGCSList : [] - property bool gcsControlStatusFlags_SystemManager: gcsControlManager ? gcsControlManager.gcsControlStatusFlags_SystemManager : false property bool gcsControlStatusFlags_TakeoverAllowed: gcsControlManager ? gcsControlManager.gcsControlStatusFlags_TakeoverAllowed : false property Fact requestControlAllowTakeoverFact: QGroundControl.settingsManager.flyViewSettings.requestControlAllowTakeover property bool requestControlAllowTakeover: requestControlAllowTakeoverFact.rawValue @@ -44,7 +43,7 @@ Item { property bool outdoorPalette: qgcPal.globalTheme === QGCPalette.Light // Used by control request popup, when other GCS ask us for control - property var receivedRequestTimeoutMs: QGroundControl.settingsManager.flyViewSettings.requestControlTimeout.defaultValue // Use this as default in case something goes wrong. Usually it will be overriden on onRequestOperatorControlReceived + property var receivedRequestTimeoutMs: QGroundControl.settingsManager.flyViewSettings.requestControlTimeout.defaultValue * 1000 // Use this as default in case something goes wrong. Usually it will be overriden on onRequestOperatorControlReceived (defaultValue is in seconds, this is in ms) property var requestSysIdRequestingControl: 0 property var requestAllowTakeover: false @@ -61,7 +60,7 @@ Item { requestSysIdRequestingControl = sysIdRequestingControl requestAllowTakeover = allowTakeover // If request came without request timeout, use our default one - receivedRequestTimeoutMs = requestTimeoutSecs !== 0 ? requestTimeoutSecs * 1000 : QGroundControl.settingsManager.flyViewSettings.requestControlTimeout.defaultValue + receivedRequestTimeoutMs = requestTimeoutSecs !== 0 ? requestTimeoutSecs * 1000 : QGroundControl.settingsManager.flyViewSettings.requestControlTimeout.defaultValue * 1000 // First hide current popup, in case the normal control panel is visible mainWindow.closeIndicatorDrawer() // When showing the popup, the component will automatically start the count down in controlRequestPopup @@ -160,13 +159,11 @@ Item { } // Allow takeover expiration time popup. When a request is received and takeover was allowed, this popup alerts - // that after vehicle::REQUEST_OPERATOR_CONTROL_ALLOW_TAKEOVER_TIMEOUT_MSECS seconds, this GCS will change back to takeover not allowed, as per mavlink specs + // that after gcsControlManager.operatorControlTakeoverTimeoutMsecs, this GCS will change back to takeover not allowed, as per mavlink specs Component { id: allowTakeoverExpirationPopup ToolIndicatorPage { - // Allow takeover expiration time popup. When a request is received and takeover was allowed, this popup alerts - // that after vehicle::REQUEST_OPERATOR_CONTROL_ALLOW_TAKEOVER_TIMEOUT_MSECS seconds, this GCS will change back to takeover not allowed, as per mavlink specs TimedProgressTracker { id: revertTakeoverProgressTracker timeoutSeconds: control.gcsControlManager.operatorControlTakeoverTimeoutMsecs * 0.001 diff --git a/src/Vehicle/GCSControlManager.cc b/src/Vehicle/GCSControlManager.cc index 48441e10bc1e..1ea554d3f7d3 100644 --- a/src/Vehicle/GCSControlManager.cc +++ b/src/Vehicle/GCSControlManager.cc @@ -1,5 +1,6 @@ #include "GCSControlManager.h" #include "Vehicle.h" +#include "MultiVehicleManager.h" #include "VehicleLinkManager.h" #include "MAVLinkProtocol.h" #include "SettingsManager.h" @@ -63,8 +64,8 @@ void GCSControlManager::requestOperatorControl(bool allowOverride, int requestTi 0 // param5: unused (removed from spec) ); - if (requestTimeoutSecs > 0) { - _requestOperatorControlStartTimer(requestTimeoutSecs * 1000); + if (safeRequestTimeoutSecs > 0) { + _requestOperatorControlStartTimer(safeRequestTimeoutSecs * 1000); } } @@ -72,12 +73,7 @@ void GCSControlManager::releaseOperatorControl() { // Releasing control makes any pending request countdown or takeover revert meaningless _timerRevertAllowTakeover.stop(); - _timerRequestOperatorControl.stop(); - disconnect(&_timerRequestOperatorControl, &QTimer::timeout, nullptr, nullptr); - if (!_sendControlRequestAllowed) { - _sendControlRequestAllowed = true; - emit sendControlRequestAllowedChanged(true); - } + _cancelRequestOperatorControlCountdown(); const VehicleTypes::MavCmdAckHandlerInfo_t handlerInfo = {&GCSControlManager::_requestOperatorControlAckHandler, this, nullptr, nullptr}; _vehicle->sendMavCommandWithHandler( @@ -88,7 +84,7 @@ void GCSControlManager::releaseOperatorControl() 0, // param2: Allow takeover (irrelevant for release) 0, // param3: Timeout (irrelevant for release) static_cast(MAVLinkProtocol::instance()->getSystemId()), // param4: GCS sysid - 0 // param5: GCS sysid upper range (0 = single GCS) + 0 // param5: unused (removed from spec) ); } @@ -114,10 +110,37 @@ void GCSControlManager::_requestOperatorControlAckHandler(void* resultHandlerDat return; } - if (ack.result == MAV_RESULT_ACCEPTED) { + switch (ack.result) { + case MAV_RESULT_ACCEPTED: qCDebug(GCSControlManagerLog) << "Operator control request accepted"; - } else { - qCDebug(GCSControlManagerLog) << "Operator control request rejected"; + break; + case MAV_RESULT_FAILED: + case MAV_RESULT_IN_PROGRESS: + // The owner was notified and the request is genuinely pending vehicle-side, so keep the + // request-lockout countdown running until it is granted or times out. + qCDebug(GCSControlManagerLog) << "Operator control request pending owner response"; + break; + case MAV_RESULT_DENIED: + // Requester is not an authorized operator of the vehicle, so nothing is pending + // vehicle-side. Cancel the local countdown so the UI stops showing a bogus wait. + manager->_cancelRequestOperatorControlCountdown(); + QGC::showAppMessage(tr("Control request denied: this ground station is not an authorized operator of vehicle %1").arg(manager->_vehicle->id())); + break; + default: + // Any other rejection (unsupported, temporarily rejected, ...) is not pending either. + manager->_cancelRequestOperatorControlCountdown(); + QGC::showAppMessage(tr("Control request rejected by vehicle (%1)").arg(static_cast(ack.result))); + break; + } +} + +void GCSControlManager::_cancelRequestOperatorControlCountdown() +{ + _timerRequestOperatorControl.stop(); + disconnect(&_timerRequestOperatorControl, &QTimer::timeout, nullptr, nullptr); + if (!_sendControlRequestAllowed) { + _sendControlRequestAllowed = true; + emit sendControlRequestAllowedChanged(true); } } @@ -144,12 +167,17 @@ void GCSControlManager::handleControlStatus(const mavlink_message_t& message) mavlink_control_status_t controlStatus; mavlink_msg_control_status_decode(&message, &controlStatus); - if (controlStatus.flags & GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER) { - // This component manages GCS control of the whole system. Operator control - // commands must be addressed to it, which is not necessarily the autopilot - _operatorControlCompId = message.compid; + if (!(controlStatus.flags & GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER)) { + // Per spec, a CONTROL_STATUS without the SYSTEM_MANAGER flag describes only the emitting + // component, not the system, so it must not drive system-level control state. + qCDebug(GCSControlManagerLog) << "Ignoring component-local CONTROL_STATUS (no SYSTEM_MANAGER flag) from compId" << message.compid; + return; } + // This component manages GCS control of the whole system. Operator control + // commands must be addressed to it, which is not necessarily the autopilot + _operatorControlCompId = message.compid; + bool updateControlStatusSignals = false; if (_gcsControlStatusFlags != controlStatus.flags) { _gcsControlStatusFlags = controlStatus.flags; @@ -218,6 +246,21 @@ void GCSControlManager::handleCommandRequestOperatorControl(const mavlink_messag (void) _vehicle->sendMessageOnLinkThreadSafe(sharedLink.get(), ackMessage); } + // Only a request for control (param1 == 1) raises a takeover prompt. A forwarded release + // (param1 == 0) from a non-ArduPilot stack must not surface a bogus takeover popup. + if (static_cast(commandLong.param1) != 1) { + return; + } + + // The takeover popup is bound to the active vehicle only, so a request arriving on a + // non-active vehicle would otherwise be ACKed and silently dropped. Surface it at the app + // level so the operator knows to switch to that vehicle to respond. + if (MultiVehicleManager::instance()->activeVehicle() != _vehicle) { + QGC::showAppMessage(tr("GCS %1 is requesting control of vehicle %2 — switch to that vehicle to respond") + .arg(static_cast(commandLong.param4)) + .arg(_vehicle->id())); + } + emit requestOperatorControlReceived( static_cast(commandLong.param4), // GCS sysid requesting control static_cast(commandLong.param2), // Allow takeover diff --git a/src/Vehicle/GCSControlManager.h b/src/Vehicle/GCSControlManager.h index 647ed15a2020..9708c5d10383 100644 --- a/src/Vehicle/GCSControlManager.h +++ b/src/Vehicle/GCSControlManager.h @@ -60,6 +60,7 @@ class GCSControlManager : public QObject private: static void _requestOperatorControlAckHandler(void* resultHandlerData, int compId, const mavlink_command_ack_t& ack, VehicleTypes::MavCmdResultFailureCode_t failureCode); void _requestOperatorControlStartTimer(int requestTimeoutMsecs); + void _cancelRequestOperatorControlCountdown(); Vehicle* _vehicle = nullptr; From 2303a2748f01f7cb9b26e1d49e1fc6bb6016ee71 Mon Sep 17 00:00:00 2001 From: davidsastresas Date: Tue, 14 Jul 2026 14:20:04 +0200 Subject: [PATCH 20/23] GCSControlManager: log if 2 compids are claiming SYSTEM_MANAGER flag --- src/Vehicle/GCSControlManager.cc | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Vehicle/GCSControlManager.cc b/src/Vehicle/GCSControlManager.cc index 1ea554d3f7d3..43a25b817224 100644 --- a/src/Vehicle/GCSControlManager.cc +++ b/src/Vehicle/GCSControlManager.cc @@ -175,7 +175,16 @@ void GCSControlManager::handleControlStatus(const mavlink_message_t& message) } // This component manages GCS control of the whole system. Operator control - // commands must be addressed to it, which is not necessarily the autopilot + // commands must be addressed to it, which is not necessarily the autopilot. + // Follow the most recent claimant so a manager that restarts under a new + // compid keeps receiving our requests, but make the switch visible: it can + // also mean two components are claiming SYSTEM_MANAGER, which the spec + // requires integrators to prevent. + if ((_operatorControlCompId != 0) && (message.compid != _operatorControlCompId)) { + qCWarning(GCSControlManagerLog) << "System manager component changed from compId" + << _operatorControlCompId << "to" << message.compid + << "- component restart, or two components claiming GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER?"; + } _operatorControlCompId = message.compid; bool updateControlStatusSignals = false; From 1d59528a25887a1b57fda9d3b4e0d06a5cae9612 Mon Sep 17 00:00:00 2001 From: davidsastresas Date: Wed, 15 Jul 2026 12:44:30 +0200 Subject: [PATCH 21/23] Update mavlink tag version --- cmake/CustomOptions.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/CustomOptions.cmake b/cmake/CustomOptions.cmake index 94d26af3f558..aa6f551a4162 100644 --- a/cmake/CustomOptions.cmake +++ b/cmake/CustomOptions.cmake @@ -95,7 +95,7 @@ option(QGC_ENABLE_QT_VIDEOSTREAMING "Enable QtMultimedia video backend" OFF) # ============================================================================ set(QGC_MAVLINK_GIT_REPO "https://github.com/mavlink/mavlink.git" CACHE STRING "MAVLink repository URL") -set(QGC_MAVLINK_GIT_TAG "c409cf690454db6d3e004bd14173bc6c7ff1e0ff" CACHE STRING "MAVLink repository commit/tag") +set(QGC_MAVLINK_GIT_TAG "362f7e0f8f37cacd79f85b567a8ae6193e8afbd0" CACHE STRING "MAVLink repository commit/tag") set(QGC_MAVLINK_DIALECT "all" CACHE STRING "MAVLink dialect") set(QGC_MAVLINK_VERSION "2.0" CACHE STRING "MAVLink protocol version") From ac70f9313aba7cc3da20846395f431b7cd2ae0ed Mon Sep 17 00:00:00 2001 From: davidsastresas Date: Wed, 15 Jul 2026 17:32:34 +0200 Subject: [PATCH 22/23] GCSControlManager: do not re-prompt if revert popup is active --- src/Vehicle/GCSControlManager.cc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Vehicle/GCSControlManager.cc b/src/Vehicle/GCSControlManager.cc index 43a25b817224..31565e16e176 100644 --- a/src/Vehicle/GCSControlManager.cc +++ b/src/Vehicle/GCSControlManager.cc @@ -261,6 +261,14 @@ void GCSControlManager::handleCommandRequestOperatorControl(const mavlink_messag return; } + // Within our own takeover-allowed window (we just approved a request and the revert timer + // is running) the vehicle grants any takeover without asking, so a repeated or crossed + // request must not re-prompt the operator for consent they already gave. + if (_timerRevertAllowTakeover.isActive()) { + qCDebug(GCSControlManagerLog) << "Suppressing takeover prompt: takeover already allowed (revert timer active)"; + return; + } + // The takeover popup is bound to the active vehicle only, so a request arriving on a // non-active vehicle would otherwise be ACKed and silently dropped. Surface it at the app // level so the operator knows to switch to that vehicle to respond. From 05b46ae76a266541084c5676444847f4133af4ea Mon Sep 17 00:00:00 2001 From: davidsastresas Date: Wed, 15 Jul 2026 18:09:41 +0200 Subject: [PATCH 23/23] Add autotests for OPERATOR_CONTROL protocol implementation --- src/Comms/MockLink/MockLink.cc | 4 + src/Comms/MockLink/MockLink.h | 3 + test/Vehicle/CMakeLists.txt | 3 + test/Vehicle/GCSControlManagerTest.cc | 505 ++++++++++++++++++++++++++ test/Vehicle/GCSControlManagerTest.h | 36 ++ 5 files changed, 551 insertions(+) create mode 100644 test/Vehicle/GCSControlManagerTest.cc create mode 100644 test/Vehicle/GCSControlManagerTest.h diff --git a/src/Comms/MockLink/MockLink.cc b/src/Comms/MockLink/MockLink.cc index 46357f841282..2897788c9697 100644 --- a/src/Comms/MockLink/MockLink.cc +++ b/src/Comms/MockLink/MockLink.cc @@ -1733,6 +1733,10 @@ void MockLink::_handleCommandLong(const mavlink_message_t &msg) _handleTakeoff(request); commandResult = MAV_RESULT_ACCEPTED; break; + case MAV_CMD_REQUEST_OPERATOR_CONTROL: + // Result is settable so tests can exercise the pending (FAILED) vs rejected paths + commandResult = _requestOperatorControlResult; + break; case MAV_CMD_MOCKLINK_ALWAYS_RESULT_ACCEPTED: // Test command which always returns MAV_RESULT_ACCEPTED commandResult = MAV_RESULT_ACCEPTED; diff --git a/src/Comms/MockLink/MockLink.h b/src/Comms/MockLink/MockLink.h index 544feb4bc9b5..111f2e1f5c35 100644 --- a/src/Comms/MockLink/MockLink.h +++ b/src/Comms/MockLink/MockLink.h @@ -99,6 +99,8 @@ class MockLink : public LinkInterface void clearReceivedMavCommandCounts() { _receivedMavCommandCountMap.clear(); _receivedMavCommandByCompCountMap.clear(); _receivedRequestMessageByCompAndMsgCountMap.clear(); } int receivedMavCommandCount(MAV_CMD command) const { return _receivedMavCommandCountMap.value(command, 0); } int receivedMavCommandCount(MAV_CMD command, int compId) const { return _receivedMavCommandByCompCountMap.value(command).value(compId, 0); } + /// Result acked for MAV_CMD_REQUEST_OPERATOR_CONTROL (defaults to MAV_RESULT_UNSUPPORTED: not implemented) + void setRequestOperatorControlResult(MAV_RESULT result) { _requestOperatorControlResult = result; } int receivedRequestMessageCount(int compId, int messageId) const { return _receivedRequestMessageByCompAndMsgCountMap.value(compId).value(messageId, 0); } void clearReceivedRequestMessageCounts() { _receivedRequestMessageCountMap.clear(); _receivedRequestMessageByCompAndMsgCountMap.clear(); } int receivedRequestMessageCount(uint32_t messageId) const { return _receivedRequestMessageCountMap.value(messageId, 0); } @@ -384,6 +386,7 @@ private slots: bool _paramRequestReadFailureFirstAttemptPending = false; bool _hashCheckNoResponse = false; int _hashCheckRequestCount = 0; + MAV_RESULT _requestOperatorControlResult = MAV_RESULT_UNSUPPORTED; bool _paramRequestListHashCheckSent = false; bool _resetSysAutostartOnParamReset = false; diff --git a/test/Vehicle/CMakeLists.txt b/test/Vehicle/CMakeLists.txt index 563c019d96e3..d653a05c61b8 100644 --- a/test/Vehicle/CMakeLists.txt +++ b/test/Vehicle/CMakeLists.txt @@ -11,6 +11,8 @@ target_sources(${CMAKE_PROJECT_NAME} FTPControllerTest.h FirmwareUpgradeControllerTest.cc FirmwareUpgradeControllerTest.h + GCSControlManagerTest.cc + GCSControlManagerTest.h InitialConnectTest.cc InitialConnectTest.h InitialConnectPeripheralStartupTest.cc @@ -46,6 +48,7 @@ add_qgc_test(APMAirframeComponentControllerTest LABELS Integration Vehicle) add_qgc_test(FTPControllerTest LABELS Integration Vehicle RESOURCE_LOCK TempFiles) add_qgc_test(FTPManagerTest LABELS Integration Vehicle) add_qgc_test(FirmwareUpgradeControllerTest LABELS Unit Vehicle) +add_qgc_test(GCSControlManagerTest LABELS Integration Vehicle) add_qgc_test(InitialConnectTest LABELS Integration Vehicle) add_qgc_test(InitialConnectPeripheralStartupTest LABELS Integration Vehicle) add_qgc_test(MAVLinkLogManagerTest LABELS Integration Vehicle) diff --git a/test/Vehicle/GCSControlManagerTest.cc b/test/Vehicle/GCSControlManagerTest.cc new file mode 100644 index 000000000000..80b6797ea84a --- /dev/null +++ b/test/Vehicle/GCSControlManagerTest.cc @@ -0,0 +1,505 @@ +#include "GCSControlManagerTest.h" + +#include +#include + +#include "MAVLinkLib.h" +#include "MAVLinkProtocol.h" +#include "Vehicle.h" + +namespace { + +mavlink_message_t makeControlStatusMessage(uint8_t systemId, uint8_t senderCompId, uint8_t flags, uint8_t gcsMain, + const QList& secondaryIds) +{ + uint8_t secondary[10] = {}; + for (int i = 0; i < secondaryIds.count() && i < 10; ++i) { + secondary[i] = static_cast(secondaryIds.at(i)); + } + + mavlink_message_t message{}; + (void) mavlink_msg_control_status_pack(systemId, senderCompId, &message, flags, gcsMain, secondary); + return message; +} + +mavlink_message_t makeForwardedRequestOperatorControlMessage(uint8_t systemId, uint8_t senderCompId, float action, + float allowTakeover, float timeoutSecs, + float requestingSysid) +{ + mavlink_message_t message{}; + (void) mavlink_msg_command_long_pack( + systemId, senderCompId, &message, static_cast(MAVLinkProtocol::instance()->getSystemId()), + static_cast(MAVLinkProtocol::getComponentId()), MAV_CMD_REQUEST_OPERATOR_CONTROL, + 0, // confirmation + action, allowTakeover, timeoutSecs, requestingSysid, 0, 0, 0); + return message; +} + +} // namespace + +void GCSControlManagerTest::_controlStatusUpdatesState() +{ + GCSControlManager* const controlManager = vehicle()->gcsControlManager(); + QVERIFY(controlManager); + QCOMPARE(controlManager->firstControlStatusReceived(), false); + + QSignalSpy statusSpy(controlManager, &GCSControlManager::gcsControlStatusChanged); + QVERIFY(statusSpy.isValid()); + + const uint8_t vehicleSysId = static_cast(mockLink()->vehicleId()); + const uint8_t myId = static_cast(MAVLinkProtocol::instance()->getSystemId()); + mockLink()->respondWithMavlinkMessage(makeControlStatusMessage( + vehicleSysId, MAV_COMP_ID_AUTOPILOT1, GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER, myId, {1, 2})); + + QVERIFY_SIGNAL_COUNT_WAIT(statusSpy, 1, TestTimeout::shortMs()); + + QCOMPARE(controlManager->firstControlStatusReceived(), true); + QCOMPARE(controlManager->sysidInControl(), myId); + QCOMPARE(controlManager->secondaryGCSList(), QList({1, 2})); + QCOMPARE(controlManager->gcsControlStatusFlags_SystemManager(), true); + QCOMPARE(controlManager->gcsControlStatusFlags_TakeoverAllowed(), false); +} + +void GCSControlManagerTest::_controlStatusIgnoredWithoutSystemManagerFlag() +{ + GCSControlManager* const controlManager = vehicle()->gcsControlManager(); + QSignalSpy statusSpy(controlManager, &GCSControlManager::gcsControlStatusChanged); + QVERIFY(statusSpy.isValid()); + + const uint8_t vehicleSysId = static_cast(mockLink()->vehicleId()); + + // A component-local CONTROL_STATUS (no SYSTEM_MANAGER flag) must be ignored. Proven by + // sending it first, then a system-level one and observing exactly one state change once + // both have necessarily been processed (messages are delivered in order over one link). + mockLink()->respondWithMavlinkMessage(makeControlStatusMessage(vehicleSysId, MAV_COMP_ID_AUTOPILOT1, 0, 99, {})); + mockLink()->respondWithMavlinkMessage( + makeControlStatusMessage(vehicleSysId, MAV_COMP_ID_AUTOPILOT1, GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER, 5, {})); + + QVERIFY_TRUE_WAIT(controlManager->sysidInControl() == 5, TestTimeout::shortMs()); + QCOMPARE(statusSpy.count(), 1); +} + +void GCSControlManagerTest::_controlStatusTracksTakeoverAllowedFlag() +{ + GCSControlManager* const controlManager = vehicle()->gcsControlManager(); + QSignalSpy statusSpy(controlManager, &GCSControlManager::gcsControlStatusChanged); + QVERIFY(statusSpy.isValid()); + + const uint8_t vehicleSysId = static_cast(mockLink()->vehicleId()); + mockLink()->respondWithMavlinkMessage(makeControlStatusMessage( + vehicleSysId, MAV_COMP_ID_AUTOPILOT1, + GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER | GCS_CONTROL_STATUS_FLAGS_TAKEOVER_ALLOWED, 0, {})); + + QVERIFY_SIGNAL_COUNT_WAIT(statusSpy, 1, TestTimeout::shortMs()); + QCOMPARE(controlManager->gcsControlStatusFlags_TakeoverAllowed(), true); + QCOMPARE(controlManager->sysidInControl(), static_cast(0)); +} + +void GCSControlManagerTest::_controlStatusGatesJoystickSend() +{ + GCSControlManager* const controlManager = vehicle()->gcsControlManager(); + const uint8_t vehicleSysId = static_cast(mockLink()->vehicleId()); + const uint8_t myId = static_cast(MAVLinkProtocol::instance()->getSystemId()); + const uint8_t otherGcsId = (myId == 1) ? 2 : 1; + + QSignalSpy statusSpy(controlManager, &GCSControlManager::gcsControlStatusChanged); + QVERIFY(statusSpy.isValid()); + + // Control held by another GCS: joystick output must be suppressed. + mockLink()->respondWithMavlinkMessage(makeControlStatusMessage( + vehicleSysId, MAV_COMP_ID_AUTOPILOT1, GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER, otherGcsId, {})); + QVERIFY_SIGNAL_COUNT_WAIT(statusSpy, 1, TestTimeout::shortMs()); + + mockLink()->clearReceivedMavlinkMessageCounts(); + vehicle()->sendJoystickDataThreadSafe(0, 0, 0, 0, 0, 0, qQNaN(), qQNaN(), qQNaN(), qQNaN(), qQNaN(), qQNaN(), + qQNaN(), qQNaN()); + QTest::qWait(TestTimeout::shortMs()); + QCOMPARE(mockLink()->receivedMavlinkMessageCount(MAVLINK_MSG_ID_MANUAL_CONTROL), 0); + + // Control reverts to this GCS: joystick output must resume. + statusSpy.clear(); + mockLink()->respondWithMavlinkMessage(makeControlStatusMessage(vehicleSysId, MAV_COMP_ID_AUTOPILOT1, + GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER, myId, {})); + QVERIFY_SIGNAL_COUNT_WAIT(statusSpy, 1, TestTimeout::shortMs()); + + vehicle()->sendJoystickDataThreadSafe(0, 0, 0, 0, 0, 0, qQNaN(), qQNaN(), qQNaN(), qQNaN(), qQNaN(), qQNaN(), + qQNaN(), qQNaN()); + QVERIFY_TRUE_WAIT(mockLink()->receivedMavlinkMessageCount(MAVLINK_MSG_ID_MANUAL_CONTROL) == 1, + TestTimeout::shortMs()); +} + +void GCSControlManagerTest::_controlStatusSecondaryListChangeEmitsSignal() +{ + GCSControlManager* const controlManager = vehicle()->gcsControlManager(); + const uint8_t vehicleSysId = static_cast(mockLink()->vehicleId()); + + QSignalSpy statusSpy(controlManager, &GCSControlManager::gcsControlStatusChanged); + QVERIFY(statusSpy.isValid()); + + mockLink()->respondWithMavlinkMessage(makeControlStatusMessage( + vehicleSysId, MAV_COMP_ID_AUTOPILOT1, GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER, 0, {1, 2, 3})); + QVERIFY_SIGNAL_COUNT_WAIT(statusSpy, 1, TestTimeout::shortMs()); + QCOMPARE(controlManager->secondaryGCSList(), QList({1, 2, 3})); + + // Shrinking the secondary list (e.g. a GCS deregistering) must also be observed. + mockLink()->respondWithMavlinkMessage(makeControlStatusMessage(vehicleSysId, MAV_COMP_ID_AUTOPILOT1, + GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER, 0, {2})); + QVERIFY_SIGNAL_COUNT_WAIT(statusSpy, 2, TestTimeout::shortMs()); + QCOMPARE(controlManager->secondaryGCSList(), QList({2})); + + // Clearing it entirely must be observed too. + mockLink()->respondWithMavlinkMessage( + makeControlStatusMessage(vehicleSysId, MAV_COMP_ID_AUTOPILOT1, GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER, 0, {})); + QVERIFY_SIGNAL_COUNT_WAIT(statusSpy, 3, TestTimeout::shortMs()); + QCOMPARE(controlManager->secondaryGCSList(), QList()); +} + +void GCSControlManagerTest::_requestOperatorControlRejectedByVehicle() +{ + GCSControlManager* const controlManager = vehicle()->gcsControlManager(); + QVERIFY(controlManager->sendControlRequestAllowed()); + + // MockLink's default result for MAV_CMD_REQUEST_OPERATOR_CONTROL is MAV_RESULT_UNSUPPORTED + // (protocol not implemented), which the manager surfaces as a generic rejection. + expectAppMessage(QRegularExpression("Control request rejected by vehicle")); + + mockLink()->clearReceivedMavCommandCounts(); + controlManager->requestOperatorControl(false, 5); + + QVERIFY_TRUE_WAIT(mockLink()->receivedMavCommandCount(MAV_CMD_REQUEST_OPERATOR_CONTROL) == 1, + TestTimeout::shortMs()); + QVERIFY_TRUE_WAIT(controlManager->sendControlRequestAllowed(), TestTimeout::shortMs()); + + verifyExpectedLogMessage(); +} + +void GCSControlManagerTest::_requestOperatorControlPendingKeepsCountdown() +{ + GCSControlManager* const controlManager = vehicle()->gcsControlManager(); + QVERIFY(controlManager->sendControlRequestAllowed()); + + // MAV_RESULT_FAILED means the owner was notified and the request is genuinely pending + // vehicle-side: unlike DENIED, the request-lockout countdown must keep running. + mockLink()->setRequestOperatorControlResult(MAV_RESULT_FAILED); + + QSignalSpy allowedSpy(controlManager, &GCSControlManager::sendControlRequestAllowedChanged); + QVERIFY(allowedSpy.isValid()); + + mockLink()->clearReceivedMavCommandCounts(); + controlManager->requestOperatorControl(false, 5); + + QVERIFY_TRUE_WAIT(mockLink()->receivedMavCommandCount(MAV_CMD_REQUEST_OPERATOR_CONTROL) == 1, + TestTimeout::shortMs()); + QTest::qWait(TestTimeout::shortMs()); + + // Exactly one transition (the lockout on send); a wrongly-cancelled countdown would have + // emitted a second, re-enabling transition after the FAILED ack. + QCOMPARE(allowedSpy.count(), 1); + QCOMPARE(controlManager->sendControlRequestAllowed(), false); +} + +void GCSControlManagerTest::_requestOperatorControlAcceptedKeepsLockoutUntilConfirmed() +{ + GCSControlManager* const controlManager = vehicle()->gcsControlManager(); + const uint8_t vehicleSysId = static_cast(mockLink()->vehicleId()); + const uint8_t myId = static_cast(MAVLinkProtocol::instance()->getSystemId()); + QVERIFY(controlManager->sendControlRequestAllowed()); + + // MAV_RESULT_ACCEPTED means the vehicle granted the request, but the lockout only lifts + // once CONTROL_STATUS actually confirms this GCS is in control - the ack alone is not enough. + mockLink()->setRequestOperatorControlResult(MAV_RESULT_ACCEPTED); + + QSignalSpy allowedSpy(controlManager, &GCSControlManager::sendControlRequestAllowedChanged); + QVERIFY(allowedSpy.isValid()); + + mockLink()->clearReceivedMavCommandCounts(); + controlManager->requestOperatorControl(false, 30); + + QVERIFY_TRUE_WAIT(mockLink()->receivedMavCommandCount(MAV_CMD_REQUEST_OPERATOR_CONTROL) == 1, + TestTimeout::shortMs()); + QTest::qWait(TestTimeout::shortMs()); + + // Exactly one transition so far (the lockout on send); the ACCEPTED ack must not itself + // re-enable requests. + QCOMPARE(allowedSpy.count(), 1); + QCOMPARE(controlManager->sendControlRequestAllowed(), false); + QVERIFY(controlManager->requestOperatorControlRemainingMsecs() > 0); + + mockLink()->respondWithMavlinkMessage(makeControlStatusMessage(vehicleSysId, MAV_COMP_ID_AUTOPILOT1, + GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER, myId, {})); + + QVERIFY_TRUE_WAIT(controlManager->sendControlRequestAllowed(), TestTimeout::shortMs()); + QCOMPARE(allowedSpy.count(), 2); + // QTimer::remainingTime() reports -1 once the countdown has been stopped. + QCOMPARE(controlManager->requestOperatorControlRemainingMsecs(), -1); +} + +void GCSControlManagerTest::_requestOperatorControlLockoutClearedByTakeoverAllowedFlag() +{ + GCSControlManager* const controlManager = vehicle()->gcsControlManager(); + const uint8_t vehicleSysId = static_cast(mockLink()->vehicleId()); + const uint8_t myId = static_cast(MAVLinkProtocol::instance()->getSystemId()); + const uint8_t otherGcsId = (myId == 1) ? 2 : 1; + + mockLink()->setRequestOperatorControlResult(MAV_RESULT_ACCEPTED); + + QSignalSpy statusSpy(controlManager, &GCSControlManager::gcsControlStatusChanged); + QVERIFY(statusSpy.isValid()); + + controlManager->requestOperatorControl(false, 30); + QVERIFY_TRUE_WAIT(mockLink()->receivedMavCommandCount(MAV_CMD_REQUEST_OPERATOR_CONTROL) == 1, + TestTimeout::shortMs()); + QTest::qWait(TestTimeout::shortMs()); + QCOMPARE(controlManager->sendControlRequestAllowed(), false); + + // Control is still held by another GCS, but TAKEOVER_ALLOWED means a request is no longer + // pointless: the lockout must clear even though sysidInControl != this GCS. + mockLink()->respondWithMavlinkMessage(makeControlStatusMessage( + vehicleSysId, MAV_COMP_ID_AUTOPILOT1, + GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER | GCS_CONTROL_STATUS_FLAGS_TAKEOVER_ALLOWED, otherGcsId, {})); + + QVERIFY_SIGNAL_COUNT_WAIT(statusSpy, 1, TestTimeout::shortMs()); + QCOMPARE(controlManager->sysidInControl(), otherGcsId); + QVERIFY_TRUE_WAIT(controlManager->sendControlRequestAllowed(), TestTimeout::shortMs()); +} + +void GCSControlManagerTest::_requestOperatorControlLockoutClearedWhenUncontrolled() +{ + GCSControlManager* const controlManager = vehicle()->gcsControlManager(); + const uint8_t vehicleSysId = static_cast(mockLink()->vehicleId()); + const uint8_t myId = static_cast(MAVLinkProtocol::instance()->getSystemId()); + const uint8_t otherGcsId = (myId == 1) ? 2 : 1; + + mockLink()->setRequestOperatorControlResult(MAV_RESULT_ACCEPTED); + + QSignalSpy statusSpy(controlManager, &GCSControlManager::gcsControlStatusChanged); + QVERIFY(statusSpy.isValid()); + + // Another GCS holds control, takeover not allowed. + mockLink()->respondWithMavlinkMessage(makeControlStatusMessage( + vehicleSysId, MAV_COMP_ID_AUTOPILOT1, GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER, otherGcsId, {})); + QVERIFY_SIGNAL_COUNT_WAIT(statusSpy, 1, TestTimeout::shortMs()); + + controlManager->requestOperatorControl(false, 30); + QVERIFY_TRUE_WAIT(mockLink()->receivedMavCommandCount(MAV_CMD_REQUEST_OPERATOR_CONTROL) == 1, + TestTimeout::shortMs()); + QTest::qWait(TestTimeout::shortMs()); + QCOMPARE(controlManager->sendControlRequestAllowed(), false); + + // The controlling GCS releases: CONTROL_STATUS reports uncontrolled with takeover still + // not allowed. The lockout must clear -- requesting is meaningful again. This is the + // third re-enable condition (in-control / takeover-allowed / uncontrolled) and was the + // original stuck-countdown bug: the old check missed the sysid_in_control == 0 case. + mockLink()->respondWithMavlinkMessage(makeControlStatusMessage( + vehicleSysId, MAV_COMP_ID_AUTOPILOT1, GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER, 0, {})); + QVERIFY_SIGNAL_COUNT_WAIT(statusSpy, 2, TestTimeout::shortMs()); + QVERIFY_TRUE_WAIT(controlManager->sendControlRequestAllowed(), TestTimeout::shortMs()); +} + +void GCSControlManagerTest::_requestOperatorControlLockoutExpiresAfterTimeout() +{ + GCSControlManager* const controlManager = vehicle()->gcsControlManager(); + + // Accepted and never confirmed by CONTROL_STATUS: only the countdown timer should re-enable + // requests. Uses the minimum allowed timeout (FlyViewSettings requestControlTimeout, 3s) to + // keep this bounded. + mockLink()->setRequestOperatorControlResult(MAV_RESULT_ACCEPTED); + + controlManager->requestOperatorControl(false, 3); + QVERIFY(!controlManager->sendControlRequestAllowed()); + + QVERIFY_TRUE_WAIT(controlManager->sendControlRequestAllowed(), TestTimeout::mediumMs()); +} + +void GCSControlManagerTest::_requestOperatorControlDeniedShowsAuthorizationMessageAndClearsLockout() +{ + GCSControlManager* const controlManager = vehicle()->gcsControlManager(); + QVERIFY(controlManager->sendControlRequestAllowed()); + + // MAV_RESULT_DENIED means this GCS is not an authorized operator of the vehicle: unlike a + // pending (FAILED/IN_PROGRESS) result, the lockout must clear immediately and a distinct, + // authorization-specific message must be shown (not the generic rejection message). + mockLink()->setRequestOperatorControlResult(MAV_RESULT_DENIED); + expectAppMessage(QRegularExpression("not an authorized operator of vehicle")); + + mockLink()->clearReceivedMavCommandCounts(); + controlManager->requestOperatorControl(false, 5); + + QVERIFY_TRUE_WAIT(mockLink()->receivedMavCommandCount(MAV_CMD_REQUEST_OPERATOR_CONTROL) == 1, + TestTimeout::shortMs()); + QVERIFY_TRUE_WAIT(controlManager->sendControlRequestAllowed(), TestTimeout::shortMs()); + + verifyExpectedLogMessage(); +} + +void GCSControlManagerTest::_requestOperatorControlDuplicateRequestShowsWaitingMessage() +{ + GCSControlManager* const controlManager = vehicle()->gcsControlManager(); + + // ACCEPTED so the first request's own (later, real) ack stays silent - this test is only + // about the second, duplicate call's immediate synchronous response. + mockLink()->setRequestOperatorControlResult(MAV_RESULT_ACCEPTED); + expectAppMessage(QRegularExpression("Waiting for previous operator control request")); + + mockLink()->clearReceivedMavCommandCounts(); + // Two back-to-back calls with no intervening event-loop turn: the second is guaranteed to + // find the first still pending in Vehicle's command queue and be treated as a duplicate, + // resolved synchronously without a second COMMAND_LONG reaching the wire. + controlManager->requestOperatorControl(false, 5); + controlManager->requestOperatorControl(false, 5); + + verifyExpectedLogMessage(); + QVERIFY_TRUE_WAIT(mockLink()->receivedMavCommandCount(MAV_CMD_REQUEST_OPERATOR_CONTROL) == 1, + TestTimeout::shortMs()); +} + +void GCSControlManagerTest::_startTimerRevertAllowTakeoverRequestsOnExpiry() +{ + GCSControlManager* const controlManager = vehicle()->gcsControlManager(); + const uint8_t vehicleSysId = static_cast(mockLink()->vehicleId()); + const uint8_t myId = static_cast(MAVLinkProtocol::instance()->getSystemId()); + + // This GCS must be the current controller for the revert-to-no-takeover request to fire. + QSignalSpy statusSpy(controlManager, &GCSControlManager::gcsControlStatusChanged); + QVERIFY(statusSpy.isValid()); + mockLink()->respondWithMavlinkMessage(makeControlStatusMessage(vehicleSysId, MAV_COMP_ID_AUTOPILOT1, + GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER, myId, {})); + QVERIFY_SIGNAL_COUNT_WAIT(statusSpy, 1, TestTimeout::shortMs()); + + // ACCEPTED so the auto-issued revert request's own ack stays silent - this test is only + // about the timer actually firing and issuing that request. + mockLink()->setRequestOperatorControlResult(MAV_RESULT_ACCEPTED); + mockLink()->clearReceivedMavCommandCounts(); + controlManager->startTimerRevertAllowTakeover(); + + // operatorControlTakeoverTimeoutMsecs() is a fixed 10s; wait past it for the timer to fire + // and issue a non-override request that reverts "allow takeover" back off. + QVERIFY_TRUE_WAIT(mockLink()->receivedMavCommandCount(MAV_CMD_REQUEST_OPERATOR_CONTROL) == 1, + TestTimeout::longMs()); +} + +void GCSControlManagerTest::_releaseOperatorControlTargetsLearnedCompId() +{ + GCSControlManager* const controlManager = vehicle()->gcsControlManager(); + const uint8_t vehicleSysId = static_cast(mockLink()->vehicleId()); + constexpr uint8_t kSystemManagerCompId = MAV_COMP_ID_ONBOARD_COMPUTER; + + QSignalSpy statusSpy(controlManager, &GCSControlManager::gcsControlStatusChanged); + QVERIFY(statusSpy.isValid()); + mockLink()->respondWithMavlinkMessage( + makeControlStatusMessage(vehicleSysId, kSystemManagerCompId, GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER, 0, {})); + QVERIFY_SIGNAL_COUNT_WAIT(statusSpy, 1, TestTimeout::shortMs()); + + mockLink()->clearReceivedMavCommandCounts(); + controlManager->releaseOperatorControl(); + + // The release must target the component that last sent CONTROL_STATUS, not the autopilot. + // (MockLink always ACKs as the autopilot component regardless of target, so the resulting + // ack is a compId mismatch from Vehicle's perspective and is not asserted on here.) + QVERIFY_TRUE_WAIT(mockLink()->receivedMavCommandCount(MAV_CMD_REQUEST_OPERATOR_CONTROL, kSystemManagerCompId) == 1, + TestTimeout::shortMs()); + QCOMPARE(mockLink()->receivedMavCommandCount(MAV_CMD_REQUEST_OPERATOR_CONTROL, MAV_COMP_ID_AUTOPILOT1), 0); +} + +void GCSControlManagerTest::_systemManagerCompIdChangeAdoptsNewManager() +{ + GCSControlManager* const controlManager = vehicle()->gcsControlManager(); + const uint8_t vehicleSysId = static_cast(mockLink()->vehicleId()); + + QSignalSpy statusSpy(controlManager, &GCSControlManager::gcsControlStatusChanged); + QVERIFY(statusSpy.isValid()); + + // First manager claimant: the autopilot. + mockLink()->respondWithMavlinkMessage( + makeControlStatusMessage(vehicleSysId, MAV_COMP_ID_AUTOPILOT1, GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER, 0, {})); + QVERIFY_SIGNAL_COUNT_WAIT(statusSpy, 1, TestTimeout::shortMs()); + + // The manager role moves to another component: the switch must be adopted (so requests + // keep reaching whoever actually manages control) and warned about (it can also mean two + // components are claiming SYSTEM_MANAGER, which the spec requires integrators to prevent). + // gcs_main changes alongside so the second processing is observable via the status signal. + expectLogMessage("Vehicle.GCSControlManager", QtWarningMsg, QRegularExpression("System manager component changed")); + mockLink()->respondWithMavlinkMessage(makeControlStatusMessage(vehicleSysId, MAV_COMP_ID_ONBOARD_COMPUTER, + GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER, 5, {})); + QVERIFY_SIGNAL_COUNT_WAIT(statusSpy, 2, TestTimeout::shortMs()); + verifyExpectedLogMessage(); + + // Subsequent commands must target the new manager, not the autopilot. + mockLink()->clearReceivedMavCommandCounts(); + controlManager->releaseOperatorControl(); + QVERIFY_TRUE_WAIT( + mockLink()->receivedMavCommandCount(MAV_CMD_REQUEST_OPERATOR_CONTROL, MAV_COMP_ID_ONBOARD_COMPUTER) == 1, + TestTimeout::shortMs()); + QCOMPARE(mockLink()->receivedMavCommandCount(MAV_CMD_REQUEST_OPERATOR_CONTROL, MAV_COMP_ID_AUTOPILOT1), 0); +} + +void GCSControlManagerTest::_forwardedRequestOperatorControlEmitsSignalAndAcks() +{ + GCSControlManager* const controlManager = vehicle()->gcsControlManager(); + const uint8_t vehicleSysId = static_cast(mockLink()->vehicleId()); + + QSignalSpy requestSpy(controlManager, &GCSControlManager::requestOperatorControlReceived); + QVERIFY(requestSpy.isValid()); + + mockLink()->clearReceivedMavlinkMessageCounts(); + mockLink()->respondWithMavlinkMessage(makeForwardedRequestOperatorControlMessage( + vehicleSysId, MAV_COMP_ID_AUTOPILOT1, 1 /* request */, 1 /* allow takeover */, 30 /* timeout secs */, + 7 /* requesting sysid */)); + + QVERIFY_SIGNAL_COUNT_WAIT(requestSpy, 1, TestTimeout::shortMs()); + const QList args = requestSpy.takeFirst(); + QCOMPARE(args.at(0).toInt(), 7); + QCOMPARE(args.at(1).toInt(), 1); + QCOMPARE(args.at(2).toInt(), 30); + + // The takeover notification must always be ACKed (command receipt, per spec; the autopilot + // clears its own pending state on send and does not consume the ack). + QVERIFY_TRUE_WAIT(mockLink()->receivedMavlinkMessageCount(MAVLINK_MSG_ID_COMMAND_ACK) == 1, TestTimeout::shortMs()); +} + +void GCSControlManagerTest::_forwardedRequestSuppressedWhileTakeoverWindowActive() +{ + GCSControlManager* const controlManager = vehicle()->gcsControlManager(); + const uint8_t vehicleSysId = static_cast(mockLink()->vehicleId()); + const uint8_t myId = static_cast(MAVLinkProtocol::instance()->getSystemId()); + + // This GCS holds control and has just approved a takeover (revert timer running). + QSignalSpy statusSpy(controlManager, &GCSControlManager::gcsControlStatusChanged); + QVERIFY(statusSpy.isValid()); + mockLink()->respondWithMavlinkMessage(makeControlStatusMessage(vehicleSysId, MAV_COMP_ID_AUTOPILOT1, + GCS_CONTROL_STATUS_FLAGS_SYSTEM_MANAGER, myId, {})); + QVERIFY_SIGNAL_COUNT_WAIT(statusSpy, 1, TestTimeout::shortMs()); + controlManager->startTimerRevertAllowTakeover(); + + QSignalSpy requestSpy(controlManager, &GCSControlManager::requestOperatorControlReceived); + QVERIFY(requestSpy.isValid()); + + // A repeated/crossed request arrives while the window is open: the vehicle grants any + // takeover without asking during this window, so the operator must not be re-prompted + // for consent they already gave. The notification is still ACKed (command receipt). + mockLink()->clearReceivedMavlinkMessageCounts(); + mockLink()->respondWithMavlinkMessage(makeForwardedRequestOperatorControlMessage( + vehicleSysId, MAV_COMP_ID_AUTOPILOT1, 1 /* request */, 0, 30, 7)); + + QVERIFY_TRUE_WAIT(mockLink()->receivedMavlinkMessageCount(MAVLINK_MSG_ID_COMMAND_ACK) == 1, TestTimeout::shortMs()); + QCOMPARE(requestSpy.count(), 0); +} + +void GCSControlManagerTest::_forwardedReleaseOperatorControlDoesNotEmitSignal() +{ + GCSControlManager* const controlManager = vehicle()->gcsControlManager(); + const uint8_t vehicleSysId = static_cast(mockLink()->vehicleId()); + + QSignalSpy requestSpy(controlManager, &GCSControlManager::requestOperatorControlReceived); + QVERIFY(requestSpy.isValid()); + + mockLink()->clearReceivedMavlinkMessageCounts(); + mockLink()->respondWithMavlinkMessage( + makeForwardedRequestOperatorControlMessage(vehicleSysId, MAV_COMP_ID_AUTOPILOT1, 0 /* release */, 0, 0, 7)); + + // A forwarded release is still ACKed, but must not raise a takeover prompt. + QVERIFY_TRUE_WAIT(mockLink()->receivedMavlinkMessageCount(MAVLINK_MSG_ID_COMMAND_ACK) == 1, TestTimeout::shortMs()); + QCOMPARE(requestSpy.count(), 0); +} + +UT_REGISTER_TEST(GCSControlManagerTest, TestLabel::Integration, TestLabel::Vehicle) diff --git a/test/Vehicle/GCSControlManagerTest.h b/test/Vehicle/GCSControlManagerTest.h new file mode 100644 index 000000000000..65d4057425bd --- /dev/null +++ b/test/Vehicle/GCSControlManagerTest.h @@ -0,0 +1,36 @@ +#pragma once + +#include "BaseClasses/VehicleTest.h" +#include "GCSControlManager.h" + +class GCSControlManagerTest : public VehicleTestNoInitialConnect +{ + Q_OBJECT + +public: + explicit GCSControlManagerTest(QObject* parent = nullptr) : VehicleTestNoInitialConnect(parent) + { + setAutopilotType(MAV_AUTOPILOT_INVALID); + } + +private slots: + void _controlStatusUpdatesState(); + void _controlStatusIgnoredWithoutSystemManagerFlag(); + void _controlStatusTracksTakeoverAllowedFlag(); + void _controlStatusGatesJoystickSend(); + void _controlStatusSecondaryListChangeEmitsSignal(); + void _requestOperatorControlRejectedByVehicle(); + void _requestOperatorControlPendingKeepsCountdown(); + void _requestOperatorControlAcceptedKeepsLockoutUntilConfirmed(); + void _requestOperatorControlLockoutClearedByTakeoverAllowedFlag(); + void _requestOperatorControlLockoutClearedWhenUncontrolled(); + void _requestOperatorControlLockoutExpiresAfterTimeout(); + void _requestOperatorControlDeniedShowsAuthorizationMessageAndClearsLockout(); + void _requestOperatorControlDuplicateRequestShowsWaitingMessage(); + void _startTimerRevertAllowTakeoverRequestsOnExpiry(); + void _releaseOperatorControlTargetsLearnedCompId(); + void _systemManagerCompIdChangeAdoptsNewManager(); + void _forwardedRequestOperatorControlEmitsSignalAndAcks(); + void _forwardedRequestSuppressedWhileTakeoverWindowActive(); + void _forwardedReleaseOperatorControlDoesNotEmitSignal(); +};