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

Plane: fixed bug in pullup and loiter to alt code #28259

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

tridge
Copy link
Contributor

@tridge tridge commented Sep 28, 2024

This fixes a bug in the L1 controller where reached_loiter_target() goes false after capture due to an unachievable loiter radius or wind. This can lead waypoint types that rely on a combination of reached_loiter_target() and another condition (such as being lined up with the next waypoint) to never complete

This also fixes a bug in the pullup code where poor pitch trim can mean we never hit the target airspeed in the pullup

Note that the loiter to alt fix is not really a complete fix. If the initial trajectory prevents capture ever happening then we can still have the same issue. We could add a check for yaw going through more than 360 degrees to make this more robust

Note: This builds on #28257

@peterbarker this includes a fix for the wait min/max/accuracy code in vehicle test suite that will impact a lot of tests. We were getting things like this:

2024-09-29T03:43:34.6080462Z AT-1349.0: Altitude=65.41 (want 67.000000 +- 3.000000)
2024-09-29T03:43:34.6081493Z AT-1349.1: Altitude=67.74 (got 67.000000 +- 3.000000) (maintain=0.3/10.0)
2024-09-29T03:43:34.6082665Z AT-1349.1: Altitude=69.52 (got 67.000000 +- 3.000000) (maintain=1.3/10.0)
2024-09-29T03:43:34.6083815Z AT-1349.2: Altitude=72.00 (got 67.000000 +- 3.000000) (maintain=2.4/10.0)
2024-09-29T03:43:34.6084763Z AT-1349.2: Altitude=73.28 (want 67.000000 +- 3.000000)
2024-09-29T03:43:34.6085559Z AT-1349.2: Altitude=73.33 (want 67.000000 +- 3.000000)
2024-09-29T03:43:34.6086494Z AT-1349.3: Altitude=72.86 (got 67.000000 +- 3.000000) (maintain=0.3/10.0)
2024-09-29T03:43:34.6087555Z AT-1349.3: Altitude=72.35 (got 67.000000 +- 3.000000) (maintain=1.4/10.0)
2024-09-29T03:43:34.6088620Z AT-1349.4: Altitude=72.02 (got 67.000000 +- 3.000000) (maintain=2.4/10.0)
2024-09-29T03:43:34.6089673Z AT-1349.4: Altitude=71.77 (got 67.000000 +- 3.000000) (maintain=3.5/10.0)
2024-09-29T03:43:34.6090905Z AT-1349.4: Altitude=71.69 (got 67.000000 +- 3.000000) (maintain=4.5/10.0)
2024-09-29T03:43:34.6091944Z AT-1349.5: Altitude=71.69 (got 67.000000 +- 3.000000) (maintain=5.6/10.0)
2024-09-29T03:43:34.6092998Z AT-1349.5: Altitude=71.72 (got 67.000000 +- 3.000000) (maintain=6.6/10.0)
2024-09-29T03:43:34.6094091Z AT-1349.5: Altitude=71.78 (got 67.000000 +- 3.000000) (maintain=7.7/10.0)
2024-09-29T03:43:34.6095129Z AT-1349.6: Altitude=71.81 (got 67.000000 +- 3.000000) (maintain=8.7/10.0)
2024-09-29T03:43:34.6096177Z AT-1349.6: Altitude=71.84 (got 67.000000 +- 3.000000) (maintain=9.8/10.0)
2024-09-29T03:43:34.6097082Z AT-1349.6: Attained Altitude=71.93927722772277

which shows it accepting an out of range value

@tridge tridge added the Plane label Sep 28, 2024
@tridge tridge force-pushed the pr-loiter-to-alt branch 2 times, most recently from de5c5e1 to 5e2f19e Compare September 29, 2024 01:06
if we have poor pitch trim it is possible we will pullup before
reaching the target airspeed. Check pitch threshold during airspeed
stage of pullup
if our target loiter radius is unachievable then we can reach the
loiter target on initial capture but be unable to maintain it. This
ensures that once we capture we return true on reached_loiter_target()

This is critical for any mission type where we take an action on
reached_loiter_target() and another condition (such as being lined up
for a waypoint). Otherwise we may continue loitering forever
use LOITER_TO_ALT and a landing, allowing for a much better test of
the full glider pullup mission
we were accepting values outside the specified range
need terrain handlers installed to support terrain targets
Copy link
Member

@IamPete1 IamPete1 left a comment

Choose a reason for hiding this comment

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

L1 changes looks good. Pull up change looks good too and is contained to glider pull up so not a high risk. Not looked at the test changes.

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.

3 participants