-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
Collision Prevention for multicopter acceleration based position flight mode #23507
base: main
Are you sure you want to change the base?
Conversation
a3d8d8b
to
d130612
Compare
dcfa190
to
9cf4838
Compare
@Claudio-Chies is this something I can pull down to flight test? Or is it needing more time in the SITL? |
@dirksavage88 it should be ready to test, the only thing i haven't tested is how non-default gains in the position and velocity controller change the CP behavior |
7fa1fcc
to
077226b
Compare
1e40431
to
bd72ecf
Compare
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.
After an in person discussion I see a lot of value in simplifying the logic. The main pain points are:
- Separate input data processing from guidance logic e.g. distance and direction to closest obstacle and distance to next obstacle in current velocity direction are the interface between obstacle map processing and guidance.
- If possible avoid cases in guidance logic. E.g. three things to take care of:
- Stick should not accelerate over the configured collision prevention minimum distance
- Brake from current velocity before before minimum allowed distance
- If obstacle is closer/"comes closer" push away from it to minimum distance
Probably these rules can run all the time, get superimposed and be readable, predictable (logged).
I added a commit for cosmetic and commenting things that I saw at a glance.
e0b6707
to
d346da5
Compare
6d72e11
to
1075492
Compare
c5834b2
to
de8fc84
Compare
@MaEtUgR can we change this to be a param?
The original collision prevention used point-lidar sensors with low resolution, but now we have advanced rotating lidars with <1 degree resolution per sample, so being able to set to the lowest increment value of 5 makes sense |
don't use it in header such that clients are free to redefine the names but include it in cpp files and make use of that.
I just tested with subtfinals on an inside corner and it no longer ping pongs between the corner walls, but it does get stuck. But overall much better. I think the lidar model on the x500 does not have rear facing coverage |
This should rather be dependent on the input interface resolution PX4-Autopilot/msg/ObstacleDistance.msg Line 17 in 4f00df6
or if that's too complicated we just run with the highest resolution and just fill the unknown directions accordingly when the resolution is lower. |
@MaEtUgR we have the obstacle distance increment set to 5 deg on the SF45 driver currently. However @Claudio-Chies is wanting to change it to 10 to match the collision prevention lib. |
@dirksavage88 I see. I'd say let's change both to 5° and check it doesn't break things. Ideally, collision prevention would adapt to the input data but I assume it doesn't explode to run through the 72 bins. |
It could cause array out of bound problems before.
f179c2c
to
70b36f2
Compare
@dirksavage88 sounds good, then let's switch the driver and Collision Prevention to 5. |
35b7e07
to
6be5dd0
Compare
Well well, your commit breaks the unit tests and mine breaks the unit test build 🙈 Let's roll back and fix. I have multiple suggestions already but I'd like to have constantly passing unit tests as a first barrier to not break things. |
5ef7c68
to
c66b03a
Compare
I fixed all my commits but had to remove yours which both line changes broke the unit tests. Here's it for reference: 6be5dd0 |
The default implementation for multicopter Position mode is FlightTaskManualAcceleration. The last missing piece was support for CollisionPrevention in this implementation.
Note, this will need a docs update (but should be fairly easy) |
One open point for discussion:
|
efc403c
to
efb2015
Compare
efb2015
to
41d95bc
Compare
fixes issues with offsets and resolution move GZ Bridge
Solved Problem
Ports and extends the current collision prevention algorithm to FlightModeManualAcceleration
Associated PR's
Solution
The algorithm has two parts, velocity compensation, and constraining of the commanded acceleration.
Velocity compensation
For this, I have ported over the existing methods from the velocity based collision prevention method and transformed the velocity into an acceleration by subtracting the velocity set-point from the collision prevention velocity and applying a proportional gain ( MPC_XY_VEL_P_ACC ) to it.
Acceleration constraining
For this I'm calculating the minimal distance to the obstacle, and scaling it quadratic in the range of [0,1] distances proportional to the maximum velocity. (See image below)
For this we split the acceleration setpoint coming from the operator into a surface normal and tangential component, then using the normal and tangential obstacle distance we scale these components individual.
This way, if we are in front of a big even wall, we are free to move tangentially but are restricted in the surface normal direction.
whereby, the acceleration constraining is done in the distance between CP_DIST and MPC_VEL_MANUAL / MPC_XY_P
Drawbacks
Removed in 87a0ee6
Changelog Entry
For release notes:
Documentation: Need to update https://docs.px4.io/main/en/computer_vision/collision_prevention.html
WIP PX4/PX4-user_guide#3364
Drawbacks
Alternatives
Test coverage
Context
CP.mp4
Hardware Tests
1D Lidar
The oscillations which are present are due to flying close to the ground, and when pitching the distance sensor picks up the ground.
EDIT1:
Updated description to include tangential scaling