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

Feat/Add PointCloud2 sensor #22

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

elsayedelsheikh
Copy link

Add option for publishing PointCloud2

Could've just add it directly to the bridge config file tb4_bridge.yaml, but it's totally separated so user can choose whether to publish PointCloud2 or not "Default" keeping the package minimal as possible

ros2 launch nav2_minimal_tb4_sim simulation.launch.py use_3dlidar:=True

@elsayedelsheikh elsayedelsheikh force-pushed the feature/add_velodyne_3d_lidar_sensor branch from d3c2751 to 7e9b330 Compare January 24, 2025 01:31
@elsayedelsheikh elsayedelsheikh changed the title Add PointCloud2 sensor for tb4 Feat/Add PointCloud2 sensor Jan 24, 2025
Copy link
Member

@SteveMacenski SteveMacenski left a comment

Choose a reason for hiding this comment

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

More philosophical, why add in a 3D lidar rather than using the PointCloud2's generated from the OAKD depth camera that's already in the model? That is a pointcloud just like any 3D lidar has a pointcloud (just different dimensions).

So, this may not be necessary for the pointcloud transport conversation? Though curious what you think / feel -- we could make this change

<gazebo reference="${name}_link">
<xacro:ray_sensor sensor_name="${name}" use_vertical_rays="true" gazebo="${gazebo}"
update_rate="10.0" visualize="true"
h_samples="360" h_res="1.0" h_min_angle="0.000" h_max_angle="6.280000"
Copy link
Member

Choose a reason for hiding this comment

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

Are these representative of a velodyne sensor?

Copy link
Author

Choose a reason for hiding this comment

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

I used arbitrary values, while these are the sensor dimensions and specs.
Just providing a template for 3D LIDARs 🥲

Copy link
Member

@SteveMacenski SteveMacenski Jan 28, 2025

Choose a reason for hiding this comment

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

I'd quickly look up the current specs for a mid-range 3D lidar (that you can actually buy; I think Velodyne brand is end of life?) and update them to be realistic / based on something in particular - especially if we want to take metrics about its performance with respect to! Then, update the name of the file to match that current 3D lidar option

remappings=[
('/scan/points', '/pointcloud')
],
arguments=['/scan/points@sensor_msgs/msg/PointCloud2[gz.msgs.PointCloudPacked'],
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

The gazebo topic /scan/points is actually available for the 2D lidar so in that case it would publish the scan points as PointCloud2 as well but I think I took it far by adding an argument to choose whether to bridge it or not.
And at some point, it could be replaced with ros_gz_point_cloud

Copy link
Member

@SteveMacenski SteveMacenski Jan 28, 2025

Choose a reason for hiding this comment

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

Have the 3D lidar publish to a different topic? Overriding scan probably isn't good here anyway, thats usually a name for 2D lidars

@elsayedelsheikh
Copy link
Author

More philosophical, why add in a 3D lidar rather than using the PointCloud2's generated from the OAKD depth camera that's already in the model? That is a pointcloud just like any 3D lidar has a pointcloud (just different dimensions).

I thought it may turn out to be useful for other applications as well that take advantage of 360°-long range data either it's SLAM /or MOT "Multi-Object Tracking"

So, this may not be necessary for the pointcloud transport conversation? Though curious what you think / feel -- we could make this change

Yeah, The camera is already there. but why not provide 3D LIDAR as well and benchmark these sensors in terms of bandwidth, latency, ....

LMK what you think and if it would add any value :)

@SteveMacenski
Copy link
Member

SteveMacenski commented Jan 28, 2025

I thought it may turn out to be useful for other applications as well that take advantage of 360°-long range data either it's SLAM /or MOT "Multi-Object Tracking"

👍 PS We have something like that from an AMD project with a clearpath jackal and OS-0 lidar https://github.com/open-navigation/opennav_amd_demonstrations/tree/main/honeybee_description. Might not be the worst idea to test against that instead since its just 'done'?

Yeah, The camera is already there. but why not provide 3D LIDAR as well and benchmark these sensors in terms of bandwidth, latency, ....

👍 Then I think we shuold make sure we're setting realistic 3D lidar update rate, resolutions, beams, etc so that its representative.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants