-
Notifications
You must be signed in to change notification settings - Fork 191
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
Add voxel_min_points param for voxel filter (ROS2) #161
Add voxel_min_points param for voxel filter (ROS2) #161
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same items from ROS1
README.md
Outdated
@@ -116,6 +116,7 @@ rgbd_obstacle_layer: | |||
inf_is_valid: false #default false, for laser scans | |||
clear_after_reading: true #default false, clear the buffer after the layer gets readings from it | |||
voxel_filter: true #default off, apply voxel filter to sensor, recommend on | |||
voxel_min_points: 10 #default 0, minimum points per voxel for voxel filter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change this to 0
.
src/measurement_buffer.cpp
Outdated
@@ -154,6 +154,7 @@ void MeasurementBuffer::BufferROSCloud( | |||
sor.setDownsampleAllData(false); | |||
float v_s = static_cast<float>(_voxel_size); | |||
sor.setLeafSize(v_s, v_s, v_s); | |||
sor.setMinimumPointsNumberPerVoxel((unsigned int) _voxel_min_points); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use static_cast<unsigned int>(_voxel_min_points);
There's an issue with minimum points per voxel in PCL, so just wanted to confirm what I'd expect, which is that the feature added in this PR doesn't really have any effect when applied?? Unless it's been fixed in newer versions of PCL... |
I'm not sure - I haven't tested this personally, @akiroz I would have thought did since they proposed it |
We actually ended up not using this param in the end but I believe it's been fixed in PCL upstream and planned for the coming 1.12 release. |
See #160