-
Notifications
You must be signed in to change notification settings - Fork 669
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
feat(autoware_pointcloud_preprocessor): redesign concatenate and time sync node #8300
base: main
Are you sure you want to change the base?
feat(autoware_pointcloud_preprocessor): redesign concatenate and time sync node #8300
Conversation
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8300 +/- ##
==========================================
+ Coverage 29.69% 30.10% +0.40%
==========================================
Files 1452 1454 +2
Lines 108962 107887 -1075
Branches 42584 42178 -406
==========================================
+ Hits 32353 32476 +123
+ Misses 73496 72203 -1293
- Partials 3113 3208 +95
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@kminoda |
@vividf Thanks. Let me do that later |
sensing/autoware_pointcloud_preprocessor/schema/cocatenate_and_time_sync_node.schema.json
Outdated
Show resolved
Hide resolved
sensing/autoware_pointcloud_preprocessor/schema/cocatenate_and_time_sync_node.schema.json
Outdated
Show resolved
Hide resolved
sensing/autoware_pointcloud_preprocessor/schema/cocatenate_and_time_sync_node.schema.json
Outdated
Show resolved
Hide resolved
sensing/autoware_pointcloud_preprocessor/docs/concatenate-data.md
Outdated
Show resolved
Hide resolved
...include/autoware/pointcloud_preprocessor/concatenate_data/concatenate_and_time_sync_node.hpp
Outdated
Show resolved
Hide resolved
...include/autoware/pointcloud_preprocessor/concatenate_data/concatenate_and_time_sync_node.hpp
Outdated
Show resolved
Hide resolved
...include/autoware/pointcloud_preprocessor/concatenate_data/concatenate_and_time_sync_node.hpp
Outdated
Show resolved
Hide resolved
...include/autoware/pointcloud_preprocessor/concatenate_data/concatenate_and_time_sync_node.hpp
Outdated
Show resolved
Hide resolved
sensing/autoware_pointcloud_preprocessor/src/concatenate_data/combine_cloud_handler.cpp
Outdated
Show resolved
Hide resolved
...ing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp
Outdated
Show resolved
Hide resolved
sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp
Outdated
Show resolved
Hide resolved
sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp
Outdated
Show resolved
Hide resolved
sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp
Outdated
Show resolved
Hide resolved
sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp
Outdated
Show resolved
Hide resolved
sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp
Show resolved
Hide resolved
sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp
Outdated
Show resolved
Hide resolved
sensing/autoware_pointcloud_preprocessor/src/concatenate_data/combine_cloud_handler.cpp
Outdated
Show resolved
Hide resolved
sensing/autoware_pointcloud_preprocessor/src/concatenate_data/combine_cloud_handler.cpp
Outdated
Show resolved
Hide resolved
sensing/autoware_pointcloud_preprocessor/src/concatenate_data/combine_cloud_handler.cpp
Outdated
Show resolved
Hide resolved
sensing/autoware_pointcloud_preprocessor/src/concatenate_data/combine_cloud_handler.cpp
Outdated
Show resolved
Hide resolved
sensing/autoware_pointcloud_preprocessor/src/concatenate_data/combine_cloud_handler.cpp
Outdated
Show resolved
Hide resolved
…hub.com:vividf/autoware.universe into feature/redesign_concatenate_and_time_sync_node
Signed-off-by: vividf <[email protected]>
sensing/autoware_pointcloud_preprocessor/src/concatenate_data/combine_cloud_handler.cpp
Outdated
Show resolved
Hide resolved
sensing/autoware_pointcloud_preprocessor/docs/concatenate-data.md
Outdated
Show resolved
Hide resolved
sensing/autoware_pointcloud_preprocessor/docs/concatenate-data.md
Outdated
Show resolved
Hide resolved
...d_preprocessor/include/autoware/pointcloud_preprocessor/concatenate_data/cloud_collector.hpp
Outdated
Show resolved
Hide resolved
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 for the changes!
There's still a bit of unused state in the cloud_collector, but refactoring so it references the chosen strategy should be quite quick.
Also, I left some small fixes for documentation rendering. see docs here or run locally via:
cd src/universe/autoware.universe
mkdocs serve
For running locally, you might also need to do this setup first.
Co-authored-by: Max Schmeller <[email protected]>
Co-authored-by: Max Schmeller <[email protected]>
Signed-off-by: vividf <[email protected]>
Signed-off-by: vividf <[email protected]>
Signed-off-by: vividf <[email protected]>
...or/include/autoware/pointcloud_preprocessor/concatenate_data/collector_matching_strategy.hpp
Outdated
Show resolved
Hide resolved
...d_preprocessor/include/autoware/pointcloud_preprocessor/concatenate_data/cloud_collector.hpp
Outdated
Show resolved
Hide resolved
...ing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp
Outdated
Show resolved
Hide resolved
...ing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp
Outdated
Show resolved
Hide resolved
Signed-off-by: vividf <[email protected]>
Signed-off-by: vividf <[email protected]>
Signed-off-by: vividf <[email protected]>
sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp
Outdated
Show resolved
Hide resolved
Signed-off-by: vividf <[email protected]>
Signed-off-by: vividf <[email protected]>
Signed-off-by: vividf <[email protected]>
Signed-off-by: vividf <[email protected]>
Description
This PR solved the issue #6832.
Previous designs have some issues concatenating the pointcloud correctly, therefore, this PR redesigns the logic of the concatenate node in order to handle the edge cases like pointcloud delay or pointcloud drop.
Changes
A more detailed description of the algorithm is on the Readme page.https://github.com/vividf/autoware.universe/blob/feature/redesign_concatenate_and_time_sync_node/sensing/autoware_pointcloud_preprocessor/docs/concatenate-data.md
Related links
Parent Issue:
How was this PR tested?
Unit test and Component test for concatenate node
Tested with sample rosbag
Please change the sample_sensor_kit_launch: autowarefoundation/sample_sensor_kit_launch#108
The result provided by @mojomex below #8300 (comment)
Tested with vehicle 1
data: TIER4_INTERNAL_LINK
modify the config file
concatenate_and_time_sync_node.param.yaml
Note
Tested with vehicle 2
data: TIER4_INTERNAL_LINK
modify the config file
concatenate_and_time_sync_node.param.yaml
Result
TIER4_INTERNAL_LINK
Time comparison (vehicle 1 bag)
From last arrived pointcloud to publish concatenate pointcloud (include publishing)
Before (move the toc to the beginning of the cloudcallback function)
Note that the huge latency is because a pointcloud is dropped.
After
By setting is_motion_compensated to false
Notes for reviewers
locking logic (mutex) might be an important part of double-checking :)
Interface changes
None.
Effects on system behavior
None.