DO_CHANGE_SPEED applied during mission sticks

[julianoes]

Describe the bug

When you set the speed using DO_CHANGE_SPEED during a mission, I would expect it to reset the speed once you leave the mission mode. That's however not what happens. Instead, the last set cruise speed stays and is applied to Hold and "Go to/reposition" as well.

This happens in v1.13 but didin't use to be the case with v1.12.

@sfuhrer I assume this broke with #18834.

FYI @hamishwillee

To Reproduce

Steps to reproduce the behavior:

1. Start SITL
2. Uploaded mission with a speed change.
3. After speed change, press in map to do a Goto
4. Check speed.

Expected behavior

In my intuition the speed should reset once out of mission mode, and re-applied when going back into mission mode.

[dagar]

This is one of those things that's been a bit ambiguous with different users wanting it both ways.

I personally think it should reset on mode change, but even within that there are questions of when it should reset within a mission (pause and resume, restart mission, etc).

[julianoes]

Yes, for a pause it needs to be re-applied, or all the previous mission items re-played, however we implement it. I'd say it should be similar to camera and gimbal settings getting re-applied which also needs to be implemented.

[dagar]

As a start we should probably differentiate between DO_CHANGE_SPEED in a mission vs command.

[julianoes]

We should, yes. The problem is that the logic is quite convoluted, spread across navigator and flight tasks.

[hamishwillee]

Values set in a mission should not escape the mission ever IMO.

The most common pattern in MAVLink is that a MAV_CMD used in a command protocol sets "system defaults" that are then used for all commands like goto, orbit or whatever (that have a movement component). This would be used as the initial value for a mission unless overridden.
The speed setting then "lives" until over-ridden by the mission. It even persists if you got into HOLD mode and return - because the author of a mission can't know that you might go into hold part way through the mission and plan for that.

The problem here though is the definition of MAV_CMD_DO_CHANGE_SPEED. What I describe above would be "the right thing" if the description said "set the cruise speed" but it does not. It says "set the speed setpoint".
A speed setting like "cruise speed" has a notion of persistence, while a speed setpoint has the semantics of "I'm touching some current value in RAM that might be varied by many things".

So it really depends on what everyone thinks this message does. If everyone thinks it is setting the cruise speed we're fine, yes this is a bug, and can we please implement as I suggested.

[sfuhrer]

I think it should rather be interpreted as "set current speed setpoint", not "default cruise speed". So yes, the speed set in the mission shouldn't be persistent when changing to Loiter mode (go to, Orbit, figure of 8, Hold) and vice-versa. I think this is something we could fairly easily implement, I already did for RTL (it will RTL at the default cruising speed, as it otherwise can be quite dangerous if the user sets a super low speed). Within the same Mode, so e.g. when doing multiple go to, the speed setpoint should be persistent, as it is not linked to one specific go to command (it's a separate command).
Does this make sense? If yes I can look at implementing this.

[julianoes]

@sfuhrer yes that would be great. I think you have the better overview of where/how to fix this than me.

[hamishwillee]

In support of this I have information from mavlink/mavlink#1890 (comment) that ArduPlane treats this setting as an override for the current mode. So if you switch out of the mode the value should revert to whatever is the default for that mode.

This sounds very much like what @sfuhrer is saying - I updated MAVLInk to test this here: mavlink/mavlink#1892

PS. @sfuhrer They are also adding a value for param 2 to allow you to say "in mission" that you want to revert to the default. I think that is a good idea.

[sfuhrer]

Fixed by #21414 and #21503