Update schema for MAV_CMD params

[DonLakeFlyer]

This provides detailed information such that a ground station can build ui from it.

Discussion here: mavlink/mavlink#623. This is the first step of getting this in for real.

[DonLakeFlyer]

Oops, this doesn't have everything I need. Need to add enums as well. I'll update.

[DonLakeFlyer]

Updated schema per discussion. This would lead to the following xml example:

               <entry value="18" name="MAV_CMD_NAV_LOITER_TURNS">
                    <description>Loiter around this MISSION for X turns</description>
                    <param index="1" label="Turns" defaultValue="2" decimalPlaces="0">Turns</param>
                    <param index="2">Empty</param>
                    <param index="3" label="Radius" units="m" defaultValue="50" decimalPlaces="2">Radius around MISSION, in meters. If positive loiter clockwise, else counter-clockwise</param>
                    <param index="4" label="Next waypoint start" defaultValues="1" decimalPlaces="0">
                        Forward moving aircraft this sets exit xtrack location: 0 for center of loiter wp, 1 for exit location. Else, this is desired yaw angle
                        <paramEnum paramValue="0">Center</paramEnum>
                        <paramEnum paramValue="1">On Loiter</paramEnum>
                    </param>
                    <param index="5">Latitude</param>
                    <param index="6">Longitude</param>
                    <param index="7">Altitude</param>
               </entry>

I'll clean up the pull (squash and so forth) after we get agreement on this.

[DonLakeFlyer]

@tridge @peterbarker @magicrub Moving discussion to here.

[WickedShell]

@DonLakeFlyer I'm sure I'm coming into this late, but what are the default values for? If a GCS doesn't fill any of those items in that you have a default for the autopilot will read it as 0 instead and not fill it with a default. Or at least that's what I'd assume it would do?

I do like the param inside the command though... If those could be automatically generated in the bindings as MAV_CMD_NAV_LOITER_TURNS_PARAM4 that would be really nice.

[DonLakeFlyer]

The default values tell the GCS what to prime to values in the ui to when a user adds a new one of these items to a mission. It's just a reasonable value to start with.

[WickedShell]

I guess? I'm of the opinion (and have implemented it such) that the values like that should be provided by the GCS and allow the user to set that stuff up. For example those defaults are not very reasonable when connecting to a plane (which uses the same command) and things that are reasonable for one person's fixed wing definitely aren't for anothers). I guess you still have the starting problem until you get input from the user on it, but I don't see how to support the ones well that are interpreted differently for a vehicle with this, or manage the range of "this value is extremely unreasonable for this vehicle type as a starting number"

[DonLakeFlyer]

I'm fine with leaving it out. QGC actually has a hierarchy of overrides for all of this information: https://github.com/mavlink/qgroundcontrol/blob/master/src/MissionManager/MissionCommandTree.h#L28. My expectation is that the base set of info comes from mavlink. And then QGC can override things as needed for vehicle type or firmware differences. I figured I would still need the upper level of override information built into QGC.

[DonLakeFlyer]

Any other comments on this?

[amilcarlucas]

This looks nice. any progress on this ?

[DonLakeFlyer]

FYI: Now that QGC 3.1 is out the door I'll be pushing on this again.

[amilcarlucas]

#47 is an extension of this to ALL Mavlink messages.
mavlink/mavlink#663 implements the changes proposed in #47 into common.xml, ardupilotmega.xml and uAvionix.xml.
These patches have less fields than #7, but they are neverteless a step forward.
If this gets accepted I will extend it to ALL software, Autoquad, paparazzi, minimal, etc

[tridge]

I've merged the smaller set of attributes from Amilcar in #47

@DonLakeFlyer, I'd be happy to merge all of your new attributes too, just to unstick this PR and make for easier work on the much larger task of annotating the XML. Are you OK with that?

[DonLakeFlyer]

I need to update this to remove defaults then i think it will be ok. Let me take another look through since I haven't had my head around this for a while.

[amilcarlucas]

Now that the units information is available upstream, I think that upstream is more receptive to this PR.

[amilcarlucas]

decimal places should probably be signed. this would allow for i.e. both milimeters (-3) and kilometers (3)

[amilcarlucas]

This is a great PR

[amilcarlucas]

decimal places should probably be signed. this would allow for i.e. both milimeters (-3) and kilometers (3)

[amilcarlucas]

Please only allow the units already available in the existing common.xml, ardupilot.xml ... files

[amilcarlucas]

use #139 as a type for the units :)

[amilcarlucas]

Please use #139 as a type for the units :)
We are getting closer at having a proper protocol definition.

[hamishwillee]

@amilcarlucas This seems like such a good idea to still be sitting here. I know some work was done, so we can now add units or enum for most fields, but not for the fields defined in MAV_CMD messages.

What else needs to happen? What is blocking this? What can I do to help?

[WickedShell]

I'd still like to see the defaults come out, and allow a GCS to select reasonable defaults for the stack it's working with.

[amilcarlucas]

Basically what is missing is:

1. Use Improve MAV_CMD parameter information #211
2. Update the XML (basically reopen Proposal: Change MAV_CMD xml format to support UI generation mavlink/mavlink#623 and extend it)
3. We test both changes both locally and on CI.
4. @WickedShell @tridge @DonLakeFlyer @LorenzMeier @dagar review our work
5. It gets merged :)

[amilcarlucas]

Closing this one in favor of #211