Pr multi gcs june2026 ap#46
Open
Davidsastresas wants to merge 8 commits into
Open
Conversation
This was referenced Jun 18, 2026
Owner
|
Unfortunately the suite is now failing with: |
Author
|
I am sorry Peter I didn´t spot this when I made the PR. Do you want me to look at it, or are you already over it? Thanks! |
The operator control handler used _operator_control_sysid (param4, the range minimum) as both the sysid range bound and the gcs_main reported in CONTROL_STATUS. When a range is requested, param4 is the minimum id, not necessarily the requesting GCS. Add _operator_control_primary to track the actual requesting GCS (msg.sysid) separately from the range. get_operator_control_sysid() and sysid_is_primary_operator() now use _operator_control_primary, aligning CONTROL_STATUS.gcs_main with the protocol spec: System ID of GCS in control.
Four fixes to the operator control request handling: - a secondary GCS within the operator control range was auto-granted control without going through the takeover path: sysid_is_gcs() matched any GCS in the range, so the same-GCS re-requesting branch granted control even when takeover was not allowed. Compare against the primary GCS instead, so secondaries must go through the takeover notification flow as the protocol requires - release was likewise accepted from any GCS in the range, letting a secondary strip control from the primary without a takeover; only the primary may release. The sysid_is_gcs() check is kept for the uncontrolled case so a no-op release is still only accepted from a recognized GCS - the takeover notification pending flag was set but never read or cleared, so a request for MSG_OPERATOR_CONTROL_NOTIFICATION outside the queued send path would transmit a stale notification. Replace it with a per-channel pending mask following the statustext pattern: queueing marks all active channels, each channel clears its bit after a successful send, and an ownership change invalidates any notification still queued - param3 of MAV_CMD_REQUEST_OPERATOR_CONTROL is specified as 3 to 60 seconds but was forwarded unchecked into the takeover notification, so an out-of-range requester timeout (including 0 from senders that do not set it) was displayed as-is by the owning GCS countdown
Update the test for the release restriction and timeout clamp (a secondary GCS in the range must be DENIED release; an out-of-range request timeout of 120 is clamped to 60 in the forwarded notification), and fix two reliability problems: - the takeover-notification check listened on self.mav, but waiting for the requester's COMMAND_ACK (run_cmd_int on a non-default connection) drains and discards everything arriving on self.mav, eating the notification before the listen loop starts; the test failed with "Did not receive REQUEST_OPERATOR_CONTROL notification" on every run. Listen on the owner's connection instead, which nothing drains, and assert the notification is addressed to the owner's system id - the operator-control heartbeat timeout is 5 seconds of sim-time but the framework paces its GCS heartbeats in wall-clock time; at the copter default speedup of 100 that is a 10+ sim-second gap between owner heartbeats, so the autopilot cleared ownership mid-test. Pin the test to speedup 4
…SID params Per the model agreed with Hamish on PR#33332 / mavlink#2535: - a control request can no longer redefine owner/secondary membership, which also removes the revert-on-release artifact. Membership is read directly from MAV_GCS_SYSID / MAV_GCS_SYSID_HI (the runtime copy of the range only existed to mirror them and could go stale if the parameters changed while controlled) - param5 (range high) was removed from the spec (mavlink#2535, merged): stop reading packet.x on requests and send 0 in param5 of the forwarded notification. param4 is kept on the notification (requester sysid, read by the GCS receiving it); on requests the requester is identified by the sender sysid, not param4 - MAV_GCS_SYSID_HI > MAV_GCS_SYSID configures multi-owner mode: the requester is validated against the param range and outsiders get MAV_RESULT_DENIED. In single-GCS mode any GCS may request control, subject to the takeover rules - the granted primary may therefore lie outside the configured range in single-GCS mode, so it is now recognized as our GCS while it holds control: its heartbeats are tracked for the operator timeout, its manual control and RC overrides are honored, its traffic holds off the GCS failsafe, and accept_packet() accepts it under GCS_SYSID_ENFORCE (previously such a primary was granted control it could not use, and under enforce its heartbeats were dropped so the grant silently reverted after the operator timeout) - gcs_secondary in CONTROL_STATUS stays all-zero in single-owner mode per the spec; previously the configured GCS's own heartbeats appeared as a phantom secondary whenever it was not the one in control
Update MAV_CMD_REQUEST_OPERATOR_CONTROL for the agreed model: - multi-owner mode is entered via MAV_GCS_SYSID/MAV_GCS_SYSID_HI instead of a range in the request (param5 removed from spec) - out-of-range requester is DENIED in multi-owner mode (request and release) - a granted request does not change gcs_secondary, release does not revert membership, and gcs_secondary stays all-zero in single-owner mode - the forwarded notification carries param5=0 - primary disconnect releases control even while a secondary keeps heartbeating - single-owner handover to an out-of-range GCS is exercised end-to-end under GCS_SYSID_ENFORCE: takeover accepted, commands ACKed, RC override honored from the primary and refused from an in-range non-primary, heartbeats keeping ownership alive across the operator timeout. The primary's heartbeats are sent from a message hook, mirroring the framework's own do_heartbeats(), and enforcement is switched off before context_pop so the unordered parameter restore cannot lock the test GCS out. Verified failing against the pre-fix GCS_MAVLink code and passing with it
Changing MAV_GCS_SYSID/_HI at runtime so the current primary falls outside the configured range deadlocked the vehicle: the multi-owner range check denied the primary's release (and re-request), while its heartbeats kept the operator timeout alive, so control could never be freed. The current primary now bypasses the range validation so it can always release or adjust allow-takeover; a fresh request after it releases is still validated against the range.
Shrink MAV_GCS_SYSID/_HI under the current primary and verify it can still adjust allow-takeover and release; once released and out of range, a fresh request is DENIED.
The disconnect test claimed control releases "even though mav3 keeps heartbeating", but mav3 sent no heartbeats during the wait, so the test passed identically under the old any-in-range-GCS liveness. Send them for real and assert the secondary survives the primary's timeout, pinning that the operator timeout tracks the primary specifically.
Davidsastresas
force-pushed
the
pr-multi-gcs-June2026-AP
branch
from
July 14, 2026 17:28
15ea302 to
e69d52c
Compare
Author
|
I pushed some changes, more info in ArduPilot#33332 (comment) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
More information in ArduPilot#33332 (comment).