Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add custom action plugin #1581

Closed
wants to merge 5 commits into from
Closed

Conversation

TSC21
Copy link
Member

@TSC21 TSC21 commented Oct 25, 2021

A custom action is an action that can be executed in a waypoint which is not standardized or handled by MAVLink and its microservices. This action is triggered by the autopilot with a command that gets sent to the onboard/mission computer, and the mission computer then handles the execution of that same action through the plugin.

On the reception of that command, the mission computer process running a MAVSDK instance with the custom_action plugin reads the custom action ID it's supposed to execute, fetches the definition of the custom action sequence with that specific ID from a JSON file, and then loads the definition to the action to the client side, where it then executes a script, a command, or a sequence of staged scripts/commands. During this process, it sends also progress status updates to the autopilot so this is aware that the action is being processed - in case of a timeout, the autopilot is supposed to continue the mission.

For now. MAV_CMD_WAYPOINT_USER_1 is being used as the command sent from the autopilot (PX4-enabled) with the information to start the action. param1 is filled with the Action ID while the param2 is filled with the Action Timeout. A follow-up to this implementation will look to renable and use mavlink/mavlink#1516 with the appropriate modifications.

A high-level action can be composed by a sequence of stages which might result on a sequence of MAVLINK commands that can be sent to both the autopilot and/or to a specific payload, or the execution of scripts. An action can also just trigger a single global scripts, instead of being composed by stages. The transition between stages can be controlled by the StateTransitionCondition enum, which can go from a timeout, to a return value from a script itself. Similar conditions apply to consider an action as complete, valued by the ActionCompleteCondition enum.

An example JSON structure defining some actions can be found under src/integration_tests/test_data/ dir, on the custom_action.json file.

The following sequence diagram explains the functionality of the plugin:
custom_action_plugin_sequence

Requires to be merged:

…de WRT to that position

This allows to run the tests with PX4 SITL with the vehicle spawning in a different position from the default in PX4.
@TSC21
Copy link
Member Author

TSC21 commented Oct 25, 2021

The integration tests should be run against the master branch so this can be tested. But we don't have yet a master branch PX4 container to test against.


Update: this can be tested against the following PX4 branch - https://github.com/PX4/PX4-Autopilot/tree/pr-support_custom_action_waypoints using:

$ build/default/src/integration_tests/integration_tests_runner --gtest_filter="SitlTest.CustomActionMission"
$ build/default/src/integration_tests/integration_tests_runner --gtest_filter="SitlTest.CustomAction"

@hamishwillee
Copy link
Collaborator

hamishwillee commented Oct 27, 2021

@TSC21 So to paraphrase (Please confirm).

  1. An autopilot is running a mission and it gets to a MAV_CMD_CUSTOM_ACTION.
  2. The autopilot emits a long running command protocol command (which one?) to tell the companion to run a custom action.
  3. The mavsdk client ultimately gets the command: it requests a json file for the command. It doesn't know or care where that json file comes from.
  4. The MAVSDK client understands the json file and is able to interpret this into a set of actions. These might be commands to do something like trigger servos etc, or run something on the companion computer. Upshot, the client then executes these commands in some kind of sequence.
  5. The client is required to provide progress notifications as it executes the sequence. These get translated to command progress updates to the long running command sent by the autopilot.
  6. When the sequence of operations in the JSON file is complete, the SDK client would trigger the final completion of the long running command.
  7. On completion of the long running command the mav_cmd in the custom action would complete, and the mission would continue.

Is that about right?

So from a MAVLink-only point of view it looks like this:

  1. An autopilot is running a mission and it gets to a MAV_CMD_CUSTOM_ACTION.
  2. Autopilot emits a long running command protocol command to tell the companion to run a custom action.
    • What command is emitted? The same one - a MAV_CMD_CUSTOM_ACTION?
    • How does the autopilot know the destination where the command should be sent?
    • I presume the action is just a number?
    • How does an autopilot emit the command when it only supports the server side of command handling? (or does it support both)?
  3. The recipient gets the command. It does "stuff", sending back regular progress updates to the long running command.
  4. The recipient completes the long running command.
  5. The Autopilot restarts the mission.

How are timeouts handled? I.e. what happens if an action can't be completed or if it fails. Is the length open ended? What if the final ack is dropped?

My first reaction is that I don't see why something "custom" should be in the standard - it seems to be what MAV_CMD_WAYPOINT_USER_1 is for: here be dragons.
further, nothing you have mentioned here couldn't be done by including the actions in the mission itself - so what's the point?

Even if you argue that it is worth putting this in MAVLink, what appears in MAVSDK is a specific implementation - inventing the JSON format etc on top. Feels like MAVSDK is diverging from standard behaviour too.

If you want MAV_CMD_CUSTOM_ACTION in the standard it needs to be prototyped and have agreement for it to go into at least two flight stacks. Come along to the MAV call to discuss sometime soon!

@julianoes
Copy link
Collaborator

If you want MAV_CMD_CUSTOM_ACTION in the standard it needs to be prototyped and have agreement for it to go into at least two flight stacks. Come along to the MAV call to discuss sometime soon!

Yes exactly.

@TSC21
Copy link
Member Author

TSC21 commented Oct 27, 2021

@hamishwillee

The autopilot emits a long running command protocol command (which one?) to tell the companion to run a custom action.

The command is exactly MAV_CMD_CUSTOM_ACTION. It's a mission item.

The mavsdk client ultimately gets the command: it requests a json file for the command. It doesn't know or care where that json file comes from.

No it doesn't request anything. It knows exactly what JSON file to read because it's passed as an argument to the plugin. i.e an application running a MAVSDK instance, when loading a plugin, should pass the abs file path of the file so it can be read.

Is that about right?

Other than the comments above, yes all correct you got it right.

What command is emitted? The same one - a MAV_CMD_CUSTOM_ACTION?

Yes.

How does the autopilot know the destination where the command should be sent?

It doesn't. It broadcasts it and whoever is able to process the action will start responding with in progress ACKs.

I presume the action is just a number?

Yes it's param1 on the command.

How does an autopilot emit the command when it only supports the server side of command handling? (or does it support both)?

I am not sure I get the question here. You are considering the autopilot here is just the FMU, correct? Not the FMU+OBC combination I assume. The FMU sends command, if it doesn't get any in_progress ACKs, it timesout and continues the mission.

The Autopilot restarts the mission.

Better word here is "continues".

How are timeouts handled? I.e. what happens if an action can't be completed or if it fails. Is the length open ended? What if the final ack is dropped?

The timeouts are handled in the FMU. Right now for PX4 we set the IN_PROGRESS ACK timeout for 1.5 secs (https://github.com/PX4/PX4-Autopilot/pull/18506/files#diff-b0f4afafd2913feb330ba2f0d33d857255face127274b7bb70ec00c12a65fbc1R240), but this can be discussed. If the action fails to be processed, the custom_action process will send a REJECTED ACK to the FMU, which the autopilot processes to continue the mission. If the action process timesout (time set by param3) in the autopilot side, the autopilot sends a COMMAND_CANCEL to the mission computer side, which it then uses the cancel the action processing (so it doesn't make sense to continue processing), and the mission the continues. If the final ACK is dropped, the autopilot will still continue the mission because the IN_PROGRESS times out.

My first reaction is that I don't see why something "custom" should be in the standard

The way that the command is sent and how the progress is sent, and when the action should be canceled, etc. those are all things that are well defined and should live in the spec. It's not the entire microservice that is "custom". The only thing that is custom is the way the client processes the actions (we use a JSON file + a sort of a state machine, Ardupilot uses Lua scripts). The current custom_action plugin could even run a Lua script if we set it up on the JSON file.

nothing you have mentioned here couldn't be done by including the actions in the mission itself - so what's the point?

What do you mean? I gave an example of a servo control. But an action can be something much more complex that only a script can process, because it's not something that the MAVLink spec is supposed to handle.

what appears in MAVSDK is a specific implementation - inventing the JSON format etc on top. Feels like MAVSDK is diverging from standard behaviour too.

The only thing here specific is the way the action is processed. Same way that we decided to use GRPC to handle server-client comms, I don't see a problem on defining a way of processing scripted waypoins that fall under a "custom action" microservice. MAVSDK IMHO doesn't have standerdize or follow the standard to things that are out of scope of MAVLink (like reading a file, the type of file, etc).

If you want MAV_CMD_CUSTOM_ACTION in the standard it needs to be prototyped and have agreement for it to go into at least two flight stacks. Come along to the MAV call to discuss sometime soon!

It's not about wanting. Is about the right way of doing things. This implementation is already in production and we just now need to make sure that there is a consensus on how this should be upstreamed.

@JonasVautherin
Copy link
Collaborator

The only thing here specific is the way the action is processed. Same way that we decided to use GRPC to handle server-client comms, I don't see a problem on defining a way of processing scripted waypoins that fall under a "custom action" microservice.

Not sure I agree here: MAVSDK uses gRPC independently of any other component. MAVSDK could remove gRPC and use FFI bindings, and no single mavlink component would know about it.

Whereas the json file has to be understood by both sides. So it's not obviously an implementation detail: mavlink could specify how this json file must be written, or it could decide not to specify it.

Probably a nitpick, but just to make sure I understand :-)

@TSC21
Copy link
Member Author

TSC21 commented Oct 28, 2021

mavlink could specify how this json file must be written, or it could decide not to specify it.

We should not specify how the JSON file is written, same as Ardupilot wouldn't want to have MAVLink specify how their Lua scripts are written.

@JonasVautherin
Copy link
Collaborator

I feel like I misunderstood the role of the json file, then. I thought that the json file was a list of steps to be run by the custom action ("send a command", "run a script", ...), where the script is obviously not a standard thing. But I thought that the way the json file describes those stages could potentially be standardized 🤔.

@julianoes
Copy link
Collaborator

@TSC21 are you working on getting this into PX4 and MAVSDK? Otherwise, I'll probably have to close this soon. I'm in the process of cleaning up the PR and issues for the MAVSDK repos.

@TSC21
Copy link
Member Author

TSC21 commented Apr 7, 2022

@TSC21 are you working on getting this into PX4 and MAVSDK? Otherwise, I'll probably have to close this soon. I'm in the process of cleaning up the PR and issues for the MAVSDK repos.

Not at this point. I will need some time (and help) bringing this to the MAVLink spec.

@julianoes
Copy link
Collaborator

@TSC21 ok, I'll close this for now and you can re-open it once it's accepted in mavlink.

Honestly, if we were to simplify it a bit, I'd be in favor of bringing it to the spec. I think there is value in it, however, in my opinion it should be simpler with just a 1 to 1 mapping between a mavlink custom command and one script with one return value. Right now there is logic for a state machine, parameters, multiple commands, etc. which I find quite complex and hard to grasp or explain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants