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: port autoware_route_handler from Autoware Universe #201

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

mitsudome-r
Copy link
Member

@mitsudome-r mitsudome-r commented Feb 18, 2025

Copy link

github-actions bot commented Feb 18, 2025

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

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

This is a porting PR so that I only conducted a minimal review.

Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

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

LGTM

@youtalk youtalk added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Feb 21, 2025
@xmfcx
Copy link
Contributor

xmfcx commented Feb 26, 2025

Here is the failed cppcheck report:

/home/runner/work/autoware.core/autoware.core/planning/autoware_route_handler/src/route_handler.cpp:64:3: style: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm]
  for (const auto & p : primitives) {
  ^
/home/runner/work/autoware.core/autoware.core/planning/autoware_route_handler/src/route_handler.cpp:74:3: style: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm]
  for (const auto & i : vectors) {
  ^
/home/runner/work/autoware.core/autoware.core/planning/autoware_route_handler/src/route_handler.cpp:149:26: style: Consider using std::transform algorithm instead of a raw loop. [useStlAlgorithm]
    piecewise_ref_points.push_back(ReferencePoint{true, piecewise_waypoint});
                         ^
/home/runner/work/autoware.core/autoware.core/planning/autoware_route_handler/src/route_handler.cpp:519:3: style: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm]
  for (const auto & llt : route_lanelets_) {
  ^
/home/runner/work/autoware.core/autoware.core/planning/autoware_route_handler/src/route_handler.cpp:1017:0: style: Consider using std::find_if algorithm instead of a raw loop. [useStlAlgorithm]
      if (isRoadLanelet(right_lanelet)) return right_lanelet;
^
/home/runner/work/autoware.core/autoware.core/planning/autoware_route_handler/src/route_handler.cpp:1083:0: style: Consider using std::find_if algorithm instead of a raw loop. [useStlAlgorithm]
      if (isRoadLanelet(left_lanelet)) return left_lanelet;
^
/home/runner/work/autoware.core/autoware.core/planning/autoware_route_handler/src/route_handler.cpp:2073:3: style: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm]
  for (const auto & llt : path)
  ^
nofile:0:0: information: Active checkers: 172/592 (use --checkers-report=<filename> to see details) [checkersReport]

I will try to fix them or add exceptions.

@xmfcx
Copy link
Contributor

xmfcx commented Feb 26, 2025

Will create a PR to add useStlAlgorithm and checkersReport suppressions.

For the porting process, let's keep the changes to minimum. And then gradually remove the restrictions once basic move operations are finished.

https://github.com/autowarefoundation/autoware.universe/blob/769673c95d12a6361c6e3e89c5bd10ab5e1cadba/.cppcheck_suppressions#L5-L11

Created the PR:

mitsudome-r and others added 6 commits February 26, 2025 14:34
Signed-off-by: mitsudome-r <ryohsuke.mitsudome@tier4.jp>
Signed-off-by: mitsudome-r <ryohsuke.mitsudome@tier4.jp>
Signed-off-by: mitsudome-r <ryohsuke.mitsudome@tier4.jp>
Signed-off-by: Ryohsuke Mitsudome <ryohsuke.mitsudome@tier4.jp>
Signed-off-by: Ryohsuke Mitsudome <ryohsuke.mitsudome@tier4.jp>
@xmfcx xmfcx force-pushed the port-autoware-route-handler branch from a45151b to 0b42f5e Compare February 26, 2025 05:34
@xmfcx
Copy link
Contributor

xmfcx commented Feb 26, 2025

@xmfcx
Copy link
Contributor

xmfcx commented Feb 26, 2025

Also the CI doesn't pass: https://github.com/autowarefoundation/autoware.core/actions/runs/13537260433/job/37831081771?pr=201#step:5:6907

In file included from /__w/autoware.core/autoware.core/planning/autoware_route_handler/test/test_route_handler.cpp:15:
/__w/autoware.core/autoware.core/planning/autoware_route_handler/test/test_route_handler.hpp:23:10: fatal error: autoware_test_utils/autoware_test_utils.hpp: No such file or directory
   23 | #include <autoware_test_utils/autoware_test_utils.hpp>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@xmfcx xmfcx added the type:moving Temporary label to represent packages that are specifically moving from Universe to Core. label Feb 26, 2025
@xmfcx xmfcx marked this pull request as draft February 28, 2025 07:43
Co-authored-by: Mete Fatih Cırıt <xmfcx@users.noreply.github.com>
Copy link

codecov bot commented Mar 5, 2025

Codecov Report

Attention: Patch coverage is 34.41503% with 768 lines in your changes missing coverage. Please review.

Project coverage is 22.32%. Comparing base (4cb18f5) to head (6499d2f).
Report is 61 commits behind head on main.

Files with missing lines Patch % Lines
...nning/autoware_route_handler/src/route_handler.cpp 30.16% 591 Missing and 78 partials ⚠️
...autoware_route_handler/test/test_route_handler.cpp 52.84% 10 Missing and 73 partials ⚠️
...r/include/autoware/route_handler/route_handler.hpp 0.00% 10 Missing ⚠️
...autoware_route_handler/test/test_route_handler.hpp 77.77% 5 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #201       +/-   ##
===========================================
- Coverage   78.75%   22.32%   -56.44%     
===========================================
  Files          11       23       +12     
  Lines         193     2101     +1908     
  Branches       73      735      +662     
===========================================
+ Hits          152      469      +317     
- Misses         11     1473     +1462     
- Partials       30      159      +129     
Flag Coverage Δ
differential 22.32% <34.41%> (?)
total ?

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) type:moving Temporary label to represent packages that are specifically moving from Universe to Core.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants