-
Notifications
You must be signed in to change notification settings - Fork 17.2k
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
Rover: SCurve navigation #19220
Rover: SCurve navigation #19220
Conversation
Hi @rmackay9 looks very interesting! You refer to the "PosControl" starting to bring in velocity control - are you looking at introducing a longitudinal acceleration controller rather than / in addition to the speed controller, so there's some control over how a vehicle accelerates and decelerates? I'm very curious to see how this plays out. One of the challenges with the current nav system is that it's hard to get the vehicle to track smoothly through a predictable path through a corner without running a lot of maths - working out your speed, desired lateral acceleration then calculating the turn radius you want and then setting WP radius to give you that without overshoot!! A couple of test cases to look at will be how the new one handles corners of much less than and much greater than 90 degrees, and how closely you can place waypoints to control your path around a specific corner (i.e. following a road). Is there a paper you're working from? I'd be interested to read the theory. |
@mikerob, txs for the feedback. I'm hoping that this change greatly improves navigation through corners for Ackermann steering vehicles (aka separate steering and throttle vehicles). The SCurve feature (originally designed for multicopters) calculates the vehicle's path to the next waypoint and the one after that including the corner. This path includes the vehicle's target position, velocity and acceleration at each moment. The new PosControl class's purpose in life is to calculate the best target turn rate and speed to get the vehicle to stay on the path. By the way, the lower level forward-back speed controller has some parameters to control how the vehicle accelerates and decelerates... this PR doesn't change that part of the code instead focusing just on the higher level navigation. |
03d653d
to
663bc68
Compare
663bc68
to
4b64f58
Compare
c76ccf9
to
6fba31b
Compare
6fba31b
to
d08d3aa
Compare
I've begun alpha testing this change here: https://discuss.ardupilot.org/t/rover-s-curves-alpha-testing/79570 |
d08d3aa
to
cf89543
Compare
09f7283
to
ec52e6e
Compare
ec52e6e
to
d6e67f7
Compare
Setting ATC_ACCEL_MAX to 0 seemingly causes the code to crash when you try to enter AUTO mode. |
d6e67f7
to
340cc46
Compare
Thanks for the report. I've added protection and rebased on master. |
340cc46
to
de9bc30
Compare
|
||
// pass desired speed to throttle controller | ||
// do not do simple avoidance because this is already handled in the position controller | ||
calc_throttle(g2.wp_nav.get_speed(), false); | ||
|
||
float desired_heading_cd = g2.wp_nav.oa_wp_bearing_cd(); | ||
if (g2.sailboat.use_indirect_route(desired_heading_cd)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should be relaxing the position controller in this case? Were basically ignoring its return.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, you must be right. The proper solution would be to add a Sailboat specific version of AR_WPNav .. I might look into how difficult that is before adding the reset. txs again.
libraries/AP_Math/vector2.h
Outdated
@@ -89,6 +89,11 @@ struct Vector2 | |||
// dot product | |||
T operator *(const Vector2<T> &v) const; | |||
|
|||
// dot product for Lua |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this is supposed to be here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed.. I hadn't noticed that there's a method directly above that can be used instead. Copter is using Vector3f::dot() so I basically just copy-pasted that but now I see Copter could also be changed to use operator*. I suspect thought that Leonard would prefer to stick with "::dot()" because it's more clear than reusing "operator*()".
.. I don't feel strongly about it but I think I might just fix the comment above Vector2f::dot() but keep using it although it's slightly wasteful to have two methods that do the same thing.. ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also think using the dot
method is easier to follow. But there is value in keeping the code base consistent. Maybe we can change them all....
Rover/sailboat.cpp
Outdated
|
||
// waypoint navigation target should speedup if boat travels faster than WP_SPEED | ||
// instead of asking vehicle to slow down which sailboats cannot do | ||
rover.g2.wp_nav.enable_overspeed(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not always the case, we can be a 100% motoring mode. I think we can just update it in real time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks very much for this. I've changed this to be:
// sailboats without motors may travel faster than WP_SPEED so allow waypoint navigation to
// speedup to catch the vehicle instead of asking the vehicle to slow down
rover.g2.wp_nav.enable_overspeed(motor_state != UseMotor::USE_MOTOR_ALWAYS);
EDIT: sorry, I updated this a couple of times and I honed in on the easiest to understand comment and code. :-).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.. and as you say it needs to be updated each iteration.. clearly I need to think about this some more. txs.
also calc_crosstrack_error may return negative values
also remove unused set_desired_speed_to_default also init previous leg in set_desired_location init accepts speed but inforces minimum
set_speed_max updates position controller limits and triggers recalculation of scurves local _desired_speed member is no longer required because max speed is held in position controller
allows pilot to override speed_max from RC input
range checks are no longer required because they are implemented within AR_WPNav
…oller also navigate-to-waypoint may trigger tacking
SCurves do not work with very fast changes of target
vehicle's speed is slightly lower when using SCurve navigation
This PR adds support for SCurve navigation for Rovers and Boats which improves navigation including cornering. The most obvious change users will notice is how the vehicle cuts the corner rather than overshooting the waypoint. Either method is arguably correct but using SCurves also means that the route through the corners is planned while in master the controllers are essentially "surprised" by each corner in that they only ever have a single target and so when the vehicle reaches a waypoint they are simply handed a new waypoint and so they suddenly have a huge error they try to reduce. I.e. In master the corners are handled in a more reactive manner while with SCurves they are planned.
The high level changes are:
There are some addition drive-by changes to parameter defaults and Rover behaviour. These will eventually be separated out into separate PRs.
Remaining To-Do items:
Completed To-Do items:
Testing required:
Alpha testing thread is here