Command int long fixes

[hamishwillee]

Following on from discussion in #1982, this updates docs generator to add a note to any command that has either hasDestination=true or isDestingation=true attribute. This renders as a line at the end of the description like this:

Are we OK with the text? More questions to follow; they wouldn't block this, but would result in follow on work.

FYI @sypaq-nexton @peterbarker

[hamishwillee]

@julianoes et al Some question:

I think these are missing hasLocation - right?

• https://mavlink.io/en/messages/common.html#MAV_CMD_DO_GO_AROUND - has altitude but no frame.
• https://mavlink.io/en/messages/common.html#MAV_CMD_DO_MOUNT_CONTROL - has positional info in 5/6 no units or frame

In some cases we need COMMAND_LONG. Specifically, if param 5, 6 can have float data.
Do the following have to be COMMAND_LONG?

• https://mavlink.io/en/messages/common.html#MAV_CMD_DO_SET_ROI_WPNEXT_OFFSET - what are units of the roll, pitch, yaw?
• https://mavlink.io/en/messages/common.html#MAV_CMD_OBLIQUE_SURVEY - are angles of fractions of degrees allowed?
• https://mavlink.io/en/messages/common.html#MAV_CMD_DO_DIGICAM_CONTROL - are 5/6 params integers?
• https://mavlink.io/en/messages/common.html#MAV_CMD_DO_DIGICAM_CONFIGURE - are 5/6 params integers?

Given they have to be command long, can we mark those with requireCommandLong?

What about reserved cases that make the command COMMAND_LONG but don't actually force this. We could remove the entries altogether (changing default to 0), or explicitly change to zero, allowing it to be sent in either: (?)

• https://mavlink.io/en/messages/common.html#MAV_CMD_VIDEO_START_CAPTURE - reserved for NaN - suggest we unreserve NaN for the 5, 6 params.

• https://mavlink.io/en/messages/common.html#MAV_CMD_VIDEO_STOP_CAPTURE- reserved for NaN

• https://mavlink.io/en/messages/common.html#MAV_CMD_AIRFRAME_CONFIGURATION

• https://mavlink.io/en/messages/common.html#MAV_CMD_FIXED_MAG_CAL_YAW - lat, lon not in right place - thoughts?

What do we do for cases where things are not intended as commands or as mission items:

• https://mavlink.io/en/messages/common.html#MAV_CMD_NAV_FENCE_POLYGON_VERTEX_EXCLUSION - can only be used in mission - it has positional information but having the command note silly.
  • Perhaps notCommand and notMission for either case? Or do we ignore the case. I think it kind of makes sense tag like this because you never know something can be used in a mission or a command, just if it can't.

Propose to remove mention of mission_item here:

• https://mavlink.io/en/messages/common.html#MAV_CMD_PAYLOAD_PREPARE_DEPLOY

[nexton-winjeel]

Are we OK with the text? More questions to follow; they wouldn't block this, but would result in follow on work.

@hamishwillee: It's a bit too general. I'd suggest something like:
"As this command specifies a position, it should be sent in a COMMAND_INT (if supported by your flight stack). If sent in a COMMAND_LONG, no frame of reference can be set, and the lat/lon values in param 5/6 are less precise."

[nexton-winjeel]

@hamishwillee: Since you tagged me initially, I'll throw in my 2 cents:

I think these are missing hasLocation - right?

• https://mavlink.io/en/messages/common.html#MAV_CMD_DO_GO_AROUND - has altitude but no frame.
• https://mavlink.io/en/messages/common.html#MAV_CMD_DO_MOUNT_CONTROL - has positional info in 5/6 no units or frame

Yes, I think so. However:

• MAV_CMD_DO_GO_AROUND is a landing command, so the altitude should be above ground level (AGL). Maybe just update the description?
• MAV_CMD_DO_MOUNT_CONTROL: It's already deprecated, so ignore? Does hasLocation imply the location is in params 5, 6, and 7 (whereas this has Altitude in param 4)?

In some cases we need COMMAND_LONG. Specifically, if param 5, 6 can have float data.
Do the following have to be COMMAND_LONG?

• https://mavlink.io/en/messages/common.html#MAV_CMD_DO_SET_ROI_WPNEXT_OFFSET - what are units of the roll, pitch, yaw?
• https://mavlink.io/en/messages/common.html#MAV_CMD_OBLIQUE_SURVEY - are angles of fractions of degrees allowed?
• https://mavlink.io/en/messages/common.html#MAV_CMD_DO_DIGICAM_CONTROL - are 5/6 params integers?
• https://mavlink.io/en/messages/common.html#MAV_CMD_DO_DIGICAM_CONFIGURE - are 5/6 params integers?

Given they have to be command long, can we mark those with requireCommandLong?

As far as I can see, none of the above need to be COMMAND_LONG -- will just lose precision if used in a COMMAND_INT. The only command that I'm aware of that must be a COMMAND_LONG is MAV_CMD_DO_SET_ACTUATOR.

What about reserved cases that make the command COMMAND_LONG but don't actually force this. We could remove the entries altogether (changing default to 0), or explicitly change to zero, allowing it to be sent in either: (?)

• https://mavlink.io/en/messages/common.html#MAV_CMD_VIDEO_START_CAPTURE - reserved for NaN - suggest we unreserve NaN for the 5, 6 params.

• https://mavlink.io/en/messages/common.html#MAV_CMD_VIDEO_STOP_CAPTURE- reserved for NaN

• https://mavlink.io/en/messages/common.html#MAV_CMD_AIRFRAME_CONFIGURATION

"Reserved" doesn't really make sense. I'd suggest changing anything that says "Reserved" to "Empty".

• https://mavlink.io/en/messages/common.html#MAV_CMD_FIXED_MAG_CAL_YAW - lat, lon not in right place - thoughts?

Too late to change? And, as above, does hasLocation imply the location is in params 5, 6, and 7?

What do we do for cases where things are not intended as commands or as mission items:

• https://mavlink.io/en/messages/common.html#MAV_CMD_NAV_FENCE_POLYGON_VERTEX_EXCLUSION - can only be used in mission - it has positional information but having the command note silly.
  • Perhaps notCommand and notMission for either case? Or do we ignore the case. I think it kind of makes sense tag like this because you never know something can be used in a mission or a command, just if it can't.

Yes, MAV_CMD_NAV_FENCE_POLYGON_VERTEX_INCLUSION and MAV_CMD_NAV_FENCE_POLYGON_VERTEX_EXCLUSION should probably be marked up. However, I'd prefer missionOnly=true or isCommand=false over notCommand (prefer not to use negatives in names).

Does this also imply we need another MAV_RESULT value?

I'm not sure there is anything that should be marked as commandOnly (notMission)?

Propose to remove mention of mission_item here:

• https://mavlink.io/en/messages/common.html#MAV_CMD_PAYLOAD_PREPARE_DEPLOY

Sounds good.

[hamishwillee]

@hamishwillee: Since you tagged me initially, I'll throw in my 2 cents:

@sypaq-nexton Seeing as how you are engaged, helpful, and succinct, please do comment.

I think these are missing hasLocation - right?

• https://mavlink.io/en/messages/common.html#MAV_CMD_DO_GO_AROUND - has altitude but no frame.
• https://mavlink.io/en/messages/common.html#MAV_CMD_DO_MOUNT_CONTROL - has positional info in 5/6 no units or frame

Yes, I think so. However:

• MAV_CMD_DO_GO_AROUND is a landing command, so the altitude should be above ground level (AGL). Maybe just update the description?

It is probably AMSL. Moved discussion of this to #1994

• MAV_CMD_DO_MOUNT_CONTROL: It's already deprecated, so ignore? Does hasLocation imply the location is in params 5, 6, and 7 (whereas this has Altitude in param 4)?

We could ignore, but it is still used. Deprecated currently means "superseded in MAVLink but may still be used by other flight stacks" (though I am hoping to add superseded tagging and make deprecated really mean that).

Either way, answer hopefully will fall out of discussion in #1994

In some cases we need COMMAND_LONG. Specifically, if param 5, 6 can have float data.
Do the following have to be COMMAND_LONG?

• https://mavlink.io/en/messages/common.html#MAV_CMD_DO_SET_ROI_WPNEXT_OFFSET - what are units of the roll, pitch, yaw?
• https://mavlink.io/en/messages/common.html#MAV_CMD_OBLIQUE_SURVEY - are angles of fractions of degrees allowed?
• https://mavlink.io/en/messages/common.html#MAV_CMD_DO_DIGICAM_CONTROL - are 5/6 params integers?
• https://mavlink.io/en/messages/common.html#MAV_CMD_DO_DIGICAM_CONFIGURE - are 5/6 params integers?

Given they have to be command long, can we mark those with requireCommandLong?

As far as I can see, none of the above need to be COMMAND_LONG -- will just lose precision if used in a COMMAND_INT. The only command that I'm aware of that must be a COMMAND_LONG is MAV_CMD_DO_SET_ACTUATOR.

What about reserved cases that make the command COMMAND_LONG but don't actually force this. We could remove the entries altogether (changing default to 0), or explicitly change to zero, allowing it to be sent in either: (?)

• https://mavlink.io/en/messages/common.html#MAV_CMD_VIDEO_START_CAPTURE - reserved for NaN - suggest we unreserve NaN for the 5, 6 params.
• https://mavlink.io/en/messages/common.html#MAV_CMD_VIDEO_STOP_CAPTURE- reserved for NaN
• https://mavlink.io/en/messages/common.html#MAV_CMD_AIRFRAME_CONFIGURATION

"Reserved" doesn't really make sense. I'd suggest changing anything that says "Reserved" to "Empty".

Reserved (with default in particular), actually has a purpose. It is the cases that are Empty that really should be changed (everywhere).

There are two reasons for this:

1. Future proofing. If you want to extend a command by applying a meaning to an empty/reserved param then you need a default "do nothing value" so that senders that have not been updated do not accidentally command a change for recipients that have been updated. By default this do-nothing value is zero (i.e. GCS send 0 for empty/reserved values. However what if a value of zero means something for your new extension - i.e. it is a valid setting. In this case you don't want to use zero - perhaps you want to use NaN or UINT_MAX to indicate "not supported". However you have to decide this when you define the command - you can't do it later because the GCS should already be sending 0 or NaN or whatever. We use reserved and default to indicate the preference, with a default of false, and 0, respectively. If the param is not defined it is assumed reserved with value 0.
2. Consistent documentation. Across the commands we use empty, reserved, whatever. Having an attribute means that we can auto document consistently (though getting changes through mavgen has been hard).

Docs on that is here: https://mavlink.io/en/guide/define_xml_element.html#reserved

I'll PR a fix and see what people think

• https://mavlink.io/en/messages/common.html#MAV_CMD_FIXED_MAG_CAL_YAW - lat, lon not in right place - thoughts?

Too late to change? And, as above, does hasLocation imply the location is in params 5, 6, and 7?

I think it might be possible to change, and yes, I think location stuff should be in the place defined by convention. Particularly for isDestination, but also for hasLocation. Conventions make things easier, and I know that this one is relied upon for isDestingation, and may be relied on for hasLocation GCS UI.

Exploring this further in #1995

What do we do for cases where things are not intended as commands or as mission items:

• https://mavlink.io/en/messages/common.html#MAV_CMD_NAV_FENCE_POLYGON_VERTEX_EXCLUSION - can only be used in mission - it has positional information but having the command note silly.

  • Perhaps notCommand and notMission for either case? Or do we ignore the case. I think it kind of makes sense tag like this because you never know something can be used in a mission or a command, just if it can't.

Yes, MAV_CMD_NAV_FENCE_POLYGON_VERTEX_INCLUSION and MAV_CMD_NAV_FENCE_POLYGON_VERTEX_EXCLUSION should probably be marked up. However, I'd prefer missionOnly=true or isCommand=false over notCommand (prefer not to use negatives in names).

Good point. I chose the negative because I want to imply "is supported in a mission" just "makes no sense for a command". missionOnly does the job even better since it implies restricted to missions/not in commands, without implying support in a particular flight stack.

@auturgy This was your point last night. I'll propose missionOnly in XSD.

Note also that I would love to generally have supportedInMissions or supportedInCommands but that would be a can of worms - and not just because it varies across flight stack. The real problem is that we would need much finer granularity - such as support indicators on vehicle type. We haven't even thought about how how some things would work in missions for some vehicle types, and when new ones come along, what do you do. I do have an idea of how to do this with profiles though.

Does this also imply we need another MAV_RESULT value?

I don't think so, or at least it it is an orthogonal discussion.

I think it is reasonable that you get a result indicating you used the wrong command type, but I think it is not so useful to tell the user that they could use that in a mission, and a pain for the implementers to have to check. Better to keep them separate.

I'm not sure there is anything that should be marked as commandOnly (notMission)?

Me either. Most commands do things, and could be send in a mission item to useful effect. Again, by apply missionOnly or commandOnly we're saying "does not make sense in the other context".
We'll add if a specific use case comes along that makes it useful.

Propose to remove mention of mission_item here:

• https://mavlink.io/en/messages/common.html#MAV_CMD_PAYLOAD_PREPARE_DEPLOY

Sounds good.

I added PR for this.

[nexton-winjeel]

Reserved (with default in particular), actually has a purpose. It is the cases that are Empty that really should be changed (everywhere).

Ack. The reason I thought "Reserved" doesn't make sense is that I'm too used to MAVLink2, where changing a field will invalidate the checksums. But it does matter for MAVLink1...

[hamishwillee]

@sypaq-nexton

Ack. The reason I thought "Reserved" doesn't make sense is that I'm too used to MAVLink2, where changing a field will invalidate the checksums. But it does matter for MAVLink1...

It doesn't matter for commands either. The message field name is still param 1, just the content changes.

[hamishwillee]

This has had some weeks to percolate and was essentially pre-agreed, so I am going to merge.

All the other discussions above now split into there own issues/PRs. May need to provide further updates following those resolving.