Skip to content
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

Use MAV_CMD_DO_SET_MISSION_CURRENT for APMFirmwarePlugin #12285

Merged

Conversation

peterbarker
Copy link
Contributor

Send the mavlink command MAV_CMD_DO_SET_MISSION_CURRENT in place of the old mavlink message MISSION_SET_CURRENT. If "UNSUPPORTED" is received from the autopilot then send the old mavlink message.

The mavlink command has existed since 2019.

The mavlink message has been deprecated since Aug 2022.

ArduPilot has supported the command since 4.1.0 (2021)

Adds infrastructure which should be reused for (eg.) the DO_REPOSITION code.

…ability code

a relatively simple wrapper around sendMavCommandWithHandler, this wrapper allows you to create a lambda function as part of the invocation.

If the flight stack has never supported the command then the lambda function is called.

Otherwise, one of three actions is taken:
 - if an UNSUPPORTED response has been received from the autopilot then the lambda function is called
 - if the command has previously been ACCEPTED then sendMavCommand is called with the appopriate arguments
 - otherwise the command is issued to the autopilot, but a special response handler used to register the result as ACCEPTED or UNSUPPORTED (so the first two actions can later be taken)
falls back to the old "mission_set_current" message if the command is not supported
@@ -986,14 +994,17 @@ private slots:

static void _rebootCommandResultHandler(void* resultHandlerData, int compId, const mavlink_command_ack_t& ack, MavCmdResultFailureCode_t failureCode);

// send mavlink command (deprecated since Aug 2022)
Q_INVOKABLE void setCurrentMissionSequenceWithMissionSetCurrent(int seq);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would make this non-invokable and private. I don't think we want anyone using this except for the lambda fallback case. I might put lambdaFallback in the name to steer people away from using it in other Vehicle code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've pushed up a patch on top which simply inlines the function into the lambda itself. I was avoiding that to reduce the patch size - but it does neaten things up a bit.

Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, thanks Peter. I like that much better.

outcome of review; a separate function may be confusing to future users of these methods
@DonLakeFlyer DonLakeFlyer merged commit 796f903 into mavlink:master Jan 8, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants