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

Disable angle/Alt-hold based avoidance by default #29041

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

Conversation

rishabsingh3003
Copy link
Contributor

I have never seen or heard of the "ping pong method" working successfully and I think we should disable/remove it by deafult. The parameter AVOID_ANGLE_MAX is also confusing because it does not work for simple avoidance and is only meant for the "ping-pong" method.

Recently I encountered a few setups that were trying to get the regular loiter-based avoidance working, and kept alt-hold on a switch for manual control/recovery. However, since the angle based avoidance was enabled by default in that mode which the operator did not know about, it almost led to a crash. I don't think anyone would mind removing this feature by default but it makes things cleaner and safer IMO.

Copy link
Contributor

@rmackay9 rmackay9 left a comment

Choose a reason for hiding this comment

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

I've approved but after that I'm happy to have it merged.

@@ -79,7 +79,7 @@ void ModeAltHold::run()
case AltHoldModeState::Flying:
motors->set_desired_spool_state(AP_Motors::DesiredSpoolState::THROTTLE_UNLIMITED);

#if AP_AVOIDANCE_ENABLED
#if AP_AVOIDANCE_ENABLED && AP_ALT_HOLD_AVOIDANCE_ENABLED
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't look right; should be a single define being checked here?

@rmackay9
Copy link
Contributor

One small thing, I wonder why the build option is AC_AVOID_ALTHOLD and the code definition is AP_ALT_HOLD_AVOIDANCE_ENABLED, maybe make them the same?

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.

5 participants