-
Notifications
You must be signed in to change notification settings - Fork 13.4k
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
AutoLineSmoothVel refactoring & bugfixes, 2nd Attempt #13596
Conversation
Tested on Pixhawk4 v5 f-450 Reduced: MPC_ACC_DOWN_MAX = 10 / MPC_Z_VEL_MAX_DN = 2.0 Observations: |
Tested on PixRacer V4:MPC_ACC_DOWN_MAX =3 A mission was made with change of altitude from 20m to 40m
Log: https://review.px4.io/plot_app?log=05394dde-32c1-440d-bdcd-05d6a2aafe9d A mission was made in a straight line with change of altitude.armed form of mi-QGC. Log: https://review.px4.io/plot_app?log=5720d298-4005-4f95-88b1-710d657ba171 A mission with altitude change was carried out. Armed form of mi-QGC. Log: https://review.px4.io/plot_app?log=205be5e3-145f-4122-9333-a162ffe88d10 Tested on PixRacer V4:MPC_ACC_DOWN_MAX =2 A mission was made with change of altitude from 20m to 40m
Log: https://review.px4.io/plot_app?log=d1df9438-0bdc-444b-9b5d-e1f54a3d3499 A mission was made in a straight line with change of altitude.armed form of mi-QGC. Log: https://review.px4.io/plot_app?log=7cfb0d0b-2464-4740-8f4e-40c63473fdbf A mission with altitude change was carried out. Armed form of mi-QGC. Log: https://review.px4.io/plot_app?log=c58ae023-a933-4958-bc95-2a33c32224e2 Observations: |
Tested on NXP FMUK66 V3:MPC_ACC_DOWN_MAX = 3 A mission was made with change of altitude from 20m to 40m
Log: https://review.px4.io/plot_app?log=30ad89e1-f35c-414f-8df1-4df0d91e7773 A mission was made in a straight line with change of altitude.armed form of mi-QGC. Log: https://review.px4.io/plot_app?log=c48f2142-f534-42ab-a8f5-0d450792b607 A mission with altitude change was carried out. Armed form of mi-QGC. Log: https://review.px4.io/plot_app?log=77a3b3b8-86e2-47b3-a46d-feb5aeb8b1b0 Tested on NXP FMUK66 V3:MPC_ACC_DOWN_MAX = 2 A mission was made with change of altitude from 20m to 40m
Log: https://review.px4.io/plot_app?log=d3735225-b09f-4eb6-bd2e-b57d18bc4dfa A mission was made in a straight line with change of altitude.armed form of mi-QGC. Log: https://review.px4.io/plot_app?log=07e92257-2fd8-40f4-bab7-f0c513bdc85f A mission with altitude change was carried out. Armed form of mi-QGC. Log: https://review.px4.io/plot_app?log=603243dd-8d8b-409f-b230-28df55ab69b4 Observations: |
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.
Awesome work! This will help a lot for obstacle avoidance and we can soon ask Navigator to provide more waypoints and perform the actual switching in the FlightTask.
Just a few style comments, but it looks good for the rest. I also had a quick look at the logs of the test team and I didn't see anything weird.
src/lib/flight_tasks/tasks/AutoLineSmoothVel/TrajectoryConstraints.hpp
Outdated
Show resolved
Hide resolved
src/lib/flight_tasks/tasks/AutoLineSmoothVel/TrajectoryConstraints.hpp
Outdated
Show resolved
Hide resolved
src/lib/flight_tasks/tasks/AutoLineSmoothVel/TrajectoryConstraints.hpp
Outdated
Show resolved
Hide resolved
src/lib/flight_tasks/tasks/AutoLineSmoothVel/TrajectoryConstraintsTest.cpp
Outdated
Show resolved
Hide resolved
src/lib/flight_tasks/tasks/AutoLineSmoothVel/TrajectoryConstraintsTest.cpp
Outdated
Show resolved
Hide resolved
src/lib/flight_tasks/tasks/AutoLineSmoothVel/TrajectoryConstraintsTest.cpp
Outdated
Show resolved
Hide resolved
src/lib/flight_tasks/tasks/AutoLineSmoothVel/TrajectoryConstraintsTest.cpp
Outdated
Show resolved
Hide resolved
@mrivi any idea why the safe landing planner SITL is failing here? Maybe we can discuss in more detail the control flow there to make sure this isn't breaking anything? |
@jkflying any news on that? |
I spoke with her offline, I need some time to discuss with you how this stuff interacts with something setting a setpoint (not waypoint) that isn't on the flight plan. Maybe we need to make it a 4-point optimization with the first being the setpoint? Anyway, it works well as long as the avoidance interface isn't involved, but needs a little rethink about how that fits in. |
Ok, after further discussion with @bresch it seems possible to change the avoidance interface to use the waypoint instead of the setpoint (so no changes here), we just need to check what is being sent to the avoidance so that it doesn't end up following its own waypoints thinking they are mission waypoints. |
@bresch can you take another look, short of reworking the entire obstacle avoidance interface I think this is the best I can do here. Longer term, I think reworking the obstacle avoidance interface is the right move though. |
310ecc6
to
bbfadcc
Compare
src/lib/flight_tasks/tasks/AutoLineSmoothVel/FlightTaskAutoLineSmoothVel.cpp
Outdated
Show resolved
Hide resolved
src/lib/flight_tasks/tasks/AutoLineSmoothVel/FlightTaskAutoLineSmoothVel.cpp
Outdated
Show resolved
Hide resolved
ed670af
to
42fb72b
Compare
Successful flight testing, at higher speeds too. |
I made some space by removing the |
ec00de2
to
7e78eed
Compare
I flew this PR this morning and I didn't find any issue |
I resolved the conflicts, they were cleanups in files that I removed in the last commit. Also I removed the merge commit for master that was in between. |
Describe problem solved by this pull request
2nd attempt at #13460 , which had a bug which made the vehicle fly faster than desired.
This is mostly refactoring, but it also makes the following minor behavior changes:
Describe your solution
The waypoint speed constraint logic was refactored to allow theoretically any number of setpoints to be chained together. This allowed the other logic fixes to be easily implemented, and will allow the obstacle avoidance interface to make use of this in the future as well, via changing the targets rather than the setpoints.
Test data / coverage
Tested extensively in SITL, and a number of unit tests added on the code as it was refactored.
@PX4/testflights could you please test fly this? It needs several different shaped mission flights.
Please add large altitude changes part-way through the flight. eg. on a straight line, add a 20m altitude change.
Also, please note that the altitude change might be more aggressive now, so you might have to reduce the MPC_ACC_DOWN_MAX / MPC_Z_VEL_MAX_DN if there is instability during descent.