FlightTasks: pass the timestamp of the message that causes FlightTask-switch

[Stifael]

This PR follows the issue described here #13293.

Not sure if https://github.com/PX4/Firmware/compare/px4/bugfix/pass-timestamp-into-flighttasks?expand=1#diff-fc77c3ef569029d45764664c75d4b0c1R790-R800 is correct, but given that it was control_mode.auto before, I suspect that this is correct.

[dagar]

What's this fixing? Is there a nav state that doesn't fit into control mode?

[Stifael]

It is just more consistent because all other transition depend on the vehicle_nav_state

[dagar]

What's the fix for a FlightTaskAuto activating before the position_setpoint_triplet is available?

[Stifael]

What's the fix for a FlightTaskAuto activating before the position_setpoint_triplet is available?

https://github.com/PX4/Firmware/pull/13526/files#diff-30c13a2a739574585e6a798c0b6e61beR143

Basically forces the vehicle to hover at current location.

By the way, it is still a hack. But for us it is actually a hotfix, because we crashed the drone because of it.

[dagar]

Ok, this still seems pretty horrible. Let's pursue a proper fix as soon as possible. Can we discuss on the dev call wednesday?

[Stifael]

Can we discuss on the dev call wednesday?

sure

[MaEtUgR]

Sorry but I'm against this change. It breaks multiple layers of modularity by using a timestamp set by the commander in every possible mode change to initialize the flight task state and then compare to timestamps which might but not necessarily are coming from the navigator. This increases the complexity a lot. If we want to check for the timestamps this way please let us directly use the existing _time_stamp_activate which is exactly for this purpose.

[Stifael]

@MaEtUgR as discussed in the issue #13293, this change is considered as a hotfix to prevent further crashes. However, I think but have not tested yet, the changes of this PR might not be necessary anymore with the changes of #13534.

If we want to check for the timestamps this way please let us directly use the existing _time_stamp_activate

See #13293 (comment). Assuming the decision was/is to use timestamp to check for valid triplets, then we cannot use _time_stamp_activate because that timestamp does not give us any information about when the commander triggered a state change.

every possible mode change to initialize the flight task state and then compare to timestamps which might but not necessarily are coming from the navigator

that is not true. These are two distinct timestamps: one that captures the timestamp of the commander state transition and one that represents the timestamp when flighttask was triggered. I would not mix them, because otherwise you would introduce inconsistency

[Stifael]

@MaEtUgR additional context: #13293 (comment)

[jorge789]

Tested on PixRacer V4:
Modes Tested

Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.
Mission Plan Mode (Automated): Good.
RTL: Good.

Procedure
Arm and Takeoff in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceeds to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint activate RTL.

Notes:
No issues noted good flight in general.

Log: https://review.px4.io/plot_app?log=080cbdf5-b0e4-4c7e-8b78-d535e2e87d8c

[Junkim3DR]

Tested on NXP FMUK66 v3

Modes Tested
Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.
Mission Plan Mode (Automated): Good.
RTL: Good.

Procedure
Arm and Takeoff in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceed to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint activate RTL.

Notes:
No issues noted, good flight in general.

Log: https://review.px4.io/plot_app?log=f1ad1779-5eb7-47d0-9492-acf0e775c8b1

Tested on Pixhawk Pro v4

Modes Tested
Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.
Mission Plan Mode (Automated): Good.
RTL: Good.

Procedure
Arm and Take off in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceed to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint activate RTL.

Notes:
No issues noted, good flight in general.

Log: https://review.px4.io/plot_app?log=947bc3df-1b43-466b-973d-009bf8e6bc76

[Tony3dr]

Tested on Pixhawk 4 V5
Modes Tested
Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.
Mission Plan Mode (Automated): Good.
RTL: Good.

Procedure
Arm and Takeoff in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceed to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint activate RTL, vehicle landed smooth and disarmed.

Notes:
No issues noted good flight in general.

Log:
https://review.px4.io/plot_app?log=140453c2-6209-44ef-8300-75607cfe1ac8&fbclid=IwAR1RAzJhsxvxJPVVJqqtLBktvxJfZxEITW3HJvXkZvBBm4HPmA3vxlXGCbo

[MaEtUgR]

We just discussed at the end of the dev call that it would be desirable to have a sequential execution of navigator, flight task, position control which would automatically solve this synchronization problem. Since the flight task library was originally designed to work that way it seems desirable.

[TSC21]

@Stifael @MaEtUgR is this PR going to be dropped, based on the latest discussions and formed workgroup around the rearchitecture of the Navigator+FlightTasks?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.