-
Notifications
You must be signed in to change notification settings - Fork 800
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
Port Add height of cluster with the most points method #323 to master #329
base: master
Are you sure you want to change the base?
Port Add height of cluster with the most points method #323 to master #329
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.
LGTM.
Hey, some changes have been merged, or will be merged, can you update the MR? |
@maximilianwulf |
5e41cda
to
0cc1b53
Compare
Thank you, will import the MR. |
One unit test was failing due to the API change, I fixed it internally. Could you provide a second MR with a unit test of this code change? |
@maximilianwulf |
@@ -47,6 +47,11 @@ class PclLoaderParameters { | |||
double resolution_ = 0.1; | |||
unsigned int minCloudPointsPerCell_ = 2; |
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.
useMaxHeightAsCellElevation_
in ClustExtractionParameters
can be deleted, right?
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.
Fixed in efc2f16
@@ -47,6 +47,11 @@ class PclLoaderParameters { | |||
double resolution_ = 0.1; | |||
unsigned int minCloudPointsPerCell_ = 2; | |||
unsigned int maxCloudPointsPerCell_ = 100000; | |||
// 0: Smallest value among the average values of each cluster | |||
// 1: Mean value of the cluster with the most 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.
What happend to type = 2?
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.
Oh i just copy ros2 version. Fixed in 1f1ab69
@maximilianwulf |
ping @maximilianwulf |
1 similar comment
ping @maximilianwulf |
friendly ping @maximilianwulf |
kindly ping @maximilianwulf |
I ported #323 to master. (ros2 -> ros1)
height_thresh
is a parameter to reduce the influence of point clouds that exist in high locations such as ceilings.0: Smallest value among the average values of each cluster (current method)
1: Largest value among the average values of each cluster (current method which is implemeted only for ros1)
2: Mean value of the cluster with the most (new method)