-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Remove support for MAVLink v1 message protocol #13670
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Gonna sit on this for a couple days to see if anyone comes up with some nasty reason to not remove v1 support |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request removes support for the MAVLink v1 protocol, enforcing MAVLink v2 as the only supported protocol version. When MAVLink v1 traffic is detected on a link, QGroundControl now notifies the user and stops processing that link.
Key Changes:
- Removed protocol version negotiation and tracking from Vehicle and MultiVehicleManager
- Eliminated the PROTOCOL_VERSION request state from the initial connection state machine
- Added MAVLink v1 traffic detection with user notification in LinkInterface
- Simplified RallyPoint and GeoFence support checks by removing protocol version requirements
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Vehicle/Vehicle.h | Removed maxProtoVersion tracking and requestProtocolVersion signal |
| src/Vehicle/Vehicle.cc | Removed protocol version negotiation logic and _setMaxProtoVersion methods |
| src/Vehicle/MultiVehicleManager.h | Removed _requestProtocolVersion slot |
| src/Vehicle/MultiVehicleManager.cc | Removed protocol version negotiation across multiple vehicles |
| src/Vehicle/InitialConnectStateMachine.h | Removed _stateRequestProtocolVersion state and handler |
| src/Vehicle/InitialConnectStateMachine.cc | Removed PROTOCOL_VERSION request logic; now assumes MAVLink v2 support |
| src/Settings/MavlinkSettings.h | Removed requireMatchingMavlinkVersions setting |
| src/Settings/MavlinkSettings.cc | Removed requireMatchingMavlinkVersions declaration |
| src/MissionManager/RallyPointManager.cc | Simplified supported() to only check capability bits |
| src/MissionManager/RallyPointController.cc | Removed requestProtocolVersion signal connection; simplified support check |
| src/MissionManager/GeoFenceManager.cc | Simplified supported() to only check capability bits |
| src/MissionManager/GeoFenceController.cc | Removed requestProtocolVersion signal connection; simplified support check |
| src/MissionManager/MavCmdInfoCommon.json | Removed MAV_CMD_REQUEST_PROTOCOL_VERSION entry |
| src/Comms/MockLink/MockLink.h | Removed _handleRequestMessageProtocolVersion handler |
| src/Comms/MockLink/MockLink.cc | Removed protocol version request handling logic |
| src/Comms/MockLink/MockConfiguration.h | Removed protocol version failure test modes |
| src/Comms/MAVLinkProtocol.h | Removed version tracking and setVersion method |
| src/Comms/MAVLinkProtocol.cc | Replaced version switching with v1 traffic detection; added QTimer include |
| src/Comms/LinkManager.cc | Removed setVersion call during link creation |
| src/Comms/LinkInterface.h | Added reportMavlinkV1Traffic method and tracking flag |
| src/Comms/LinkInterface.cc | Implemented v1 traffic detection with user notification |
| test/Vehicle/InitialConnectTest.cc | Removed protocol version failure test cases |
884f44a to
fada667
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/Vehicle/InitialConnectStateMachine.h:81
- The
_rgProgressWeightsarray has 8 elements but the first comment still references "_stateRequestCapabilities" which was removed. The array indices don't match the state machine functions. The array should have comments matching the actual states:
_stateRequestAutopilotVersion(not "Capabilities")_stateRequestStandardModes_stateRequestCompInfo_stateRequestParameters_stateRequestMission_stateRequestGeoFence_stateRequestRallyPoints_stateSignalInitialConnectComplete
This mismatch could cause confusion when maintaining or debugging the progress tracking logic.
static constexpr const int _rgProgressWeights[] = {
1, //_stateRequestCapabilities
1, //_stateRequestStandardModes
5, //_stateRequestCompInfo
5, //_stateRequestParameters
2, //_stateRequestMission
1, //_stateRequestGeoFence
1, //_stateRequestRallyPoints
1, //_stateSignalInitialConnectComplete
};
128edc0 to
3c75fe8
Compare
Notify user if v1 traffic is detected on link
3c75fe8 to
b6d4594
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
|
|
||
| qCDebug(LinkInterfaceLog) << "_allocateMavlinkChannel" << _mavlinkChannel; | ||
|
|
||
| mavlink_set_proto_version(_mavlinkChannel, MAVLINK_VERSION); // We only support v2 protcol |
Copilot
AI
Nov 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in comment: "protcol" should be "protocol".
| mavlink_set_proto_version(_mavlinkChannel, MAVLINK_VERSION); // We only support v2 protcol | |
| mavlink_set_proto_version(_mavlinkChannel, MAVLINK_VERSION); // We only support v2 protocol |
|
I may be the minority, but I think this is a good step forward. Thank you. |
Uh oh!
There was an error while loading. Please reload this page.