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: Perform transition towards home location #14780

Merged
merged 2 commits into from
Feb 22, 2021

Conversation

dayjaby
Copy link
Contributor

@dayjaby dayjaby commented Apr 28, 2020

Describe problem solved by this pull request
Followup on #14728. To prevent performing a transition while still loitering around the home location, this PR makes sure that the drone heads towards the home location before triggering the transition. For this purpose, it is reasonable to add a new parameter RTL_LOITER_RAD to allow the user to set a loiter radius for the altitude descending that still allows the drone to turn towards the home location before initiating a transition.

Test data / coverage
Sitl gazebo log: https://review.px4.io/plot_app?log=b71abace-d7ba-4f9d-a2ef-71ac005d09e1

@stale
Copy link

stale bot commented Aug 22, 2020

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

@stale stale bot added the stale label Aug 22, 2020
@sfuhrer
Copy link
Contributor

sfuhrer commented Jan 4, 2021

@dayjaby there were a lot of VTOL RTL changes coming in with #16377, resulting in conflicts here that would need to be addressed first.
I general I think having a separate loiter radius during RTL could make sense (beside NAV_LOITER_RAD). And I agree that doing the transition towards home would be much better in most cases.
Could you rebase and then share some logs/screen recordings of how it looks like?

@LorenzMeier
Copy link
Member

@dayjaby Do you have time to rebase this?

@dayjaby
Copy link
Contributor Author

dayjaby commented Jan 20, 2021

Sure! Did not notice Silvans message.

@dayjaby dayjaby changed the base branch from rtl_descend_vtol to master January 20, 2021 20:31
@dayjaby
Copy link
Contributor Author

dayjaby commented Jan 20, 2021

Log file: https://logs.px4.io/plot_app?log=455021db-8971-4254-a61f-51a95cbfb88f

The transition overshoots a little, which can be compensated by setting larger loiter radius, for example 70m instead of 50m: https://logs.px4.io/plot_app?log=a8c69cac-4c30-4546-9579-ea48f7886047

@dayjaby
Copy link
Contributor Author

dayjaby commented Jan 28, 2021

@sfuhrer can you check this?

@sfuhrer
Copy link
Contributor

sfuhrer commented Feb 1, 2021

@dayjaby It needs another rebase. Otherwise I think it's a nice improvement with rather minimal changes. I wonder though if we shouldn't just use NAV_LOITER_RAD for the loiter radius (I know, previously I commented otherwise, but we really only should introduce new params if we're really convinced) - I guess you introduced a new one to specifically be able to tune if for the optimal distance required to finish the transition right over home?

@dayjaby dayjaby force-pushed the rtl_descend_vtol branch 2 times, most recently from 19173da to 78b4071 Compare February 1, 2021 23:00
@dayjaby
Copy link
Contributor Author

dayjaby commented Feb 1, 2021

@sfuhrer "Right over home" not so much, but rather having not too high turning angles, especially close to home.

In the future I would like to perform the back transition against the wind. Are the values from the wind_estimator reliable enough for that?

@sfuhrer
Copy link
Contributor

sfuhrer commented Feb 2, 2021

"Right over home" not so much, but rather having not too high turning angles, especially close to home.

Makes sense. But then I guess it's okay to use NAV_LOITER_RAD, or do you see a reason why the RTL loiter should have different radius than the Hold loiter? Both could e.g. be set to 75 for a 2m wingspan VTOL.

In the future I would like to perform the back transition against the wind. Are the values from the wind_estimator reliable enough for that?

I would say so yes, especially as during the loiter down you can estimate the wind quite well

@dayjaby
Copy link
Contributor Author

dayjaby commented Feb 2, 2021

I think the descend loiter radius should in general be higher than the normal loiter radius. While descending you gain more speed, so having less sharp turns might be beneficial. This also means that you can descend faster with a larger radius. If this benefit can be neglected, we can go for the NAV_LOITER_RAD approach.

@sfuhrer
Copy link
Contributor

sfuhrer commented Feb 4, 2021

I think the descend loiter radius should in general be higher than the normal loiter radius. While descending you gain more speed, so having less sharp turns might be beneficial. This also means that you can descend faster with a larger radius. If this benefit can be neglected, we can go for the NAV_LOITER_RAD approach.

@RomanBapst what's your opinion here? I still feel like having just one loiter radius param for now would be better, and add a separate one if flight testing shows the clear need for it.

Copy link
Contributor

@RomanBapst RomanBapst left a comment

Choose a reason for hiding this comment

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

@dayjaby I'm not quite following how this should work. Where are you now triggering the transition once the home waypoint is reached?

I tested in SITL and I had a flyaway so I think something is not quite right.
Could you please re-test?

@mrpollo
Copy link
Contributor

mrpollo commented Feb 10, 2021

Hey @dayjaby can you please help us address some of the reviewer comments above? we would love to get this merged soon

@dayjaby
Copy link
Contributor Author

dayjaby commented Feb 12, 2021

@RomanBapst I am sorry; a slice of code did not survive one of the rebases. Thanks for testing it! This time I tested it properly: https://logs.px4.io/plot_app?log=aa3bf100-4cb5-453b-9dcb-a4738ab04126

The backtransition is done if the default acceptance radius is reached.

@dayjaby
Copy link
Contributor Author

dayjaby commented Feb 16, 2021

@julianoes Regarding the failed SITL test: I am pretty sure this PR does not influence any iris flights. I also tried your failure injection and it lands successfully.

pxh> failure gps off
WARN  [failure] inject failure unit: gps (4), type: off (1), instance: 0
WARN  [simulator] CMD_INJECT_FAILURE, GPS off
pxh> WARN  [commander] Failsafe enabled: no global position
INFO  [commander] Failsafe mode activated
INFO  [tone_alarm] battery warning (fast)
WARN  [ecl/EKF] 143604000: GPS data stopped
WARN  [ecl/EKF] 143604000: GPS data stopped
WARN  [ecl/EKF] 143608000: GPS data stopped
INFO  [ecl/EKF] 143608000: reset position to last known position
INFO  [ecl/EKF] 143608000: reset velocity to zero
WARN  [ecl/EKF] 143608000: stopping navigation
INFO  [ecl/EKF] 143604000: reset position to last known position
INFO  [ecl/EKF] 143604000: reset position to last known position
INFO  [ecl/EKF] 143604000: reset velocity to zero
INFO  [ecl/EKF] 143604000: reset velocity to zero
WARN  [ecl/EKF] 143604000: GPS data stopped
INFO  [ecl/EKF] 143604000: reset position to last known position
INFO  [ecl/EKF] 143604000: reset velocity to zero
WARN  [ecl/EKF] 143604000: stopping navigation
WARN  [ecl/EKF] 143604000: stopping navigation
WARN  [ecl/EKF] 143604000: stopping navigation
WARN  [ecl/EKF] 143612000: GPS data stopped
WARN  [ecl/EKF] 143612000: GPS data stopped
INFO  [ecl/EKF] 143612000: reset position to last known position
INFO  [ecl/EKF] 143612000: reset velocity to zero
INFO  [ecl/EKF] 143612000: reset position to last known position
WARN  [ecl/EKF] 143612000: stopping navigation
INFO  [ecl/EKF] 143612000: reset velocity to zero
WARN  [ecl/EKF] 143612000: stopping navigation
INFO  [commander] Landing detected
INFO  [commander] Disarmed by landing
INFO  [commander] Failsafe mode deactivated

Do you have any idea what's wrong here?

* @increment 0.5
* @group Return Mode
*/
PARAM_DEFINE_FLOAT(RTL_LOITER_RAD, 50.0f);
Copy link
Contributor

Choose a reason for hiding this comment

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

@dayjaby This is now the same default as NAV_LOITER_RAD. Does this really makes sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think not, but I would like some test flights first before setting it to a more sane value.

@RomanBapst
Copy link
Contributor

@dayjaby I have one last comment regarding the parameter default. Please advise on that. Other than that I think this PR is good to go.

@dayjaby
Copy link
Contributor Author

dayjaby commented Feb 17, 2021

@dayjaby I have one last comment regarding the parameter default. Please advise on that. Other than that I think this PR is good to go.

@RomanBapst Comparing the two previous log files (https://logs.px4.io/plot_app?log=a8c69cac-4c30-4546-9579-ea48f7886047 and https://logs.px4.io/plot_app?log=455021db-8971-4254-a61f-51a95cbfb88f), by just increasing the descend loiter radius from 50m to 70m, the roll angle when backtransition commences decreases from 42 degrees to 15 degrees. Let's go for this value?

@sfuhrer sfuhrer merged commit ed7a531 into PX4:master Feb 22, 2021
This pull request was closed.
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.

6 participants