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

Fix off-track tracking in AutoLineSmoothVel #13321

Merged
merged 8 commits into from
Nov 4, 2019
Merged

Fix off-track tracking in AutoLineSmoothVel #13321

merged 8 commits into from
Nov 4, 2019

Conversation

jkflying
Copy link
Contributor

@jkflying jkflying commented Oct 30, 2019

Describe problem solved by this pull request
Before, if a vehicle was pulled very off-track (eg. by the obstacle avoidance system) the smooth dynamics planning would plan wrong routes, assuming it was still on-track

In addition, there were some cases where code calculating the optimal speed to go through a waypoint had very complicated / hard to follow logic, which was potentially also a source of problems.

Describe your solution
This fixes the angle that is used for the planning, so it uses the angle from the drone instead of from the previous waypoint as the entry angle into the next waypoint acceptance radius.

It also simplifies the logic by adding the capability to 'chain' the constraints from later setpoints' velocities, so that we don't have to override the smoothing to move nicely through waypoints that we don't need to turn for.

Also, I got rid of the MPC_XY_TRAJ_P for the straight line stuff, since it wasn't doing anything in my tests - it is still being used for the acceptance radius calculation. Maybe I'm wrong on this and we should add it back in.

Describe possible alternatives
I used a single speed term instead of separate x/y components, but I didn't see an easy way to do separate x/y without rewriting the whole thing. Not sure if this is an issue, but it is also way simpler this way.

Test data / coverage
I've flown this in SITL a fair amount.

Current master doesn't fix the tracking on the line after an overshoot. What isn't visible here is that the vehicle actually flew sideways, with yaw pointing towards the setpoint but continuing parallel to the flight line.
image
https://logs.px4.io/plot_app?log=020c2e97-f6db-4b94-a7d3-059876cb0011

This PR tracks the overshoot, correcting it much better, and always flying with yaw straight ahead.
image
https://logs.px4.io/plot_app?log=0abcbfa8-ffc5-4664-86b1-b8d665f38497

Both the above flown with Iris, without Avoidance running

Additional context
IMO this should be pulled into 1.10, it is a pretty serious fix for Avoidance which pushes the vehicle offtrack as part of normal operation. FYI @dagar @julianoes

@bresch
Copy link
Member

bresch commented Oct 30, 2019

@jkflying Awesome! However, something seems quite wrong in one of the waypoint:
bokeh_plot(30)

Compared to before (which was also wrong because not aiming at the next waypoint):
bokeh_plot(31)

@jkflying
Copy link
Contributor Author

Attached is the test flight plan I used
smooth_investigation.zip

@jkflying
Copy link
Contributor Author

jkflying commented Oct 31, 2019

So this issue is being caused by YAW_MODE=4, it tracks on the outside which then causes it to stop because the yaw setpoint is just a tiny bit higher than on the old implementation, due to pointing at the setpoint instead of along the flight line. Using YAW_MODE=1 I don't see this issue at all.

However, I've re-written it to use separated x/y components, since it wasn't too hard and the behavior is more similar to what was there before in terms of separate constraints.

Here is a log after the changes, with YAW_MODE=1:
https://logs.px4.io/plot_app?log=9b992506-a7c3-42a9-87dd-c56eb1a95d6f
image

@jkflying
Copy link
Contributor Author

jkflying commented Oct 31, 2019

Ok, a small fix, and another comparison to master.

Master: notice how the flight lines in each segment are parallel to the line between the waypoints, rather than towards the next waypoint. They are a little offset.
https://logs.px4.io/plot_app?log=136c174b-87b2-4fb8-b881-7fb26c8d7f76
bokeh_plot(6)

This PR: lines between waypoints always point directly to the next waypoint.
https://logs.px4.io/plot_app?log=73ffd69d-c7eb-417e-aa16-761a179c677a
bokeh_plot(7)

The small divergences at the end (in both master and this PR) are due to the waypoint acceptance radii overlapping, so it starts going to the next waypoint before reaching this one.

Multicopter automation moved this from In progress to Reviewer approved Oct 31, 2019
bresch
bresch previously approved these changes Oct 31, 2019
Copy link
Member

@bresch bresch left a comment

Choose a reason for hiding this comment

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

Awesome! I especially like the fact that it improves the functionality and simplify the code.

@ndepal
Copy link
Contributor

ndepal commented Oct 31, 2019

In the "this PR" plot, is there a dent in the setpoint line right around the (37, 20) coordinate?

@jkflying
Copy link
Contributor Author

In the "this PR" plot, is there a dent in the setpoint line right around the (37, 20) coordinate?

There is, I'm honestly not sure where it's from, but it is very small. I've run a bunch of tests in SITL and haven't seen anything else strange related to this. It might be related to doing separate planning in x and y axes?

@bresch One other thing I noticed, which is an issue already existing in master, is that we overshoot waypoints which the vehicle is meant to stop at (eg start of survey), because we no longer slow down enough to stop at them.

This calculates the target velocities better taking into account disturbances along
the flight route. Previously entry angles and more were calculated assuming the flight path
originates directly from the direction of the previous waypoint. This corrects this assumption
to instead make the direction come from the vehicle location.
This is to allow planning to not stop at a waypoint, but instead
to reach the waypoint while maintaining a certain velocity
Multicopter automation moved this from Reviewer approved to Review in progress Nov 1, 2019
Multicopter automation moved this from Review in progress to Reviewer approved Nov 1, 2019
bresch
bresch previously approved these changes Nov 1, 2019
@bresch
Copy link
Member

bresch commented Nov 1, 2019

@PX4/testflights Can you please fly that PR on a few multicopters please? Especially auto modes (takeoff, RTL, mission, land).

@jorge789
Copy link

jorge789 commented Nov 1, 2019

Tested on PixRacer V4:

Mode Tested: Mission Plan Mode (Automated):

  • Armed from QGC.
    Took-off on mission plan mode, the commands were sent form QGC (Armed and takeoff)
    Once all waypoint completed vehicle switched to RTL and landed.

Note
When the vehicle was landing, and I touched the ground, it began to make small leaps but did not land completely after a few seconds, it was necessary to land manually. Loaded Beta v1.10.0 (beta) (5c6d16c) there was no issue, the vehicle disarmed upon landing.

Logs:
https://review.px4.io/plot_app?log=8c3fee44-1753-4f36-b86e-31f9dc4d58f8

https://review.px4.io/plot_app?log=8674017d-e90c-492a-8a23-6fb889fb39a8

Comparison with v1.10.0 (beta) (5c6d16c)
Log: https://review.px4.io/plot_app?log=fa6fda58-a1e5-4713-a1aa-ac6c1563730a

@bresch
Copy link
Member

bresch commented Nov 4, 2019

@jorge789 Thanks for the tests. The landing problem is apparently an issue on current Master (unrelated to this PR), we're working on it.

@jkflying I checked the logs and everything seems good regarding this PR, I'm re-running CI and we can merge that once completed.

Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

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

Nice! Just some small comments otherwise I would merge it and flag also to take it in for the release.

Multicopter automation moved this from Reviewer approved to Review in progress Nov 4, 2019
Multicopter automation moved this from Review in progress to Reviewer approved Nov 4, 2019
@bresch bresch merged commit b8b7527 into master Nov 4, 2019
Multicopter automation moved this from Reviewer approved to Done Nov 4, 2019
@bresch bresch deleted the offtrack-smooth branch November 4, 2019 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Multicopter
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants