-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
Removed Functionality: Remove collision avoidance interface on PX4 side #24172
base: main
Are you sure you want to change the base?
Conversation
🔎 FLASH Analysispx4_fmu-v5x [Total VM Diff: -10248 byte (-0.5 %)]
px4_fmu-v6x [Total VM Diff: -10376 byte (-0.53 %)]
Updated: 2025-01-15T09:25:46 |
Plus the corresponding mavlink pieces once these are gone. #24172 (comment) |
76e5487
to
6bdddd4
Compare
Signed-off-by: Silvan <[email protected]>
Remove yaw_acceptance and altitude_acceptance_radius fields as they were only filled by now removed avoidance controller. Signed-off-by: Silvan <[email protected]>
Signed-off-by: Silvan <[email protected]>
- TrajectoryBezier.msg - TrajectoryWaypoint.msg - VehicleTrajectoryBezier.msg - VehicleTrajectoryWaypoint.msg Additionally remove TRAJECTORY_REPRESENTATION_WAYPOINTS mavlink stream. Signed-off-by: Silvan <[email protected]>
Signed-off-by: Silvan <[email protected]>
As it is no longer needed w/o avoidance. Signed-off-by: Silvan <[email protected]>
6bdddd4
to
8b09e7c
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.
I rebased and removed some more:
- Heartbeat processing for the obstacle avoidance MAVLink component since you already removed the handling for it
- ROS1 Gazebo classic test which you had already removed the target for in the Makefile
Please double-check. Also we should probably mark the MAVLink messages that are no longer supported as deprecated to avoid confusion.
Signed-off-by: Silvan Fuhrer <[email protected]>
Signed-off-by: Silvan Fuhrer <[email protected]>
…quired Signed-off-by: Silvan Fuhrer <[email protected]>
This pull request has been mentioned on Discussion Forum for PX4, Pixhawk, QGroundControl, MAVSDK, MAVLink. There might be relevant details there: |
Waiting with merging due to the topic version bump it includes (68be795) for which, after my understanding, we need the translation mechanism in first. @dagar @MaEtUgR @DanMesh beside the missing translation layer - good to go from your side? |
Solved Problem
We have archived the ROS side of the avoidance some time ago, and the interface on the PX4 side is not actively maintained (most of the initial contributors have left or have a changed focus). With the introduction of external flight modes we have arguably a better way of implementing it, and thus I propose to remove the interface on the PX4 side completely. This saves us quite some flash (5.5k), removes code complexity and prevents new developers to be sent on the wrong (old) track.
Solution
Remove CollisionAvoidance library (which was used in FlightTask), remove health check for avoidance, remove
COM_OBS_AVOID
, remove from messagePositionControllerStatus.msg
the fieldsyaw_acceptance
andaltitude_acceptance
that were exclusively used for avoidance and with that also remove some acceptance logic from Navigator.Changelog Entry
For release notes:
Alternatives
We could for a start only not build it but leave in code base (as it's already done for constrained builds). If somebody would complain we could revert more easily. Wouldn't resolve the lingering of a not-maintained PX4 code then though.