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

Cotper: RTL correct height for RTL_ALT_FINAL > 0 (home vs origin) #29244

Merged
merged 2 commits into from
Feb 11, 2025

Conversation

IamPete1
Copy link
Member

@IamPete1 IamPete1 commented Feb 7, 2025

I didn't know we had this feature, it allows the vehicle to return to the home location but hover at some given altitude above home.

When doing code review I spotted that the descent_target location is setup relative to home and then passed to the position controller which is relative to origin. This converts it to above origin and then updates the comparison function to use inertial nav altitude which is also relative to origin.

I suspect this has been broken for ages, I wonder if anyone uses the feature? Although admittedly the difference between home and origin is typically small so it may not have been obvious. Maybe we could remove the functionality?

But I can understand why you might want this, maybe we should fix it and add a autotest?

@Hwurzburg Hwurzburg added the WikiNeeded needs wiki update label Feb 7, 2025
@IamPete1
Copy link
Member Author

IamPete1 commented Feb 9, 2025

Master flys to -10 meters relative to home which is the correct altitude relative to origin:
image

This PR as expected, fly to 10 relative home:
image

Test that made the logs is added.

There are still some ways you could trip it up, the path is calculated when RTL is entered so if you change origin on the way then we will have the altitude difference as error. But I suspect the same would be true for normal RTL if you moved home.

@rmackay9 rmackay9 merged commit e62ca11 into ArduPilot:master Feb 11, 2025
79 checks passed
@rmackay9 rmackay9 added the BUG label Feb 11, 2025
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.

4 participants