Skip to content
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): enable to count empty pointclouds topic in concatenate pointclouds nodelet #6277

Conversation

YoshiRi
Copy link
Contributor

@YoshiRi YoshiRi commented Feb 1, 2024

Description

Problem

  • concatenate process exit with completing topics subscription or timeout
  • topic with empty pointclouds are returned and not counted as subscribed, so that the concatenate node will wait until timeout. This will cause a slight delay.
    • Our XX1 vehicle faces lower topic rate with this problem

PR solutions

  • enable concatenate node to count empty pointclouds as well
  • Need some escape for the latter concatenation process

Related links

TIER IV INTERNAL SLACK DISCUSSION

Tests performed

Tested with jpntaxi settings and rosbag.

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.

  • The PR follows the pull request guidelines.
  • The PR has been properly tested.
  • The PR has been reviewed by the code owners.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

  • There are no open discussions or they are tracked via tickets.
  • The PR is ready for merge.

After all checkboxes are checked, anyone who has write access can merge the PR.

Signed-off-by: yoshiri <yoshiyoshidetteiu@gmail.com>
@github-actions github-actions bot added the component:sensing Data acquisition from sensors, drivers, preprocessing. (auto-assigned) label Feb 1, 2024
@iwatake2222
Copy link
Contributor

iwatake2222 commented Feb 1, 2024

@YoshiRi
I confirmed the delay was improved when one of input topics of concatenate node is empty.
The following logs show the frequency of output topic ( /sensing/lidar/concatenated/pointcloud )

  • Original code
$ ros2 topic hz /sensing/lidar/concatenated/pointcloud -w 10
average rate: 7.855
	min: 0.096s max: 0.179s std dev: 0.02487s window: 9
average rate: 8.227
	min: 0.096s max: 0.163s std dev: 0.02371s window: 10
average rate: 8.633
	min: 0.088s max: 0.165s std dev: 0.02208s window: 10
  • With this PR
$ ros2 topic hz /sensing/lidar/concatenated/pointcloud -w 10
average rate: 9.987
	min: 0.083s max: 0.114s std dev: 0.00757s window: 10
average rate: 10.039
	min: 0.049s max: 0.151s std dev: 0.02364s window: 10
average rate: 9.926
	min: 0.091s max: 0.112s std dev: 0.00576s window: 10

Signed-off-by: yoshiri <yoshiyoshidetteiu@gmail.com>
@YoshiRi YoshiRi marked this pull request as ready for review February 1, 2024 14:16
@miursh miursh self-assigned this Feb 6, 2024
@miursh miursh added the tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Feb 6, 2024
Copy link
Contributor

@miursh miursh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
confirmed with topic hz
before:

average rate: 8.285
	min: 0.100s max: 0.164s std dev: 0.02018s window: 10
average rate: 8.328
	min: 0.099s max: 0.157s std dev: 0.01780s window: 10
average rate: 7.838
	min: 0.100s max: 0.179s std dev: 0.02874s window: 10

after

average rate: 10.014
	min: 0.082s max: 0.114s std dev: 0.00906s window: 10
average rate: 10.078
	min: 0.090s max: 0.110s std dev: 0.00518s window: 10
average rate: 9.975
	min: 0.094s max: 0.107s std dev: 0.00405s window: 10

@miursh miursh enabled auto-merge (squash) February 6, 2024 17:53
@miursh miursh merged commit 7cb77dd into autowarefoundation:main Feb 6, 2024
42 of 44 checks passed
@YoshiRi YoshiRi deleted the fix/concatenate_sync_nodelet_to_accept_zero_pointclouds branch February 7, 2024 05:39
shmpwk pushed a commit to tier4/autoware.universe that referenced this pull request Feb 7, 2024
…c in concatenate pointclouds nodelet (autowarefoundation#6277)

* feat: enable to count empty pointclouds topic

Signed-off-by: yoshiri <yoshiyoshidetteiu@gmail.com>

* fix: time sync function to adapt empty pc topic

Signed-off-by: yoshiri <yoshiyoshidetteiu@gmail.com>

---------

Signed-off-by: yoshiri <yoshiyoshidetteiu@gmail.com>
anhnv3991 pushed a commit to anhnv3991/autoware.universe that referenced this pull request Feb 13, 2024
…c in concatenate pointclouds nodelet (autowarefoundation#6277)

* feat: enable to count empty pointclouds topic

Signed-off-by: yoshiri <yoshiyoshidetteiu@gmail.com>

* fix: time sync function to adapt empty pc topic

Signed-off-by: yoshiri <yoshiyoshidetteiu@gmail.com>

---------

Signed-off-by: yoshiri <yoshiyoshidetteiu@gmail.com>
StepTurtle pushed a commit to StepTurtle/autoware.universe that referenced this pull request Feb 28, 2024
…c in concatenate pointclouds nodelet (autowarefoundation#6277)

* feat: enable to count empty pointclouds topic

Signed-off-by: yoshiri <yoshiyoshidetteiu@gmail.com>

* fix: time sync function to adapt empty pc topic

Signed-off-by: yoshiri <yoshiyoshidetteiu@gmail.com>

---------

Signed-off-by: yoshiri <yoshiyoshidetteiu@gmail.com>
karishma1911 pushed a commit to Interplai/autoware.universe that referenced this pull request Jun 3, 2024
…c in concatenate pointclouds nodelet (autowarefoundation#6277)

* feat: enable to count empty pointclouds topic

Signed-off-by: yoshiri <yoshiyoshidetteiu@gmail.com>

* fix: time sync function to adapt empty pc topic

Signed-off-by: yoshiri <yoshiyoshidetteiu@gmail.com>

---------

Signed-off-by: yoshiri <yoshiyoshidetteiu@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:sensing Data acquisition from sensors, drivers, preprocessing. (auto-assigned) tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants