-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
Plane: add param LAND_RNGFND_HLD to hold-off lidar during landing #25549
base: master
Are you sure you want to change the base?
Conversation
2d02948
to
e8be476
Compare
libraries/AP_Landing/AP_Landing.cpp
Outdated
@@ -163,6 +163,15 @@ const AP_Param::GroupInfo AP_Landing::var_info[] = { | |||
// @User: Advanced | |||
AP_GROUPINFO("WIND_COMP", 18, AP_Landing, wind_comp, 50), | |||
|
|||
// @Param: RNGFND_HLD | |||
// @DisplayName: Rangefinder hold-off | |||
// @Description: Rangefinder hold-off distance from land point. Hold off using the rangefinder's altitude correction until you're this close to the NAV_LAND point. Requires RNGFND_LANDING = 1. This is useful to avoid the rangefinder reacting to objects/trees, or pits, before the level-ish runway starts. Set to zero to disable the hold-off (don't inhibit the rangefinder). Using this feature will cause the rangefinder to correct the baro offset later in the landing which will lead to a larger slope correction. It is suggested to use LAND_ABORT_DEG with this feature so that unacheivable slopes will cause automatic go-arounds while retaining the rangefinder correction in the baro for a smoother landing on the next attempt. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe say "this close horizontally" to the land point (not clear on first read it is horizontal)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added!
ArduPlane/commands_logic.cpp
Outdated
if (landing.get_rangefinder_holdoff_distance() <= 0 || // rangefinder holdoff is disabled (never hold off, always use rtanegfinder) | ||
auto_state.wp_proportion >= 1 || // or we have reached the land waypoint | ||
landing.is_flaring() || // or we are flaring | ||
current_loc.get_distance(next_WP_loc) >= landing.get_rangefinder_holdoff_distance()) // or we've reached the holdoff distance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't that be <= not >= ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whoops, fixed! I went back and forth about it being a bunch of not ANDs instead of positive ORs.. guess I missed this one
e8be476
to
58c6dc8
Compare
// use rangefinder to correct if possible | ||
float height = height_above_target() - rangefinder_correction(); | ||
if (landing.get_rangefinder_holdoff_distance() <= 0 || // rangefinder holdoff is disabled (never hold off, always use rtanegfinder) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (landing.get_rangefinder_holdoff_distance() <= 0 || // rangefinder holdoff is disabled (never hold off, always use rtanegfinder) | |
if (landing.get_rangefinder_holdoff_distance() <= 0 || // rangefinder holdoff is disabled (never hold off, always use rangefinder) |
float height = height_above_target() - rangefinder_correction(); | ||
if (landing.get_rangefinder_holdoff_distance() <= 0 || // rangefinder holdoff is disabled (never hold off, always use rtanegfinder) | ||
auto_state.wp_proportion >= 1 || // or we have reached the land waypoint | ||
landing.is_flaring() || // or we are flaring |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure its a good idea to change the altitude once the flare has started. I don't think it will do anything in anycase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not?
What are you expecting to happen once the flare has already started? Suddenly the correction means were now above flare height and we stop flaring? (which of course means we won't do this correction next loop... )
I just think if your already in the flair its too late to start moving the target altitude about, trying to do it might cause unexpected behavior.
Why not pass the rangefinder correction in to AP landing and let it decide? |
I think the idea is to ensure that in tricky landing scenarios (carrier style which is extremely common with uneven terrain), you want it to do less deciding if you have planned accordingly. |
I'm not saying the logic should be any different, I just think its in the wrong place. This new logic is calling |
devcall result: refactor to AP_Landing and then lets discuss again |
I think the param name is misleading. Even though it technically IS holding off, what we're really trying to say is only engage the rangefinder within X many meters from NAV_LAND. The length of the landing glide slope is not considered, only distance to the point. I was talking to someone about this and they interpreted the name (not the desc) as X many meters to NOT use the lidar before engaging it. With that interpretation your landing start makes a huge difference and is harder to mission plan |
Will be good to run this through some varying terrain based SITL tests (is that possible, I will need to learn to help) and see if we can induce some errors and what it takes to create the errors so we can identify the limits of the current implementation. Then we can give it some real-world tests and put/plan objects in our final path to reproduce. Maybe move one of the shipping containers to just before the runway (and also try and not hit is too...). |
IMHO it would be good to have a dedicated waypoint for barometer alignment this would allow for baro alignment near the center of the runway instead of the threshold just before touchdown. This would prevent situation where plane flies into the terrain before entering rangefinding zone due to altitude error. |
@LupusTheCanine I've made a script to do that using Jump Tags as markers. |
@ryanjAA How's the testing going? Need a rebase? |
Add param LAND_RNGFND_HLD to holdd-off lidar during landing
// @Description: Rangefinder hold-off distance from land point. Hold off using the rangefinder's altitude correction until you're this close to the NAV_LAND point. Requires RNGFND_LANDING = 1. This is useful to avoid the rangefinder reacting to objects/trees, or pits, before the level-ish runway starts. Set to zero to disable the hold-off (don't inhibit the rangefinder). Using this feature will cause the rangefinder to correct the baro offset later in the landing which will lead to a larger slope correction. It is suggested to use LAND_ABORT_DEG with this feature so that unacheivable slopes will cause automatic go-arounds while retaining the rangefinder correction in the baro for a smoother landing on the next attempt.
Inspired by Ryan Johnston (@ryanjAA) at Applied Aeronautics
An even better solution would be to add it to the NAV_LAND point but it's pretty maxed out for us :(