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

AR_WPNav: Rover 4.3+ does not honor WP_RADIUS #23457

Closed
yuri-rage opened this issue Apr 12, 2023 · 5 comments
Closed

AR_WPNav: Rover 4.3+ does not honor WP_RADIUS #23457

yuri-rage opened this issue Apr 12, 2023 · 5 comments
Labels

Comments

@yuri-rage
Copy link
Contributor

yuri-rage commented Apr 12, 2023

_reached_destination = near_wp || past_wp;

The above referenced line is only ever reached when the vehicle has passed the destination waypoint (a result of s_finished remaining false until enough time has elapsed to have arrived at the destination). Thus, there is always an overshoot of the desired waypoint, which necessitates a course correction upon pivot completion.

If I change the logic to ignore s_finished except during fast waypoints, then _reached_destination becomes true in a timely manner, but it seems the controller is still awaiting time elapsed on that leg before initiating a pivot, and the resulting behavior is actually worse:

    // check if we've reached the waypoint
    if (!_reached_destination) {
        // "fast" waypoints are complete once the intermediate point reaches the destination
        if (_fast_waypoint && s_finished) {
            _reached_destination = true;
        } else {
            // regular waypoints also require the vehicle to be within the waypoint radius or past the "finish line"
            const bool near_wp = current_loc.get_distance(_destination) <= _radius;
            const bool past_wp = current_loc.past_interval_finish_line(_origin, _destination);
            _reached_destination = near_wp || past_wp;
        }
    }

On the other hand, if I use a Lua script (workaround) to force the WP ahead when WP_RADIUS is satisfied, the vehicle behaves extremely well, so I'm certain that I'm on the right track.

I suspect this is among the chief reasons that the community has had a hard time tuning the new nav controller. Any vehicle that is slightly "loose" in deceleration will overshoot nearly every waypoint, sometimes by a significant margin, causing path control issues thereafter.

@xianglunkai
Copy link
Contributor

I think you're right. Last year, I conducted a investigation into Scuve NAV for a long time.
In addition, the new navigation is difficult to solve the obstacle avoidance problem with relatively low acceleration(<=0.1m/s2); And the issue of returning to the route after avoiding obstacles, although I have done some work, I believe that it is not coordinated and graceful enough within the current framework

@xianglunkai
Copy link
Contributor

#22957

@rmackay9
Copy link
Contributor

Thanks for the investigation. The SCurves will slow their "time" state to handle slow moving vehicles but it doesn't support speeding up time to handle overspeeding vehicles.. the idea was that a vehicle can always slow down but it may not be able to speed up... maybe that assumption is incorrect and we should allow time to being sped up slowed down... within limits of course.

@rmackay9 rmackay9 added the Rover label Apr 13, 2023
@yuri-rage
Copy link
Contributor Author

Rather than a speed inaccuracy, I think the problem may be that segment time is calculated to the waypoint itself rather than waypoint distance minus radius. I couldn't quite find where the end_time value is assigned, so that is only a guess.

@rmackay9
Copy link
Contributor

We've merged PR #25623 which I think resolves the overshoot issue so I will go ahead and close this as is normal when we merge a fix to master.

As a follow-up I will also backport the above fix and release as a Rover beta release in the coming week

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

No branches or pull requests

3 participants