-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feature/add frequency tolerance parameter #1
base: master
Are you sure you want to change the base?
Conversation
velodyne_driver/cfg/VelodyneNode.cfg
Outdated
@@ -11,5 +11,6 @@ gen = ParameterGenerator() | |||
gen.add("time_offset", double_t, 1, "A manually calibrated offset (in seconds) to add to the timestamp before publication of a message.", | |||
0.0, -1.0, 1.0) | |||
gen.add("enabled", bool_t, 2, "Switch to enable and disable lidar packet consumption", True); |
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.
Maybe make the window_size also a parameter then?
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.
I think we get enough control with the tolerance parameter and a fixed window size of 10, my preference would go for less parameters
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.
Is there any reason we go outside this boundary?
PR only for internal review, after approval I will create a PR to the base repo.