AP_MISSION: Add missing do nothing commands mavsdk compatability

[trolledbypro]

In support of #24692
Hello,
As I continue to troubleshoot ArduPilot compatibility with MAVSDK Mission module, I noticed that several camera action commands are missing. ArduPilot was rejecting mission sequences generated by MAVSDK that included MAV_CMD_DO_GIMBAL_MANAGER_CONFIGURE and MAV_CMD_IMAGE_STOP_CAPTURE.

After examining the code in the AP_Mount and AP_Camera libraries, I concluded that these two commands do not do anything of importance. My solution to extend MAVSDK compatibility is to add these two commands as mission commands that do nothing for now and then implement the proper functions of the commands as they are implemented in AP_Mount/AP_Camera.

MAV_CMD_DO_GIMBAL_MANAGER_CONFIGURE:
Support for this command was recently added to AP_Mount from PR #23737. This PR added the ability to set who is in primary control of a gimbal to the struct mavlink_control_id.

ardupilot/libraries/AP_Mount/AP_Mount_Backend.h

Lines 270 to 275 in 7d264f1

	// structure holding mavlink sysid and compid of controller of this gimbal
	// see MAV_CMD_DO_GIMBAL_MANAGER_CONFIGURE and GIMBAL_MANAGER_STATUS
	struct {
	uint8_t sysid;
	uint8_t compid;
	} mavlink_control_id;

This struct is not used anywhere except to broadcast who is in control of the gimbal in the gimbal_manager_status message.
Since this struct is not used for actual gimbal control, my interim solution to allow for MAVSDK compatibility is to allow for MAV_CMD_DO_GIMBAL_MANAGER_CONFIGURE to be accepted as a mission command and have it do nothing. In the future, I can try to implement this command like in PR #23737 to allow for the gimbal control struct to be populated (if it is deemed important enough to need to be done now) and definately once gimbal development advances to a point where gimbal control is developed.

MAV_CMD_IMAGE_STOP_CAPTURE:
This command currently not implemented in the AP_Camera library. My interim solution to allow for MAVSDK compatibility is to allow for this mission command to be accepted and do nothing until support is added into AP_Camera.

In terms of testing, the only testing I have done is in ArduPilot SITL is to see if the commands are accepted. I have not tested with real hardware.

Does this sound like a reasonable solution to add MAVSDK compatibility now while allowing for further gimbal/camera development?

[peterbarker]

We have a general policy of not introducing dead code into ArduPilot.

The intent of those commands seems more than "unimportant" to me - how did you come to the conclusion they weren't?

[trolledbypro]

Hello @peterbarker,
I think "unimportant" was a poor choice of words for me to use. What I meant is that these are commands that aren'
t implemented yet, and that would need to be implemented for support with MAVSDK mission module.
Knowing that it is not good practice to integrate dead code, would it be preferable to integrate DO_GIMBAL_MANAGER_CONFIGURE with the same compatibility of the AP_Mount library and then MAV_CMD_IMAGE_STOP_CAPTURE once it is added to AP_Camera?

[khanasif786]

@trolledbypro, These are just some beside code things to consider:

1. you should separate MAV_CMD_DO_GIMBAL_MANAGER_CONFIGURE and MAV_CMD_IMAGE_STOP_CAPTURE in two different PR's.
2. I was just going to implement MAV_CMD_IMAGE_STOP_CAPTURE so if you are having seprate PR than i can add my changes on top of yours only if its seprated.
3. We don't allow merge commits so you need to rebase on master to have the latest changes.

[trolledbypro]

Hello @khanasif786,
I intend to close this PR and do separate PRs for the AP_Mission implementation of the two commands.
Are you intending to implement MAV_CMD_IMAGE_STOP_CAPTURE into the AP_Camera module? I can wait to implement it into AP_Mission until after you are done or we implement into both modules together.
My discord username is: trolledbypro

[rmackay9]

Hi, I wonder why MAVSDK support requires adding support for new mission items. I thought MAVSDK would normally be used for real-time control of a vehicle rather than for creating missions. I can imagine that MAVSDK can create missions.. but I would have thought that would have been a lesser used feature.

The MAV_CMD_DO_GIMBAL_MANAGER_CONFIGURE mission command is near useless I think. We've added real-time support for this command so as to ease QGC development but I really question even the real-time messages usefulness.

MAV_CMD_IMAGE_STOP_CAPTURE is useful and we should definitely add support for that one.

[trolledbypro]

At the moment, with the pull requests I have contributed to MAVSDK and have been approved, the mission module is compatible with ArduPilot save for camera and gimbal control. The mission module is the main area of MAVSDK that is incompatible with ArduPilot. Still lots of testing required to validate my changes to Mission module
For camera control, MAV_CMD_IMAGE_STOP_CAPTURE needs to be implemented for image control to work. This is why I intend to implement MAV_CMD_DO_GIMBAL_MANAGER_CONFIGURE. My intention is to mirror the work @rmackay9 has done in the AP_Gimbal library.
I have another pull request #24701 that is working on allowing the proper NaN support for the reserved parameters of the camera mission commands.
For gimbal control, MAVSDK wants to set itself as in control of the gimbal everytime it senses their is a gimbal. The same PR #24701 is also implementing NaN support for MAV_CMD_DO_GIMBAL_MANAGER_PITCHYAW as MAVSDK sends NaN parameters for that command (I still have work to do to get that PR approved).

[rmackay9]

@trolledbypro,

By the way, thanks very much for working on improving AP's MAVSDK compatibility!