Skip to content

Include option to use PointCloud Transport #761

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.

Sign up for GitHub

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

Jump to bottom
Merged
SteveMacenski merged 4 commits into from
Aug 12, 2025
Merged

Include option to use PointCloud Transport #761

SteveMacenski merged 4 commits into from
Aug 12, 2025

Conversation

@elsayedelsheikh
Copy link
Contributor

@elsayedelsheikh elsayedelsheikh commented Aug 10, 2025

Updates related to ros-navigation/navigation2#5264
@elsayedelsheikh
Add configuration guides
691327a 
Signed-off-by: ElSayed ElSheikh <elsayed.elsheikh97@gmail.com>
SteveMacenski
SteveMacenski reviewed Aug 11, 2025
View reviewed changes
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.

LGTM! Can you add in the migration guide reference for support for point cloud transport compression in the collision monitor and costmap layers?
@elsayedelsheikh
Copy link
Contributor Author

elsayedelsheikh commented Aug 11, 2025

LGTM! Can you add in the migration guide reference for support for point cloud transport compression in the collision monitor and costmap layers?

Sure! :-)
@SteveMacenski
Copy link
Member

SteveMacenski commented Aug 11, 2025

@elsayedelsheikh I think that's all that's blocking here. I'd be curious if you had any perf metrics if using compression vs not using compression to share in it :-)
@elsayedelsheikh
Add migration guide
9efb80f 
Signed-off-by: ElSayed ElSheikh <elsayed.elsheikh97@gmail.com>
@elsayedelsheikh
make pre-commit happy
2218e94 
Signed-off-by: ElSayed ElSheikh <elsayed.elsheikh97@gmail.com>
@elsayedelsheikh
Copy link
Contributor Author

elsayedelsheikh commented Aug 12, 2025 •
edited
Loading

@SteveMacenski Check this migration guide.
I looked for benchmarking tools for these transports, also checked Cloudini repo, but only found a mention of rosbag_benchmark I can dig deeper if needed.

The guide currently covers bandwidth only for performance metrics as I didn't mention encoding/decoding speed, lossy vs. lossless, or CPU load. I guess if the sensor manufacturer provides point_cloud_transport support out of the box, it’s likely just a matter of selecting the right transport_type for Nav2.
SteveMacenski
SteveMacenski requested changes Aug 12, 2025
View reviewed changes
@elsayedelsheikh
Feedback
cac522c 
Signed-off-by: ElSayed ElSheikh <elsayed.elsheikh97@gmail.com>
@elsayedelsheikh elsayedelsheikh force-pushed the include_point_cloud_transport branch from 0ff80e9 to cac522c Compare August 12, 2025 16:36
@SteveMacenski SteveMacenski merged commit bb3f0b9 into ros-navigation:master Aug 12, 2025
4 checks passed
@elsayedelsheikh elsayedelsheikh deleted the include_point_cloud_transport branch August 12, 2025 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants

@elsayedelsheikh @SteveMacenski