search

Alpaca-zip / ultralytics_ros

ROS/ROS 2 package for Ultralytics YOLOv8 real-time object detection and segmentation. https://github.com/ultralytics/ultralytics
GNU Affero General Public License v3.0
190 stars 37 forks source link

Adding excluded points publisher in `track_with_cloud_node`. #57

Open h-wata opened 7 months ago

h-wata commented 7 months ago

Branch

noetic-devel

Description

Thanks to sharing great project.

I've noticed that the tracker_with_cloud node currently only publishes /detection_points. However, it would be highly beneficial to also have it publish excluded_points, which would be the original Point Cloud excluding the /detection_points.

For example, when doing Mapping or Localization, we may need a point cloud excluding people and cars. I think excluded_points would be useful in such cases.

Additional

I've implemented the publisher in my fork of the project. If you're interested, I'd be happy to send over a PR. Just let me know!

Are you willing to submit a PR?

Alpaca-zip commented 7 months ago

Thank you @h-wata for your report! 👍🏻

Your idea is quite interesting and valuable, especially for SLAM where filtering out dynamic objects such as people and cars is essential. Since the KITTI datasets come with IMU data, it could be easy to evaluate this concept with SLAM like LIO-SAM.

I would really appreciate it if you could submit a Pull Request. Your contribution could significantly enhance the project's utility.

On another note, I am considering updating the noetic-devel branch to align with humble-devel. Specifically, I will move the VoxelGrid filter from euclideanClusterExtraction() to syncCallback().

https://github.com/Alpaca-zip/ultralytics_ros/blob/a8a6a7e4d4a9a2609148f9dc8a6eed45d2ce9804/src/tracker_with_cloud_node.cpp#L53-L80

https://github.com/Alpaca-zip/ultralytics_ros/blob/a8a6a7e4d4a9a2609148f9dc8a6eed45d2ce9804/src/tracker_with_cloud_node.cpp#L266-L304

In that case, Instead of creating a new function like processPointsWithMask(), it may be more efficient to refine euclideanClusterExtraction() to directly extract the removed_cloud from pcl::PointIndices.

Best regards,

h-wata commented 7 months ago

Thank you for your replay and sharing the road map of this project.

In that case, Instead of creating a new function like processPointsWithMask(), it may be more efficient to refine euclideanClusterExtraction() to directly extract the removed_cloud from pcl::PointIndices.

I think your suggestion is a good idea, but how do you deal with the case of multiple targets? I am concerned that combining removed_cloud with another removed_cloud, as we do with detection_cloud, might recreate the original point cloud. Wouldn't it be simpler to exclude combine_detection_cloud from the original point cloud for clarity?

Alpaca-zip commented 7 months ago

I think your suggestion is a good idea, but how do you deal with the case of multiple targets? I am concerned that combining removed_cloud with another removed_cloud, as we do with detection_cloud, might recreate the original point cloud. Wouldn't it be simpler to exclude combine_detection_cloud from the original point cloud for clarity?

I see that your point is valid after looking at the code again. It looks like significant changes to the projectCloud() function would be necessary to split the original point cloud into combine_detection_cloud and excluded_cloud using pcl::PointIndices.

Considering a short-term implementation of this feature, I think your approach should be fine. Could you please go ahead and submit a pull request? I'll do a functionality check and a detailed code review.

However, due to my current academic schedule, I may be a bit slow to respond. Thank you for your understanding.

Best regards,