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

enable intra-process communication for point clouds #2599

Merged

Conversation

christian-rauch
Copy link
Contributor

The node currently publishes point cloud messages directly and does not use a unique shared pointer, which prevents using efficient zero-copy intra-process communication as it requires copying the data between nodes in the same process.

This PR fixes this by publishing the point cloud via a unique shared pointer. I verified by the address of the data vector that the camera node and a point cloud subscriber indeed share that message without copying data.

The PR also contains some cleanup commits and one commit to guard different method signatures behind a version check to enable compilation on Ubuntu 20.04 with ROS2 rolling. Note that those checks should be done by library version and not ROS2 distribution names, as the same ROS2 distribution (e.g. rolling in this case) can have different versions of a library on different Ubuntu versions.

@christian-rauch
Copy link
Contributor Author

@Nir-Az The CI is failing for galactic for the same reasons as in #2381 (comment) (Could not resolve the rosdep key 'ros_workspace'). Are there plans to fix the CI for galactic or can we merge this PR with those pipelines failing?

@Nir-Az
Copy link
Collaborator

Nir-Az commented Jan 18, 2023

@Nir-Az The CI is failing for galactic for the same reasons as in #2381 (comment) (Could not resolve the rosdep key 'ros_workspace'). Are there plans to fix the CI for galactic or can we merge this PR with those pipelines failing?

Thanks for the PR, looks like a great enhancement.
I agree about the versions vs distro check, thanks for that too.
We will review the PR and test it.
The galactic fail of pre-release can be ignored, looks like a regression from industrial-ci
The other failure seems like a network glitch in GHA, I reran it now lets see.. should be OK.

@christian-rauch
Copy link
Contributor Author

@Nir-Az Can you approve the CI pipeline for this PR?

@christian-rauch christian-rauch requested review from Nir-Az and removed request for SamerKhshiboun January 31, 2023 15:12
Copy link
Collaborator

@Nir-Az Nir-Az left a comment

Choose a reason for hiding this comment

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

Great enhancement, thanks!

@Nir-Az Nir-Az merged commit e795da7 into IntelRealSense:ros2-development Jan 31, 2023
@christian-rauch christian-rauch deleted the shared_point_cloud branch January 31, 2023 21:02
@tonynajjar
Copy link
Contributor

tonynajjar commented Jun 14, 2023

With this PR should the readme be updated? It still says that pointclouds are not supported for ipc

@Nir-Az
Copy link
Collaborator

Nir-Az commented Jun 14, 2023

With this PR should the readme be updated? It still says that pointclouds are not supported for ipc

You are right, thanks.
If you can PR it, it would be even awesome :)

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.

3 participants