AP_Mount: Only save converted mount parameter if mount was previously set in the first place

[joshanne]

The mount library force configures the mount type on conversion, even if the mount was never configured in the first place.

[rmackay9]

Hi @joshanne,

Thanks for this. Have you noticed any issue with this save? This is actually done on purpose so that we don't run the parameter conversion on each boot.

[joshanne]

Thanks for this. Have you noticed any issue with this save? This is actually done on purpose so that we don't run the parameter conversion on each boot.

If you flash the flight controller then boot, the parameter is immediately considered configured. So if you update to software with a configured defaults.parm the default mount is not reflected.

The conversion probably should only occur if the old MNT_TYPE parameter is non zero and the current mount is not configured && the current mount type is non-zero.

ie. Do not run the conversion if:

• if a defaults file is used then the new parameter is unconfigured but non-zero.
• if a defaults file is not used, and the parameter is non-zero.

What we want to achieve is don't trample the user config if updating from an old version of software.

[joshanne]

At the very least the conversion should only occur if the old mount type is set to a non-zero mount type.

The underlying problem is this conversion runs and writes that mount parameter even if the flight controller only ever saw a later version of software.

[joshanne]

Moving this inside the else ensures that we only write a value to MNT1_TYPE if the previous MNT_TYPE was set to a non zero value.

ie. don't set MNT1_TYPE to a configured value of zero, as this is no longer "default".

[joshanne]

@rmackay9 - updated the changes, comments added to both source and PR.

[joshanne]

I still lean toward the initial commit, because upgrading of the other mount parameters could be perfectly valid if the mount type was not set at the time of the upgrade.

[rmackay9]

@joshanne,

OK, thanks. The existing code is a very common pattern though so I think we should discuss with @tridge (and perhaps others) at the EU dev call later today.

I think the ideal change would actually be to set the MNT1_TYPE parameter to the default value of the parameter instead of 0. I think that would maintain the current behaviour and give you what you want. As far as I know we don't have an easy way to retrieve what the default of a parameter is.

[joshanne]

If the old MNT_TYPE value is zero, it is not configured, and upgrade is not required - so bail.