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

Allow pointcloud create_cloud function to set specific point_step #223

Merged

Conversation

broughtong
Copy link
Contributor

If I try to convert a pointcloud message to an array, and then use create_cloud to convert it back to a message, it will not work if there is padding at the end.

The original lidar data is laid out like this (note the itemsize/padding):

{'names':['x','y','z','intensity','t','reflectivity','ring','ambient','range'], 'formats':['<f4','<f4','<f4','<f4','<u4','<u2','<u2','<u2','<u4'], 'offsets':[0,4,8,16,20,24,26,28,32], 'itemsize':48}

The current create_cloud will try to reconstruct the point layout from the field info with dtype_from_fields, which in this case will take into account field padding, but not the padding at the end. It generates the following (again note the itemsize):

{'names':['x','y','z','intensity','t','reflectivity','ring','ambient','range'], 'formats':['<f4','<f4','<f4','<f4','<u4','<u2','<u2','<u2','<u4'], 'offsets':[0,4,8,16,20,24,26,28,32], 'itemsize':36}

The mismatch in data layout will fail an assertion.

The dtype_from_fields function does allow for the point_step (itemsize) to be specficied explicitly when there is padding present at the end of the data, but there is currently no way to pass this in from the create_cloud function.

Fix adds an optional parameter to allow users to pass in a custom point_step, allowing the data to align properly.

@broughtong broughtong requested a review from tfoote as a code owner May 12, 2023 14:05
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Can you please add a test showing that this PR fixes the problem you are describing?

@broughtong
Copy link
Contributor Author

Done :)

@tfoote tfoote requested a review from clalancette May 31, 2023 08:37
Signed-off-by: George Broughton <broughton_g182@hotmail.com>
Signed-off-by: George Broughton <broughton_g182@hotmail.com>
@broughtong broughtong force-pushed the create_cloud_specify_point_step branch from b689b06 to 980c3f4 Compare June 5, 2023 11:35
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

This looks reasonable to me, thanks for the additional tests. I'll run CI on it next.

@clalancette
Copy link
Contributor

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@clalancette clalancette merged commit bf7dbe8 into ros2:rolling Jun 5, 2023
3 checks passed
tfoote pushed a commit that referenced this pull request Apr 10, 2024
* Allow pointcloud create_cloud function to set specific point_step

Signed-off-by: George Broughton <broughton_g182@hotmail.com>
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.

None yet

2 participants