Add support for multi-GCS operation #12410
Conversation
Useful to test several instances of QGC on the same machine, to test multi GCS setups
Davidsastresas
force-pushed
the
PR/multi-GCS-master
branch
from
7865f77 to
0410b5b
Compare
|
|
|
@HTRamsey Yes, it is only meant for testing. I really can not imagine a situation on real world usage where you would want to have 2 GCS instances. As it is a command line argument, I doubt users will boot QGC with this option without noticing in a production GCS. I would really like to leave that option, I don't need multiple QGC instances often, but every now and then it helps to have during development, I think it is handy to have. I checked the SingleApplication link you mentioned, it is very interesting, but I think it is overkill for us, at least for the moment. Thanks! |
|
I haven't look in detail yet, but a few things:
|
My logic to not dim this one, together with gimbal control, is that users might want to eventually leave the panel open momentarily during some part of the operation, and dim is inconvenient on such scenarios.
I pushed a couple off commits addressing the QT_DEBUG matter for controlIndicator.qml and --allow-multiple argument. |
Reasonable. I didn't even know not dimming was an option. Can you make them all not dim by changing main popup in MainRootWindow and then remove the option of dim or not. I think that's better overall.
Make sense |
|
@DonLakeFlyer I applied your latest comments, thanks for the review! |
| @@ -109,4 +109,9 @@ | |||
| <qresource prefix="/gimbal"> | |||
| <file alias="payload.svg">resources/gimbal/payload.svg</file> | |||
| </qresource> | |||
| <qresource prefix="/controlIndicator"> | |||
There was a problem hiding this comment.
Would be more intuitive if there where gcscontrolindicator
| @@ -13,6 +13,7 @@ | |||
| <file alias="TelemetryRSSIIndicator.qml">src/UI/toolbar/TelemetryRSSIIndicator.qml</file> | |||
| <file alias="APMSupportForwardingIndicator.qml">src/UI/toolbar/APMSupportForwardingIndicator.qml</file> | |||
| <file alias="GimbalIndicator.qml">src/UI/toolbar/GimbalIndicator.qml</file> | |||
| <file alias="ControlIndicator.qml">src/UI/toolbar/ControlIndicator.qml</file> | |||
There was a problem hiding this comment.
Same things here: GCSControlIndicator
| @@ -0,0 +1,74 @@ | |||
| import QtQuick | |||
There was a problem hiding this comment.
I would rename this TimedProgressTracker
| @@ -647,6 +651,16 @@ void Vehicle::_mavlinkMessageReceived(LinkInterface* link, mavlink_message_t mes | |||
| _handleMessageInterval(message); | |||
| break; | |||
| } | |||
| case MAVLINK_MSG_ID_CONTROL_STATUS: | |||
| { | |||
There was a problem hiding this comment.
Why the scoping here? There are no additional variables defined in the scoping so it's not needed.
| if (commandLong.target_system != MAVLinkProtocol::instance()->getSystemId()) { | ||
| return; | ||
| } | ||
| if (commandLong.command == MAV_CMD(32100)) { // MAV_CMD_REQUEST_OPERATOR_CONTROL |
There was a problem hiding this comment.
Why the magic number here?
| safeRequestTimeoutSecs = SettingsManager::instance()->flyViewSettings()->requestControlTimeout()->cookedDefaultValue().toInt(); | ||
| } | ||
| sendMavCommand(_defaultComponentId, | ||
| MAV_CMD(32100), // MAV_CMD_REQUEST_OPERATOR_CONTROL |
| } | ||
| sendMavCommand(_defaultComponentId, | ||
| MAV_CMD(32100), // MAV_CMD_REQUEST_OPERATOR_CONTROL | ||
| false, // Don't show errors, as per Mavlink control protocol Autopilot will report result failed prior to forwarding the request to the GCS in control. |
There was a problem hiding this comment.
What happens if there is no response from the vehicle because the message never makes it to the vehicle.
|
The other top level problem with this is that the toolbar indicator dropdown doesn't really match the new style of ui. That's fine for now. I can come in after this is merged in and clean it up. |
This PR adds support for the recently added multi-GCS mavlink subprotocol mavlink/mavlink#2158. This allows graceful change in control ownership on a system with multiple GCS. Please note this subprotocol does not attempt to cover security, it assumes all the operators are in contact between them and they work in a collaborative manner.
For more information about the protocol itself, please read that thread instead. Mavlink docs will be updated shortly, when we confirm all of this looks good.
On this PR, the implementation is as follows:
New top toolbar indicator
This indicator will be populated if the active vehicle is sending the new CONTROL_STATUS message. On this icon we can grasp:
Note this icon will have an animation effect whenever the control status changes, whether it is because of a change in takeover allowed, or because of a change in system in control.
This way, we have some variants:
On this case this GCS (252) is in control, as we see the the sysId label green, and also the aircraft icon and the segment joining label and aircraft.
On this case, this GCS (252) is not in control ( label is white, not green ), and also the aircraft icon and segment joining is white, indicating automatic takeover is not allowed ( we need to ask to the GCS in control first )
On this case it is similar to the above situation, only automatic takeover is allowed ( aircraft icon in green ) so we don't need to request permission to the GCS in control, we can adquire control directly.
Expanded menu after clicking top toolbar indicator
After clicking the top toolbar icon, we see an expanded panel. This panel changes depending on the particular control situation. On this panel, we can:
Control request procedure when takeover is allowed
In the most simple case where we are not in control and takeover is allowed, just clicking "Acquire control" will grant us control of the vehicle.
Note that on this situation, we can choose before acquiring control if we want to allow automatic takeover or not, on the tickbox. If we want to change this after being in control it is possible too, see previous screenshot.
Control request procedure when takeover is not allowed
On this situation, we will have the following panel:
Clicking "send request" will send a control request to the GCS in control, with a timeout specified in "Request timeout (sec)", in this case 7 seconds. This timeout will be sent as a parameter in the command, to syncronize progress indicators in both GCS.
We can not sent another request until that timeout expires. This way, after clicking "send request", we will see the following:
And in the current GCS in control, a popup will appear indicating the request, with a syncronized countdown:
Clicking "Ignore" will just discard the popup, with no consequences, and if we click "allow takeover" the requestor GCS will be able to adquire control directly, as in the previous section "Control request procedure when takeover is allowed". However, we will see the following popup appear:
We have a timeout of 10 seconds after clicking "allow takeover" above. If after those 10 seconds the requestor GCS didn't take control, automatic takeover will be set to disabled again. This will happen regardless of discarding ( clicking Ignore ) on the panel above. This is done as a security measure so operators don't forget they accepted a takeover that was never fully completed.
Aditional comments
A couple of new command line arguments were added, in order to make testing easier:
Using both arguments it is handier to test this in SITL
Testing status
This has been tested in SITL using the following Ardupilot branch ArduPilot/ardupilot#29252
Here is a short video showing this testing:
multi-GCS-demo-2025-02-08_17.12.46.mp4
For awareness @hamishwillee @rmackay9. Also @julianoes might want to take a look at it too, for Px4 support.
Thanks to https://harrisaerial.com and https://www.lincesystems.com for sponsoring this PR, and to @hamishwillee for his wise and valuable support, experience and patience during all the iterations that preceded this work.