AP_Param: responses to GCS re failed write-param requests are confusing

[rmackay9]

During the investigation of this Mission Planner issue in which the user is not informed of failures to set the parameter, AP's odd response to parameter write failures is exposed.

When the GCS tries to write to a read-only parameter this is what AP returns (according to @meee1):

1. status text with the failure is returned
2. a response saying the parameter was set to the new value
3. a response saying the parameter has the old value

I actually remember the discussion around this behaviour and the thought was that without "2" some GCSs might get stuck or confused. My guess is that this behaviour actually makes things more confusing for the GCS and we should just remove (2).

All comments welcome.

[meee1]

Having the response showing it didn't change would have a better outcome, and is more accurate as it indicates the value actually never changed.
the mavlink spec also says it should broadcast the actual value
https://mavlink.io/en/services/parameter.html

[WickedShell]

I get stuck in that situation if you don't say it changed, because I have to assume some other component on the system changed it, or it was a routine broadcast of the param value. Because you can upload while streaming params down there are numerous ways to get it coming down with the old value without having succeeded in uploading it. I need a positive confirmation of some form that the command was received, changing it to the new value was the only one I could find that tells you that you achieved the result you wanted. What's the problem that this did to a GCS that is prompting the discussion?

[rmackay9]

@WickedShell, there's a link above to the bad MP behaviour that I've observed when changing a read-only parameter...

[WickedShell]

That looks like a mission planner bug though? If the underlieing parameter value gets changed at any time the tuning screen should just be pulling that in rather then leaving it as it was (arguably you could try and filter to just the things that haven't had user input). If you took out the sending of the requested value then how does mission planner give you a useful indication that it couldn't write the parameter? Surely it either silently drops it (which is bad in the general case/temporary radio drop outs), or it hangs forever trying to write it (which doesn't give you any meaningful error indication here?)

[DonLakeFlyer]

QGC doesn't currently handle the case if the value back doesn't match the value sent. It waits for the value to come back but it only matches on parameter name. It will retry 5 times if it doesn't here back though. So as it stands now QGC won't care either way. Mainly because it's crappy with respect to closing this case. That said I think the spec itself should be updated to say how this should be handled whatever the answer to that is.

[tridge]

I think the unchanged value should be sent

[rmackay9]

@meee1, @DonLakeFlyer,

Tridge has created a PR to change how AP deals with a failure to change the parameter value. I'm happy to test with MP and QGC but we wanted you to be aware of this change.

[DonLakeFlyer]

I should have time to try this out tomorrow. I'm in Seattle so we are pretty much stuck in the house for a while!

[rmackay9]

This is fixed in master so closing. The change will also go out with Copter-4.0.4-rc1