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

FlightTaskAuto: set state to None if previous and current setpoint are equal #22445

Merged
merged 1 commit into from
Nov 27, 2023

Conversation

sfuhrer
Copy link
Contributor

@sfuhrer sfuhrer commented Nov 27, 2023

Solved Problem

If the previous and current setpoint are set to the same, the logic in FlightTaskAuto::_getCurrentState() sets the state to offtrack, which then though doesn't get handled correctly as we don't have a track to follow in the first place.

Solution

Set state to None in that case.

Changelog Entry

For release notes:

Bugfix: FlightTaskAuto: harden line following logic (only do line following if previous and current setpoints are not equal).

Alternatives

It would also be nice to simply the whole (currently very convoluted) logic in FlightTaskAuto and PositionSmoother. Likely the desired behavior would be that on triggering Land, the vehicle breaks and then descends vertically where it came to reset, instead of going back to the point where the land was triggered as it does currently.

Test coverage

SITL tests.

Some prints from the state where the vehicle is drifting away in land mode:

******************
triplet_update: 0, _current_state: 0
OFFTRACK
_next_wp: -56.693569, -7.025488
_target: -56.693569, -7.025488
_prev_wp: -79.595253, -10.094345
turning
return_l1_point: -79.595253, -10.094345, -14.527375
closest_pt: -79.595253, -10.094345, -14.527375
alongtrack_error: 4.943437
crosstrack_error: 0.749954
xy_target_valid, is single waypoint: 0
2D pos sp to vel sp, vel_sp_constrained: -1.145724, -0.260856
set tmp_target from lock_position_xy, _lock_position_xy: -56.693569, -7.025488 
return_state: 0, _target_acceptance_radius: 2.000000
_triplet_target: -56.693569, -7.025488
_triplet_prev_wp: -56.693569, -7.025488
u_prev_to_target_xy: 0.000000, 0.000000
pos_to_target_xy: 22.915249, 3.071390
prev_to_pos_xy: -22.915249, -3.071390
closest_pt_xy: -56.693569, -7.025488
_position: -79.608818, -10.096878
_closest_pt: -56.693569, -7.025488
******************

Otherwise if is set to Offtrack, which in turn leads to weird behavior.
E.g. when triggering Land while flying fast forward, the vehcile doesn't
descned to the land point it keeps getting a velocity setpoint from the
smoother that pushes it away from the land point.

Signed-off-by: Silvan Fuhrer <silvan@auterion.com>
Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if the all original implementation took care of this case but I'd assume not and it was normally not something regular: https://github.com/PX4/PX4-Autopilot/blob/6e62beb560b1e9f47ff4ee897617ff88af10b52f/src/lib/FlightTasks/tasks/FlightTaskAutoLine.cpp#L148C11-L153

In any case I think this is a safe fix since if the vector u_prev_to_target_xy is zero (or even close to) then there is no line to follow and be offtrack from. Next step is removing this logic step by step.

Thank you @sfuhrer for looking into it and spending the time to find that problem and a fix for it!

@sfuhrer sfuhrer merged commit 2734c44 into main Nov 27, 2023
89 of 90 checks passed
@sfuhrer sfuhrer deleted the pr-harden-flight-task-auto-offtrack-main branch November 27, 2023 17:01
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

2 participants