common: add MAV_CMD_DO_UPGRADE command

[TSC21]

Context

Today's UAS usually come with several different components with which one is able to interact, program and configure. Most of today's system also have a onboard/companion computer capable of executing different tasks. One of these tasks is the ability of upgrading the other system components with the execution of appropriate scripting routines. Although this can be controlled by direct access to the computer (ex: using SSH), the ability of doing the same using the ground station exponentially increases the user experience.

Approach

In order to be able to start, cancel or even restart an upgrade process in a UAS sub-system, MAV_CMD_DO_UPGRADE was added. This command uses 3 params to direct usage plus a WIP param field. First param basically uses the MAV_COMPONENT enumeration in order to be able to identify which system component is target for the upgrade. Second param identifies the action. Third param is used to set if the component is to be rebooted or not after the action is executed. Finally, the WIP param is used to set the rate of the COMMAND_ACK with MAV_RESULT_IN_PROGRESS set, together with the progress percentage.

[TSC21]

@julianoes @DanielePettenuzzo @cmic0 FYI.

[hamishwillee]

@auturgy FYI

[auturgy]

I’d like to see a better description of how this is intended to be implemented, as it isn’t clear how it is known that an update is available or required, how it is fetched etc. None of that specifically matters for this message when considered in isolation, however are we accidentally designing a component management microservice piecemeal without taking a step back and doing it wholistically? Does this command only relate to mavlink components? I assume so. Can something without a component ID be upgraded? Yes: we do this with UAVCAN devices already, but not via mavlink. Should we expressly permit or forbid that within the mavlink spec? I don’t know. At the very least it seems that there should some mention of the relationship with COMPONENT_INFORMATION, or an explicit statement that this proposed message is independent from other Component management mechanisms.

…

On 22 May 2020, at 7:15 pm, Nuno Marques ***@***.***> wrote:  @TSC21 commented on this pull request. In message_definitions/v1.0/common.xml: > @@ -2037,6 +2037,17 @@ <param index="6">Reserved, send 0</param> <param index="7">WIP: ID (e.g. camera ID -1 for all IDs)</param> </entry> + <entry value="247" name="MAV_CMD_DO_UPGRADE" hasLocation="false" isDestination="false"> + <wip/> + <description>Request the upgrade of system components. The command can be used to start a specific upgrade process that is controlled by a component of the system that has the authority and the capability of upgrading other components, or even itself (ex: an onboard computer). For a progress update, COMMAND_ACK can be continuosly sent with result MAV_RESULT_IN_PROGRESS and the progress percentage.</description> + <param index="1" label="Component ID" enum="MAV_COMPONENT">Target component identifier to which the upgrade is aimed for. If set to 0, the command is directed to all components.</param> + <param index="2" label="Action" minValue="0" maxValue="2" increment="1">0: Start component upgrade, 1: Cancel component upgrade, 2: Restart component upgrade.</param> + <param index="3" label="Reboot" minValue="0" maxValue="1" increment="1">0: Do not reboot component after the action is executed, 1: Reboot component after the action is executed.</param> + <param index="4">Reserved, send 0</param> + <param index="5">Reserved, send 0</param> + <param index="6">Reserved, send 0</param> + <param index="7">WIP: upgrade progress report rate (can be used for more granular control).</param> @hamishwillee I think I have address your concerns in some way. I updated the commit. Can you recheck? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[hamishwillee]

Thanks @auturgy

Good questions. As is, yes this can only update something with a component ID (unless we trigger an update for "everything").

My two bits

• I think it is reasonable you might want to update something that is non-mavlink. Either way should be made clear.
• Component IDs are already a slight concern because they aren't guaranteed to be "as they are assigned". I say slight, because as this is designed the receiving "updater" can know everything about the system
• This design that assumes we want to trigger an update by sending a message to an updating system that has other mechanisms for knowing how to go about doing an update. I.e you could really call this "do something" and feed in a number for that something.
• Yes, if we stick with this approach we should comment that it is independent of other component management systems.

@TSC21 I know this is a simple design initially designed just to allow companion to update flight controller and it serves that purpose. It might be just what we need. I agree with James we should at least do the due diligence of seeing if it can be made more flexible to avoid having to redesigned in future. Any chance you can attend the mavlink meeting tomorrow so we can sort out any details in person?

So brainstorming .... the COMPONENT_INFORMATION system allows us to query for metadata. So we could query individual components about what updates they support and where updates are stored. Or more likely we could use it only for "updator components". They could then list the updateable components and IDs used to update them (even if they are not mavlink), along with urls for checking update status and downloading new update packages etc. We'd still trigger the update via a command.

[hamishwillee]

Hi @TSC21

Dev call had a chat; we have no concrete suggestions that should block this going through; but it is WIP, so that might change :-)

Looking at this I think the only problem is the cancel and restart sequence - IMO the cancel should result in an accepted response - you're accepting the command to cancel not failing the original command.
Similarly the restart command would be followed by the normal "first time" in progress COMMAND_ACK sequence.

So perhaps reword description as:

Request a target system to start, cancel, or restart upgrade of a component (or components). For example, the command might be sent to a companion computer to cause it to upgrade a connected flight controller. The system doing the upgrade will report progress using the normal command protocol sequence (COMMAND_ACK regularly sent with result=MAV_RESULT_IN_PROGRESS, followed by a final result of MAV_RESULT_ACCEPTED or MAV_RESULT_FAILED). The operation can be cancelled, in which case the updating system would send COMMAND_ACK with MAV_RESULT_ACCEPTED. The operation can be restarted, in which case the updating system should respond with progress updates (as though it had a new message).

Hope that makes sense. If it doesn't do what you need, then let me know.

[TSC21]

Hi @TSC21

Dev call had a chat; we have no concrete suggestions that should block this going through; but it is WIP, so that might change :-)

Looking at this I think the only problem is the cancel and restart sequence - IMO the cancel should result in an accepted response - you're accepting the command to cancel not failing the original command.
Similarly the restart command would be followed by the normal "first time" in progress COMMAND_ACK sequence.

So perhaps reword description as:

Request a target system to start, cancel, or restart upgrade of a component (or components). For example, the command might be sent to a companion computer to cause it to upgrade a connected flight controller. The system doing the upgrade will report progress using the normal command protocol sequence (COMMAND_ACK regularly sent with result=MAV_RESULT_IN_PROGRESS, followed by a final result of MAV_RESULT_ACCEPTED or MAV_RESULT_FAILED). The operation can be cancelled, in which case the updating system would send COMMAND_ACK with MAV_RESULT_ACCEPTED. The operation can be restarted, in which case the updating system should respond with progress updates (as though it had a new message).

Hope that makes sense. If it doesn't do what you need, then let me know.

Does make sense! Thanks!

[hamishwillee]

@TSC21 I've improved the description, and will now merge.

Note that param 7 will need to be updated before WIP tag is removed on command - to add label and to remove the WIP message. FWIW I hope you remove this altogether - I feel that progress update rates should be managed by the system emitting them and more be defined by MAVLink itself.