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

fix(pointcloud_preprocessor): organize input twist topic #5125

Conversation

kminoda
Copy link
Contributor

@kminoda kminoda commented Sep 26, 2023

Description

  • Concatenate filter node now supports both twist and odom type input for velocity and angular velocity input.
  • Concatenate filter node used in perception module now uses odometry input from localization.

Related to: #5108

Should be merged with

Tests performed

Tested with pilot-auto.x2 (TIER IV internal product version Autoware) and confirmed that the change addresses the incorrect timestamp difference compensation using Logging Simulator.

Effects on system behavior

  • The concatenate filter node in perception will now use twist input from localization, which should improve the concatenation accuracy.
  • All the concatenation node will now require an additional parameter

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.

  • There are no open discussions or they are tracked via tickets.

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

* fix(pointcloud_preprocessor): organize input twist topic

Signed-off-by: kminoda <[email protected]>

* style(pre-commit): autofix

* fix build bug

Signed-off-by: kminoda <[email protected]>

* fix format error

Signed-off-by: kminoda <[email protected]>

* style(pre-commit): autofix

* fix

Signed-off-by: kminoda <[email protected]>

---------

Signed-off-by: kminoda <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@github-actions github-actions bot added component:sensing Data acquisition from sensors, drivers, preprocessing. (auto-assigned) component:launch Launch files, scripts and initialization tools. (auto-assigned) labels Sep 26, 2023
@kminoda kminoda added the tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Sep 26, 2023
Signed-off-by: kminoda <[email protected]>
@kminoda kminoda marked this pull request as ready for review September 26, 2023 02:07
@github-actions github-actions bot added the type:documentation Creating or refining documentation. (auto-assigned) label Sep 26, 2023
@kminoda
Copy link
Contributor Author

kminoda commented Sep 26, 2023

@miursh @yukkysaito Would you review this PR? 🙏

@kminoda kminoda changed the title fix(pointcloud_preprocessor): organize input twist topic (#25) fix(pointcloud_preprocessor): organize input twist topic Sep 26, 2023
@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

Attention: 25 lines in your changes are missing coverage. Please review.

Comparison is base (443386d) 14.91% compared to head (e7b33eb) 14.88%.
Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5125      +/-   ##
==========================================
- Coverage   14.91%   14.88%   -0.04%     
==========================================
  Files        1625     1630       +5     
  Lines      112412   112656     +244     
  Branches    34699    34691       -8     
==========================================
  Hits        16764    16764              
- Misses      76910    77154     +244     
  Partials    18738    18738              
Flag Coverage Δ *Carryforward flag
differential 5.57% <0.00%> (?)
total 14.91% <ø> (+<0.01%) ⬆️ Carriedforward from 443386d

*This pull request uses carry forward flags. Click here to find out more.

Files Coverage Δ
...r_path_planner/scene_module/lane_change/normal.hpp 0.00% <ø> (ø)
...nner/utils/lane_change/lane_change_module_data.hpp 0.00% <ø> (ø)
...h_planner/src/scene_module/lane_change/manager.cpp 5.20% <ø> (ø)
...th_planner/src/scene_module/lane_change/normal.cpp 3.83% <ø> (+0.07%) ⬆️
...atenate_data/concatenate_and_time_sync_nodelet.hpp 0.00% <ø> (ø)
...atenate_data/concatenate_and_time_sync_nodelet.cpp 0.00% <0.00%> (ø)

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@miursh
Copy link
Contributor

miursh commented Sep 26, 2023

@kminoda May I request two additional items?

  1. Could you make the same change in time_synchronizer as well? This is because sync_and_concat node is going to be deprecated, and we are going to use sync node and concatenate node separately.
  2. Could you add warning here when the twist queue is empty?

Signed-off-by: kminoda <[email protected]>
@kminoda
Copy link
Contributor Author

kminoda commented Sep 26, 2023

@miursh
Thank you for the comment!

For the first point, fixed here: 44bf023
For the second point, let me handle that in another PR.

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

@kminoda kminoda merged commit 68ee0f2 into autowarefoundation:main Sep 26, 2023
19 of 23 checks passed
@kminoda kminoda deleted the fix/pointcloud_preprocessor/organize_twist_in_concatenate_filter branch September 26, 2023 11:19
kyoichi-sugahara pushed a commit to kyoichi-sugahara/autoware.universe that referenced this pull request Sep 26, 2023
…ndation#5125)

* fix(pointcloud_preprocessor): organize input twist topic (autowarefoundation#25)

* fix(pointcloud_preprocessor): organize input twist topic

Signed-off-by: kminoda <[email protected]>

* style(pre-commit): autofix

* fix build bug

Signed-off-by: kminoda <[email protected]>

* fix format error

Signed-off-by: kminoda <[email protected]>

* style(pre-commit): autofix

* fix

Signed-off-by: kminoda <[email protected]>

---------

Signed-off-by: kminoda <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* minor fixes

Signed-off-by: kminoda <[email protected]>

* style(pre-commit): autofix

* add warning

Signed-off-by: kminoda <[email protected]>

* style(pre-commit): autofix

---------

Signed-off-by: kminoda <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
kminoda added a commit to kminoda/autoware.universe that referenced this pull request Sep 28, 2023
…ndation#5125)

* fix(pointcloud_preprocessor): organize input twist topic (#25)

* fix(pointcloud_preprocessor): organize input twist topic

Signed-off-by: kminoda <[email protected]>

* style(pre-commit): autofix

* fix build bug

Signed-off-by: kminoda <[email protected]>

* fix format error

Signed-off-by: kminoda <[email protected]>

* style(pre-commit): autofix

* fix

Signed-off-by: kminoda <[email protected]>

---------

Signed-off-by: kminoda <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* minor fixes

Signed-off-by: kminoda <[email protected]>

* style(pre-commit): autofix

* add warning

Signed-off-by: kminoda <[email protected]>

* style(pre-commit): autofix

---------

Signed-off-by: kminoda <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
kminoda added a commit to tier4/autoware.universe that referenced this pull request Sep 29, 2023
…nize_twist_in_concatenate_filter

fix(pointcloud_preprocessor): organize input twist topic (autowarefoundation#5125)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:launch Launch files, scripts and initialization tools. (auto-assigned) 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) type:documentation Creating or refining documentation. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants