Flight task custom

[ChristophTobler]

@MaEtUgR @Stifael

As discussed, I added the CUSTOM state to launch a custom flight task. We use this for logging purposes and to avoid other switching corner cases.
I also moved the command handling into the position controller so that upon failure the behavior is the same for all tasks.

Things that need to be changed/improved in following PRs:

• feedback to commander if a task fails
• getting rid of all the control mode flags -> The flight tasks themselves know if a switch is possible

[ChristophTobler]

@dagar What's up with CI? It seems to fail randomly. Before it passed and now without doing anything it failed...

[dagar]

Those are real failures (memory leaks from not px4 posix not shutting down cleanly), but can be ignored for now. Something changed in #10173.

I'll silence it until it can be properly addressed.

[dagar]

Things that need to be changed/improved in following PRs:

• feedback to commander if a task fails
• getting rid of all the control mode flags -> The flight tasks themselves know if a switch is possible

Before you dive into doing this I would suggest opening an issue to review the requirements and concerning issues. I'm wondering about state machine interaction overall, especially with respect to failsafes.

[dagar]

Could you elaborate on the switching corner cases? It might not be that hard for us to find a solution that cleanly coordinates with commander rather than placing it in a custom unknown state.

[ChristophTobler]

The "old" way would been to create a navigation state for every possible task. And we think this isn't the way to go as we would like to move the code that decides if a task can be performed out of the commander -> every task should know if enough sensor information is available.

@MaEtUgR's first go was (please correct me if I'm wrong):
Just switch task when a vehicle command arrives with a task without switching the navigation state. Add a check within FlightTasks.cpp if the navigation state changed and only start a new task if it changed.

This has the following corner case:
If you e.g. are in position and send an orbit command, the flight task then switches to orbit but the navigation state is still position. If the user then wants to switch to position control, it doesn't switch as the navigation state didn't change...
Plus: When looking at a log afterwards we think it's still in position

[dagar]

My intention isn't to preserve the "old" way, but at the same time we need to be careful not to circumvent anything safety critical. Adding CUSTOM doesn't address the logging issue.

Manual position control mode is vaguely defined, you're free to interpret manual input however you want. I still think there's a way it could be structured such that these exist as states within position control mode (entirely MC side).

In your use case how are you switching into orbit to begin with? Then how are you switching back into position control mode?

[dagar]

This isn't safe, it ignores nearly all failsafes and current system conditions.

[dagar]

At a minimum should be handled like position control mode (could even be a fall through).

[ChristophTobler]

My intention isn't to preserve the "old" way, but at the same time we need to be careful not to circumvent anything safety critical. Adding CUSTOM doesn't address the logging issue.

Well I think it does as we know it's in custom (navigation state is being logged) and we can check the vehicle command msg, right?

Manual position control mode is vaguely defined, you're free to interpret manual input however you want.

I'm not quite sure what you mean by that.

I still think there's a way it could be structured such that these exist as states within position control mode (entirely MC side).

We're open to any other suggestions that don't need too many changes for now.

In your use case how are you switching into orbit to begin with? Then how are you switching back into position control mode?

You have to use the https://github.com/PX4/Firmware/blob/master/msg/vehicle_command.msg#L14 to get into orbit mode. Switching back could be done using the MAV_CMD_DO_SET_MODE as QGC e.g. does it.

[MaEtUgR]

Adding CUSTOM doesn't address the logging issue.

Logging which the task state is inevitable for future development anyways. Adding a custom mode solves the case where you look at the navigation state and think that's the end of the story.

Manual position control mode is vaguely defined, you're free to interpret manual input however you want. I still think there's a way it could be structured such that these exist as states within position control mode (entirely MC side).

That forces all tasks that run within this navigation state to inherit all existing checks and not add any.
Example: You don't need RC to fly an Orbit. But we'll not say "it's auto" because the user can interact via RC if he wants.
This would preserve the "old" way of you need to adjust the commander for every flight feature (or task) that can be entered which we know doesn't scale.

In your use case how are you switching into orbit to begin with?

Via vehicle command (internally)/MAVLink command (externally).

Then how are you switching back into position control mode?

For now exactly like it was before. Reading the navigation state.

[dagar]

That forces all tasks that run within this navigation state to inherit all existing checks and not add any.

That's exactly the point. The system needs to know if manual control is required or not so that it can do the right thing if and when you lose RC or the data link. That doesn't mean it can't be structured such that Orbit is possible without manual control. We need to find a way that works WITH the system, not around it.

For logging you should be adding a status message that publishes the MC position controller/flighttask internal state.

[ChristophTobler]

@dagar After thinking about how the implementation could look like without the custom nav state, we think that this is still cleaner. Because otherwise we would need to check for the nav state and the vehicle command if something changed and that gets a bit messy again because a "custom" task could be under auto or manual and then pretty much all vehicle commands would need to be checked to switch back to actual auto or manual...

So I just addressed your comment and made the check for custom more restrictive. What do you think?

[dagar]

That's closer, but commander should still be involved in the broad categories of nav state.

• manual mode
  • with valid local position (position control)
  • without (altitude control)
  • requires manual input
  • etc
• auto mode
  • no manual input required

The system condition, control mode flags, failsafes, etc should be tightly integrated. Something closer to offboard_control_mode.

How about this?

• NAVIGATION_STATE_POSCTL_CUSTOM
• NAVIGATION_STATE_AUTO_CUSTOM

[ChristophTobler]

Hm, I'm not sure if this is any better than adding a state for every task...

How about we treat the custom state as an auto for now (for all tasks) till we have a more long term solution?

[dagar]

Hm, I'm not sure if this is any better than adding a state for every task...

Possibly, but I'm still fairly certain that circumventing the state machine will only cause problems down the line.

How about we treat the custom state as an auto for now (for all tasks) till we have a more long term solution?

At the moment it's primarily a question of how the RC and datalink loss failsafes should be treated.

[ChristophTobler]

Ok. I guess this needs some more discussion. For now we will add the change downstream because we need it now. But obviously we still want a solution that works for upstream as well so we don't diverge.

I'll close this PR for now and I'll link an issue for future discussion.