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

AC_WPNav: protect from divide by zero especially when using CH6 tuning of WP speed #18560

Merged
merged 4 commits into from
Sep 6, 2021

Conversation

rmackay9
Copy link
Contributor

@rmackay9 rmackay9 commented Sep 3, 2021

This PR includes a few cosmetic fixes to AC_WPNav:

  • correct spelling of "terrain"
  • move definitions from .h to .cpp

.. and one real fix to avoid a divide-by-zero if ch6 tuning is being used to change the WP speed inflight. The reason the divide-by-zero could happen was that channel 6 tuning would call AC_WPNav::set_speed_xy() before AC_WPNav::wp_and_spline_init() was called. This means that _wp_desired_speed_xy_cms was still zero and led to the divide-by-zero.

This has been tested in SITL both before and after this change. Below is a screenshot of the ch6 tuning knob being used to set the WPNAV_SPEED value. Before this fix the simulator would crash and report the divide-by-zero.
image

This issue was discovered as part of the Copter-4.1.0-beta report, "Ch6 tuning of WP speed broken" on the 4.1 issues list and was reported here on the forums.

Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

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

LGTM

libraries/AC_WPNav/AC_WPNav.h Outdated Show resolved Hide resolved
@khancyr
Copy link
Contributor

khancyr commented Sep 3, 2021

We should take this opportunity to change the defines for stztic const. That is far better on debugging

@peterbarker
Copy link
Contributor

peterbarker commented Sep 4, 2021 via email

@rmackay9
Copy link
Contributor Author

rmackay9 commented Sep 4, 2021

Thanks for the review and comments. Let's merge this after Leonard's PR #18531 goes in.

@rmackay9 rmackay9 merged commit 8a5ade1 into ArduPilot:master Sep 6, 2021
@rmackay9
Copy link
Contributor Author

rmackay9 commented Sep 6, 2021

Leonard's PR is in so merging this one too.

@rmackay9 rmackay9 deleted the wpnav-div-by-zero-protect branch September 6, 2021 07:03
@rmackay9 rmackay9 added this to pending in Copter 4.1 Sep 9, 2021
@rmackay9 rmackay9 moved this from pending to 4.1.0-rc1 in Copter 4.1 Sep 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Copter 4.1
4.1.0-rc1
Plane 4.1
Awaiting triage
Rover 4.1
Awaiting triage
Development

Successfully merging this pull request may close these issues.

None yet

3 participants