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: Fix throttle nudge in GUIDED mode #26107

Closed

Conversation

AutomationEngineer
Copy link

This PR is intended to fix #26094 issue.

guided_state.target_airspeed_cm is initialised to -1 and when OFFBOARD_GUIDED == ENABLED, check for is_zero was not correct, causing throttle nudge not working in GUIDED mode. Also > 0.0 changed to is_positive, to keep in line with other code.

Test results in SITL: logs before and after.zip

@@ -257,7 +257,7 @@ void Plane::calc_airspeed_errors()

// when using the special GUIDED mode features for slew control, don't allow airspeed nudging as it doesn't play nicely.
#if OFFBOARD_GUIDED == ENABLED
if (control_mode == &mode_guided && !is_zero(guided_state.target_airspeed_cm) && (airspeed_nudge_cm != 0)) {
if (control_mode == &mode_guided && is_positive(guided_state.target_airspeed_cm) && (airspeed_nudge_cm != 0)) {
Copy link
Member

Choose a reason for hiding this comment

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

There is a change in behavior here. The current way we default is to set to -1. Here:

plane.guided_state.target_airspeed_cm = -1; // same as above, although an airspeed of -1 is rare on plane.

Of course -1 is not zero, but it is not positive.

So i think were now allowing airspeed nudge where we previously would not. That sound reasonable to me, but the current behavior might be baked into the texts.

Copy link
Author

@AutomationEngineer AutomationEngineer Feb 1, 2024

Choose a reason for hiding this comment

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

As it states from THROTTLE_NUDGE parameter description throttle input should affect target airspeed in auto-throttle modes and GUIDED mode is one of them. Also in Tuning Cruise Configuration it is written that this option should allow pilot to tweak target airspeed in AUTO and GUIDED modes. So I think the current behavior is not in line with texts and we should bring it in line.
Also on low processing power devices it will work from the box because this check will be skipped by #if OFFBOARD_GUIDED == ENABLED
But I have a concern about the reason of failing plane sitl tests while passing quadplane tests.

Copy link
Author

Choose a reason for hiding this comment

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

As I expected, failing climb test was because of not neutral throttle stick, making it neutral solved the issue.
As for fence issues, they also related to throttle stick position, and making throttle neutral solves them, but things are more complicated there...
In FenceRetRally test, RTL_RADIUS is set however WP_LOITER_RAD is used in autopilot in the first part of a test and by default it is 80. When autopilot try to circle 80 it can't, but the error is exactly 20 and testing for 100 passes :)
I tried setting WP_LOITER_RAD = 100 combined with NAVL1_LIM_BANK = 60, but still there is an error of 10 meters dew to lack of integral part in L1 controller while loitering.

@AutomationEngineer AutomationEngineer marked this pull request as draft February 2, 2024 12:51
@AutomationEngineer
Copy link
Author

@IamPete1, finally I got a test passing variant but it is not really 'tidy' cause it has lots of loose commits, is it ok to close this PR and oppen PR from #plane-fixThrNudge which has only one commit ahead of master?

@AutomationEngineer AutomationEngineer marked this pull request as ready for review March 29, 2024 04:55
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.

I think it makes more sense to put the throttle back to center after the take of is complete here:

https://github.com/AutomationEngineer/ardupilot/blob/4edb2547e62942e476d206bfc655fce6fc7a02ae/Tools/autotest/arduplane.py#L152-L154

@IamPete1
Copy link
Member

@IamPete1, finally I got a test passing variant but it is not really 'tidy' cause it has lots of loose commits, is it ok to close this PR and oppen PR from #plane-fixThrNudge which has only one commit ahead of master?

Better to rebase and squash. Instructions here: https://ardupilot.org/dev/docs/git-interactive-rebase.html

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

Successfully merging this pull request may close these issues.

Plane: THROTTLE_NUDGE doesn't work in GUIDED Mode
2 participants