-
Notifications
You must be signed in to change notification settings - Fork 658
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
refactor(autoware_pointcloud_preprocessor): rework concatenate_pointcloud and time_synchronizer_node parameters #8509
Conversation
Signed-off-by: vividf <[email protected]>
Signed-off-by: vividf <[email protected]>
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
Signed-off-by: vividf <[email protected]>
Signed-off-by: vividf <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8509 +/- ##
==========================================
- Coverage 23.96% 23.81% -0.15%
==========================================
Files 1382 1388 +6
Lines 102019 101936 -83
Branches 38886 38860 -26
==========================================
- Hits 24446 24281 -165
- Misses 75141 75178 +37
- Partials 2432 2477 +45
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Apart of comments, please consider adding boundaries for all integer / numeric parameters.
"properties": { | ||
"timeout_sec": { | ||
"type": "number", | ||
"default": 0.1, |
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.
Assign default values as string
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.
Thanks! fixed them in 08ec218
}, | ||
"max_queue_size": { | ||
"type": "integer", | ||
"default": 5, |
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.
Assign default values as string
"properties": { | ||
"timeout_sec": { | ||
"type": "number", | ||
"default": 0.033, |
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.
Assign default values as string
}, | ||
"max_queue_size": { | ||
"type": "integer", | ||
"default": 5, |
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.
Assign default values as string
Signed-off-by: vividf <[email protected]>
…time_synchronizer_parameter
Signed-off-by: vividf <[email protected]>
…onizer_parameter' of github.com:vividf/autoware.universe into refactor/rework_concatenate_pointclouds_and_time_synchronizer_parameter
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
…time_synchronizer_parameter
…loud and time_synchronizer_node parameters (autowarefoundation#8509) * feat: rewort concatenate pointclouds and time synchronizer parameter Signed-off-by: vividf <[email protected]> * chore: fix launch files Signed-off-by: vividf <[email protected]> * chore: fix schema Signed-off-by: vividf <[email protected]> * chore: fix schema Signed-off-by: vividf <[email protected]> * chore: fix integer and number default value in schema Signed-off-by: vividf <[email protected]> * chore: add boundary Signed-off-by: vividf <[email protected]> --------- Signed-off-by: vividf <[email protected]>
…loud and time_synchronizer_node parameters (autowarefoundation#8509) * feat: rewort concatenate pointclouds and time synchronizer parameter Signed-off-by: vividf <[email protected]> * chore: fix launch files Signed-off-by: vividf <[email protected]> * chore: fix schema Signed-off-by: vividf <[email protected]> * chore: fix schema Signed-off-by: vividf <[email protected]> * chore: fix integer and number default value in schema Signed-off-by: vividf <[email protected]> * chore: add boundary Signed-off-by: vividf <[email protected]> --------- Signed-off-by: vividf <[email protected]>
Description
This PR includes the following changes
nodelet
tonode
.Note that I didn't update the readme since after this PR #8300 the readme file will change a lot (parameters are different).
I will update the readme in this PR #8300
Related links
Parent Issue:
How was this PR tested?
ros2 launch autoware_pointcloud_preprocessor concatenate_pointcloud.launch.xml
and
ros2 launch autoware_pointcloud_preprocessor time_synchronizer_node.launch.xml
Notes for reviewers
None.
Interface changes
None.
Effects on system behavior
None.