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

RTL: fix corner case where vehicle loiters in place instead of doing the RTL to Home #22984

Merged
merged 2 commits into from
Apr 11, 2024

Conversation

sfuhrer
Copy link
Contributor

@sfuhrer sfuhrer commented Apr 5, 2024

Solved Problem

With RTL_TYPE=0 and RTL_APPR_FORCE=1, the logic in RTL::findRtlDestination() does not check if the mission is valid, only if the mission has a valid land item. If the mission is not valid, it will result in anyway switching into the Mission RTL, only to then find that the mission is invalid and loiter at place instead of returning.

Solution

Instead of adding an additional check for the mission validity there, the proposal here is to not return hasMissionLandStart() as true.

Changelog Entry

For release notes:

Bugfix: RTL: fix corner case where vehicle loiters in place instead of doing the RTL to Home.

Alternatives

Add a check for mission validity in

if (((_param_rtl_type.get() == 1) || (_param_rtl_type.get() == 3) || (fabsf(FLT_MAX - min_dist) < FLT_EPSILON))
&& hasMissionLandStart()) {

@sfuhrer sfuhrer added the bug label Apr 5, 2024
@sfuhrer sfuhrer requested a review from KonradRudin April 5, 2024 13:57
Copy link
Contributor

@KonradRudin KonradRudin left a comment

Choose a reason for hiding this comment

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

The changes do not fix the issue. The RTL has another function here https://github.com/PX4/PX4-Autopilot/blob/main/src/modules/navigator/rtl.cpp#L618-L621 You would need to fix this function. But both the RTL and the mission_base should have the same logic in the function. Maybe a library function would be better here.

…s valid

Signed-off-by: Silvan Fuhrer <silvan@auterion.com>
Signed-off-by: Silvan Fuhrer <silvan@auterion.com>
@sfuhrer sfuhrer force-pushed the pr-mission-base-fix-hasMissionStart-main branch from 4d0a88f to c7f61dc Compare April 11, 2024 08:39
Copy link
Contributor

@KonradRudin KonradRudin left a comment

Choose a reason for hiding this comment

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

LGTM. Current behavior as i tested it:

  • If a mission was evaluated invalid, it wont choose the mission landing. If there is no valid RTL point available, it will fly back to takeoff
  • If the mission is valid and the RTL_type right, it will use the mission landing
  • If the mission is still evaluated valid although it is invalid (e.g. through mission checker parameter change) It will use the mission landing because the mission is not evaluated anymore (For a mission though it will enter mission mode, but then loiter as it wont execute the mission).
    All the above seems ok for me, as a fix for the last point would need some restructuring on how the mission checker is performed.

@sfuhrer
Copy link
Contributor Author

sfuhrer commented Apr 11, 2024

If the mission is still evaluated valid although it is invalid (e.g. through mission checker parameter change) It will use the mission landing because the mission is not evaluated anymore (For a mission though it will enter mission mode, but then loiter as it wont execute the mission).

Corner case though is that when you try to start an invalid mission it will then keep the mission invalidated, meaning that it won't use if for the RTL neither.
But as you said, to cleanly solve that we need to restructure a lot, and basically split up the landing part of a mission from the main mission body.

@sfuhrer sfuhrer merged commit 926e787 into main Apr 11, 2024
87 of 92 checks passed
@sfuhrer sfuhrer deleted the pr-mission-base-fix-hasMissionStart-main branch April 11, 2024 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants