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: remove unassigned _track_desired #17122

Merged
merged 1 commit into from
Apr 8, 2021

Conversation

chobitsfan
Copy link
Contributor

_track_desired is never assigned but is used in shift_wp_origin_to_current_pos(). It may be a little dangerous

@khancyr
Copy link
Contributor

khancyr commented Apr 8, 2021

That is an important bug ! Nice catch.
_track_desired will be default to 0 and thus we will never pass the first check on shift_wp_origin_to_current_pos() ... that is an issue

Copy link
Contributor

@khancyr khancyr left a comment

Choose a reason for hiding this comment

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

Should be safe to merge. it will be similar to the other shift functions so that isn't an issue

@rmackay9
Copy link
Contributor

rmackay9 commented Apr 8, 2021

Great thank, I'll merge in a moment.

I don't think this will make a functional change because, as @khancyr says, _track_desired will default to zero so it should never cause AC_WPNav::shift_wp_origin_to_current_pos() to return early.

@rmackay9 rmackay9 merged commit fd88e53 into ArduPilot:master Apr 8, 2021
@chobitsfan
Copy link
Contributor Author

chobitsfan commented Apr 9, 2021

Hi @rmackay9 @khancyr I am sorry but I may found a bigger possible problem.

I trace the git log and found the _track_desired assignment is removed when #15896 merged. Before #15896 merged, shift_wp_origin_to_current_pos() will return early if vehicle is not takeoff from origin (for exmaple, vehicle startup here and moved to other place). This will cause a horizontal drift when copter takeoff. I notice this problem when doing indoor ext nav flying test

@rmackay9
Copy link
Contributor

rmackay9 commented Apr 9, 2021

@chobitsfan,

OK, thanks. I've added an issue to the Copter-4.1 issues list (see #16478). If you have logs of the issue that would be great.

@chobitsfan
Copy link
Contributor Author

Hi @rmackay9 Thank you
log 20 2021-4-8 am 09-41-18.zip

Log Browser - 20 2021-4-8 上午 09-41-18 bin 2021_4_9 上午 11_22_14

@chobitsfan
Copy link
Contributor Author

I create #17132 to fix this issue

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