-
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
feat(pointcloud_preprocessor): add pipeline latency time debug information for pointcloud pipeline #6056
Conversation
c792b08
to
bcb2de2
Compare
sensing/pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_nodelet.cpp
Outdated
Show resolved
Hide resolved
d6549e8
to
4873f34
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #6056 +/- ##
==========================================
+ Coverage 14.39% 14.46% +0.06%
==========================================
Files 1906 1906
Lines 129860 130105 +245
Branches 37582 37766 +184
==========================================
+ Hits 18699 18817 +118
- Misses 90167 90234 +67
- Partials 20994 21054 +60
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. |
FYI: I've created a related PR for perception module #6181 |
e2ceac9
to
30c4b90
Compare
…ation for pointcloud pipeline Signed-off-by: Berkay Karaman <[email protected]> update
30c4b90
to
cbf3c5e
Compare
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
…ation for pointcloud pipeline (autowarefoundation#6056)
…ation for pointcloud pipeline (autowarefoundation#6056)
…ation for pointcloud pipeline (autowarefoundation#6056)
…ation for pointcloud pipeline (autowarefoundation#6056)
Description
#6032
As mentioned in the issue, we are mostly using the
ros2 topic delay /topic_name
which shows us the time between the message header and the current time. This method is suitable for small-size messages, however, it adds extra delay because accessing the point cloud messages from outside decreases the performance of the pipeline because of the large size of the point cloud message.Comparision of
ros2 topic delay
andaccumulated_time
Left to right: Cropbox Filter Self -> Cropbox Filter Mirror -> Distortion Corrector -> Ring Outlier Filter
As you can see, the
ros2 topic delay
is adding some extra-delay to actual delay in every step. Also, although accumulated time is increasing in every step (as we expected),ros2 topic delay
gives us not accurate and weird output.The primary reason is that our sensing/perception nodes run within composable node containers and utilize intra-process communication. External subscriptions to these messages, like using ros2 topic delay or rviz2, add additional delays. Even slow down existing pipelines just by subscribing from outside.
Because of these reasons, it is better to publish the delay time which is reported by the node to not damage the intra-process communication of the point cloud pipeline.
To achieve this, we added
accumulation_time
information to each stage of the pipeline to measure how much time takes in the processes for each lidar.The user can see the accumulated_time measurement for the lidar sensor output which is named "LidarX" by subscribing following topics:
Related links
Tests performed
Notes for reviewers
Interface changes
Effects on system behavior
Pre-review checklist for the PR author
The PR author must check the checkboxes below when creating the PR.
In-review checklist for the PR reviewers
The PR reviewers must check the checkboxes below before approval.
Post-review checklist for the PR author
The PR author must check the checkboxes below before merging.
After all checkboxes are checked, anyone who has write access can merge the PR.