[WIP] flighttask static method to check if data is available

[Stifael]

No description provided.

[Stifael]

With the current Flighttask architecture it is possible to enter a state where the commander demands to switch to a task (for instance Auto), but the FlightTaskAuto has not received a valid triplet. If that happens, FlightTaskAuto will return 0 and the task will exit. Unfortunately, this also means that the old FlightTask, from which a switch was demanded, is lost as well due to FlightTask union.

It would be beneficial to check for the required data of a specific task before instantiating that particular task. One possible solution could be to add a static method to each task which can be called before the task is actually instantiated. I quickly laid out this approach in this PR, but I am not very happy in terms of implementation.

To see what I mean, I would do the following test (first with master, then with this branch):

• start sitl
• go directly into position
• wait until gps fusion was completed and do takeoff
• then switch to auto

Using the master, FlightTaskAuto will be instantiated, but due to an invalid triplet the activation fails. The previous state of FlightTask is lost as well. With this branch, the vehicle will remain in the old flighttask state.

[dagar]

What about adding a 2nd slot (TaskUnion) for the next task, and not fully switch until it's active and happy?

If there's a failure it should be reported (position controller status) so that commander can enter a proper failsafe state.

I'm still thinking about different ways we could tie this all together coherently in a modular fashion.

[dagar]

In the AUTO example you mentioned are you expecting the position setpoint triplet to continue to be invalid in any case?

Here we can do a better job with a separate mission status coming from navigator. Navigator mission can still check this while inactive and with feedback the state machine can block the transition to mission in the first place.

[Stifael]

What about adding a 2nd slot (TaskUnion) for the next task, and not fully switch until it's active and happy?

That would require to have a second instance. This is possible and definitely an option, but I would prefer to find a better solution instead of having two flighttasks instantiated. But then again, if there is no better solution, then this option is the way to go. However, have you looked at my suggestion?

If there's a failure it should be reported (position controller status) so that commander can enter a proper failsafe state.

Not in this case, otherwise it would never enter Auto. It is not a failsafe case. The issue that I am describing is caused by having different threads running at different frequencies.

Simple example:

• commander wants to go into Auto
• navigator and mc_pos_control switch into Auto
• mc_pos_control requires triplets from the navigator, but there is no guarantee that these triplets have already been updated by the time when mc_pos_control wants to execute Auto.

In the AUTO example you mentioned are you expecting the position setpoint triplet to continue to be invalid in any case?

No. The triplet will be valid after a few loops.

Here we can do a better job with a separate mission status coming from navigator. Navigator mission can still check this while inactive and with feedback the state machine can block the transition to mission in the first place.

In this particular case that could work. However it does not solve the general handling of consuming data from different threads.

[dagar]

Yes I looked at your solution. It's okay if it solves the immediate problem, but I would try to minimize all of this custom generated code. Having a 2nd slot might also be interesting for cleanly transitioning between FlightTasks.

Not in this case, otherwise it would never enter Auto. It is not a failsafe case. The issue that I am describing is caused by having different threads running at different frequencies.

I'm aware, and what I was proposing was an attempted solution to both this and an actual failure case. Activating a FlightTask can depend on certain data with an overall timeout.

[bkueng]

Seems like a second TaskUnion is the best solution for now, and then keep switching between them.

[Stifael]

Seems like a second TaskUnion is the best solution for now, and then keep switching between them.

And why that? I don't see why that is the case, since we haven't searched for alternatives yet. Let's step back for a second.
Right now I don't look for an immediate solution, because it already works with the current state. In addition, failsafe feedback to the commander is already implemented if switching does not work: https://github.com/PX4/Firmware/blob/master/src/modules/mc_pos_control/mc_pos_control_main.cpp#L1101-L1150

I will try to write down the pros and cons of the two approaches that we have right now, but I actually was hoping to gather first a couple more solutions before evaluating the pros and cons.

Second union

Advantage

• easy to implement
• does not require any changes in mc_pos_control and only small changes in FlightTasks

Disadvantage

• increases size of Flighttasks. In fact, there will be almost two "position-controllers" active at the same time, which is just a waste of memory.
• most likely it will not fit into older platforms such as fmu-v2
• in the future it is desired to have Fixedwing and Vtol to use FlightTasks as well. With a second union we would have 6 position controller active at the same time.

Static method

Advantage

• enables for checking. It can also be used by other modules to just check if a flighttask can be run or not
• will not increase memory usage

Disadvantage

• with current approach, requires another method that needs to be generated
• checking for data might require duplicate code: in the static method and once during the update-method after FlightTask got instantiated.

[dagar]

What about having the FlightTask (eg FlightTaskAuto) activate immediately, but do something safe and continuous in the interim until all the necessary data is available (updated valid position setpoint triplet)? The local position validity checks you're doing are redundant and could be dropped. Again, if the data doesn't become available in some generous period the MPC should publish a failure status (not a mode change) to commander, which would then activate the failsafe.

Then we need to take a look at all of these cases overall and lock it down (state machine) so that potentially invalid transitions are prevented in the first place (eliminating the need to fallback to the previous FlightTask as much as possible).

In addition, failsafe feedback to the commander is already implemented if switching does not work: https://github.com/PX4/Firmware/blob/master/src/modules/mc_pos_control/mc_pos_control_main.cpp#L1101-L1150

This is actually kind of problematic. If you change a mode like that it could be difficult or even impossible to discern from a mode change that came in from mavlink or rc. Regardless of the outcome of this discussion let's create an issue to change this.

[ChristophTobler]

What about having the FlightTask (eg FlightTaskAuto) activate immediately, but do something safe and continuous in the interim until all the necessary data is available

That would mean that every FlightTask that has race conditions needs its own "in the meantime failsafe", right? I guess that's undesired. And if we would put a general "in the meantime failsafe" into FlightTask, we just move it from the position controller, no?

Edit
I thought we want to check for the tasks before hand so that we can keep the current mode if it doesn't work.

[dagar]

It's a different side to the same problem. You either try to handle the dependencies upfront and not use the task until everything is ready, or handle it within the task itself. Not ideal either way.

In some cases I could see handling it explicitly in the new task being better than continuing with whatever setpoints you already had.

Currently a TaskUnion is 760 bytes of memory. Having a second instance isn't great, but we can handle that even on fmu-v2. The problem we kept hitting on px4fmu-v2 (and will hit again soon) is limited flash (code space). Generating code can end up using more flash than you might expect.

[Stifael]

What about having the FlightTask (eg FlightTaskAuto) activate immediately, but do something safe and continuous in the interim until all the necessary data is available (updated valid position setpoint triplet)?

I also don't think that this is a good idea, because then every Flighttask needs to have such a logic. I still think it is better to have an implementation that allows for checking first (let's assume it is done with a second union), and then if it fails, we need to define what should happen.

In the legacy code, if the triplets were not valid but auto was requested, then all the velocity setpoints were just set to 0. However, that was actually never really defined officially nor discussed what the desired behavior should be, but rather was added because it was not handled at all. Now we have the chance to define it once and for all.

There are several options:

• stay in the current task
• just set velocity to 0
• exit current task and go into failsafe task in which loiter, zero velocity or even land is executed depending on what estimation states are available

In all three options feedback can be sent back to the commander. The commander then needs to decide if he wants to continue to request a specific mode or not. This decision, however, is not something very simple to make. While the commander receives the feedback, the mc_pos_control would execute one of the options above (or something else).

I also want to emphasize that the last option does not overwrite the commander nor does it replace the commander state-machine. It just handles that case similar to the legacy code where the setpoints were set to 0.

[dagar]

What about having the FlightTask (eg FlightTaskAuto) activate immediately, but do something safe and continuous in the interim until all the necessary data is available (updated valid position setpoint triplet)?

I also don't think that this is a good idea, because then every Flighttask needs to have such a logic. I still think it is better to have an implementation that allows for checking first (let's assume it is done with a second union), and then if it fails, we need to define what should happen.

I agree, I was just trying to brainstorm possible alternatives that wouldn't require an additional slot.

Having the current active FlightTask fully available while constructing the next could be useful. For example you could initialize the relevant setpoints based on what the previous was actually doing.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

Closing as stale.