ardupilot
ardupilot copied to clipboard
Plane: Fix MAV_CMD_NAV_LOITER_TURNS with zero turns skipping waypoint location
In the case where MAV_CMD_NAV_LOITER_TURNS waypoint is used the AV can skip travelling to the waypoint and jump directly to the next waypoint (if the conditions are correct).
This ensures loiter turns cannot be verified as complete until both loiter target is reached and turn count is met (the latter half is unchanged).
First commit shows the issue occurring, and is an example use case where you might use 0-turn loiters. Second commit shows the fix to the issue.
| Before | After |
|---|---|
@peterbarker @tridge I suspect a similar issue of short circuiting a waypoint can occur with LOITER_TO_ALT, given it is not checking for reaching of the loiter target either.
@peterbarker @tridge I suspect a similar issue of short circuiting a waypoint can occur with
LOITER_TO_ALT, given it is not checking for reaching of the loiter target either.
Although, i haven't been able to prove this through a separate autotest.
@joshanne I'd like to look at this a bit more
@tridge have you had a chance to look at this yet?
I think this PR is good. It's not addressing the underlying issue, but it's applying the same fix that other places in the code have employed and is the simplest correction with the lowest chance to hurt anything. And this is, I think, a pretty critical bug.
The root of the issue seems to stem from here. In loiter_angle_update, we specifically try to stop
the angle sum from accumulating until we reach the loiter target, but between clearing the angle and calling update_loiter (where the reached_loiter_target finally switches back to false), this update occurs, so this protection doesn't always work. https://github.com/ArduPilot/ardupilot/blob/b722ea8f2f88719f7c1ee92d5257f215fee0b68e/ArduPlane/navigation.cpp#L26-L36
This snippet in loiter_angle_update would work correctly if loiter_angle_reset also reset the flag in the navigation controller that tracks if we have reached the loiter target. I made a quick patch that added a function to clear the _WPcircle flag and that does work to fix this bug, but it's also less clear to me that it doesn't come with other unintended consequences.
@joshanne @robertlong13 I've rebased this PR on master
No worries, what's your thoughts on it being an actual issue?
@joshanne my apologies for forgetting about this one!
No worries, what's your thoughts on it being an actual issue?
yes, I think it is, we'll merge, thanks!
Merged, thanks!
I've pulled the code change into 4.6.2-beta1 but was unable to pull in the autotest change due to a merge conflict
This is included in 4.6.2-beta1 via https://github.com/ArduPilot/ardupilot/pull/30466