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

ArduPlane: parameterize lookahead climb ratio #28891

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

timtuxworth
Copy link
Contributor

@timtuxworth timtuxworth commented Dec 16, 2024

Plane altitude terrain lookahead currently uses a fairly arbitrary but safe 50% factor when calculating the maximum pitch to assume when calculating the if the plane can safely fly over the approaching terrain.

Some users want to be more aggressive than the default. so this makes the 50% factor into a parameter. The default is no-change to current behavior, but by changing TERRAIN_LKA_CLMB users can now chose a more (or less) aggressive factor for the percentage of maximum climb factor to use when calculating terrain lookahead.

@timtuxworth timtuxworth requested a review from Georacer December 16, 2024 22:05
@timtuxworth
Copy link
Contributor Author

Hi George - could you take a look? Any suggestions?

@timtuxworth timtuxworth force-pushed the pr-terrain-climb-ratio branch 2 times, most recently from b2bc7c5 to 4da32dc Compare December 16, 2024 22:12
Copy link
Contributor

@Georacer Georacer left a comment

Choose a reason for hiding this comment

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

Hi Tim!

I've tried to read and internalize the lookahead code, but it's quite opaque; in the way I understand it, it's plainly wrong.
So I probably understand it wrong.

But that means that I shouldn't just expect your intervention to have no side effects.

Instead, do you have any test results (logs), where before the plane wouldn't manage a climb and now it does?

ArduPlane/altitude.cpp Outdated Show resolved Hide resolved
@Hwurzburg Hwurzburg added the WikiNeeded needs wiki update label Dec 18, 2024
@timtuxworth
Copy link
Contributor Author

Hi Tim!

I've tried to read and internalize the lookahead code, but it's quite opaque; in the way I understand it, it's plainly wrong. So I probably understand it wrong.

No you have it right. I had to rewrite terrain lookahead in my terrain avoidance Lua script #28625 because of that. In fact the only thing I really want out of it is the parameter value :-), so that my Lua based lookahead can use the same ratio.

@timtuxworth timtuxworth force-pushed the pr-terrain-climb-ratio branch from 4da32dc to 981c536 Compare December 30, 2024 18:26
@timtuxworth
Copy link
Contributor Author

But that means that I shouldn't just expect your intervention to have no side effects.

The code should have no side effects iff the default value is used, which should give the same results as the previous hard coded value.

@Georacer
Copy link
Contributor

@timtuxworth I've spent some time parsing the code.

To me, it looks like if the ratio is 0, only then will the full climb rate be asked by the lookahead.

But I want to see comparative tests.
If you beat me to that, please post those logs.

@Georacer
Copy link
Contributor

Hello @timtuxworth !

I've done some testing with your code today.

I built a mission that I think is a repeatable stress-test for the lookahead code:
dadi.txt

You can launch your SITL at those locations with

sim_vehicle.py -v ArduPlane --frame quadplane --console --map -D -G -l 38.638235,22.595485,388,0

and then load the mission.

I've run this mission with various settings and you can see cumulative results in the following figure.

  • With purple, you see the terrain height.
  • With blue, you see a run with TERRAIN_LOOKAHD=0. It crashes into the mountain ~90s in.
  • With red, you see a run with PTCH_LKAHD_CLMB=0. This should provide the biggest clearance, but because of the way the code is structured, it returns early and is effectively the same as TERRAIN_LOOKAHD=0. This is a bug and I'll make a note of it later.
  • With green, you see a run with PTCH_LKAHD_CLMB=1. This is actually what gives the greatest clearance. In theory this means that TECS capabilities will not be taken into account.
  • With orange, you see the default (and currently applied on master) case of PTCH_LKAHD_CLMB=50. This is almost the same as the one prior, except for the end of the climb, where it flies lower.
  • With pink, you see the case of PTCH_LKAHD_CLMB=100. In theory, this will reduce the lookahead altitude by 100% of the TECS capabilities. You see this is the setting that flies closest to the ground.

image

That said, I'd like to ask you for the following things:

  1. Please repeat my experiments. Sometimes I'm getting flaky results.
  2. Change the parameter behaviour, so that you get the most clearance at PTCH_LKAHD_CLMB=100. I think this is more intuitive.
  3. Make it so that when setting PTCH_LKAHD_CLIMB=0 the lookahead behaviour isn't disabled. Limiting to a minimum of 1 should do the trick, but feel free to do what you think best.

@timtuxworth
Copy link
Contributor Author

This is great. Thanks so much @Georacer . I should be able to look at this in the next couple of days.

One big question. HOW did you get this graph of multiple flights on the same graph?

@Georacer
Copy link
Contributor

One big question. HOW did you get this graph of multiple flights on the same graph?

Heh...

meme_08

@timtuxworth
Copy link
Contributor Author

This is interesting @Georacer - I'm getting Q_ASSIST kicking in at 0 altitude. According to the description of Q_ASSIST_ALT, 0 should mean disabled, but I'm definitely seeing it firing.

@Georacer
Copy link
Contributor

This is interesting @Georacer - I'm getting Q_ASSIST kicking in at 0 altitude. According to the description of Q_ASSIST_ALT, 0 should mean disabled, but I'm definitely seeing it firing.

You know how it goes @timtuxworth : Log or didn't happen! :)

@timtuxworth timtuxworth force-pushed the pr-terrain-climb-ratio branch from 981c536 to 455739a Compare January 30, 2025 23:14
@timtuxworth
Copy link
Contributor Author

timtuxworth commented Jan 30, 2025

Made the changes requested by @Georacer and ran a suite of tests, to compare the results with
a. no lookahead
b. TERRAIN_LKA_CLMB = 0, 25, 50 (the default), 75, 100
The plane crashes into the side of he mountain with no lookahead and with 0% ratio. It's successively "safer" with 25 ... 100.

Logs are here: (for as long as the links last). https://www.dropbox.com/scl/fo/pohi2wssydjefod689khp/APSSKzsSW3mkgHw29A0Ysjw?rlkey=seedu7gzeslzw9l6dfbdcnzpe&dl=0

I graphed this in PlotJuggler and got this. The dark blue line is the physical terrain. As you can see it flies over a very steep mountain. For each value of TERRAIN_LKA_CLMB I show TERR/CHeight (the height above terrain) and POS/Alt - the actual altitude. As you can see climb-050 (the default) is actually quite safe and pushing it up to 100 seems like it might only be necessary in very steep terrain such as this example where it maintains good clearance.
Screenshot 2025-01-30 165815

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WikiNeeded needs wiki update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants