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(autoware_path_generator): function to smooth the route #227

Open
wants to merge 33 commits into
base: main
Choose a base branch
from

Conversation

sasakisasaki
Copy link
Contributor

@sasakisasaki sasakisasaki commented Feb 26, 2025

Description

This is kind of feature porting from autoware.universe as follows

  • Import PathWithLaneId DefaultFixedGoalPlanner::modifyPathForSmoothGoalConnection from the autoware.universe side code
  • Also import all related functions from the autoware.universe side

Quality of Code

  • NOT ENOUGH YET. I'm improving the code quality so this PR is ready for review.
  • The unit tests for the added feature has not been added yet.

The refactoring plan is mentioned in this issue.
Perhaps we can work on the refactoring with a few PRs. Including this idea for refactoring, please feel free to share your idea for the improvement of code quality. Thank you!

How was this PR tested?

  • I did vcs import at around 18:00 JST on 26th Feb. 2025
  • The executed commands are as follows:
$ git clone https://github.com/autowarefoundation/autoware.git
$ cd autoware

# Before the build
$ ./setup-dev-env.sh -y

(Enter password for user)

(ADDED: Perhaps you need to do `sudo shutdown -r now` for applying NVIDIA driver update)

$ vcs import src < autoware.repos

$ vim autoware-nightly.repos

(Edited `autoware.core` part so it used my branch on forked)

$ vcs import src < autoware-nightly.repos
$ source ~/autoware/install/setup.bash
$ sudo apt update && sudo apt upgrade
$ rosdep update
$ rosdep install -y --from-paths src --ignore-src --rosdistro $ROS_DISTRO

# Deploy the attached three files for the following place, respectively
# - src/launcher/autoware_launch/autoware_launch/config/planning/scenario_planning/lane_driving/behavior_planning/path_generator.param.yaml
# - src/launcher/autoware_launch/autoware_launch/launch/components/tier4_planning_component.launch.xml
# - src/universe/autoware.universe/launch/tier4_planning_launch/launch/scenario_planning/lane_driving/behavior_planning/behavior_planning.launch.xml

behavior_planning_launch_xml.txt
tier4_planning_component_launch_xml.txt
path_generator_param_yaml.txt

# Now start colcon build
$ colcon build --symlink-install --cmake-args -DCMAKE_BUILD_TYPE=Release

# Checked by psim
$ ros2 launch autoware_launch planning_simulator.launch.xml map_path:=$HOME/autoware_map/sample-map-planning vehicle_model:=sample_vehicle sensor_model:=sample_sensor_kit
Screencast_psim.webm

Status of Additional Tests

(ADDED) It seems the tests on the evaluator shows a failed scenario link to failed one. I'm now investigating the issue.

Notes for reviewers

Please do not enable auto merge as this PR needed to be merged with this launch side PR at the same time.
Please feel free to provide all the needed tests for merging this PR. I'm really happy for performing the tests 👍

Effects on system behavior

Needed to be investigated during the testing process.

  Description:
    This commit is kind of feature porting from `autoware.universe` as follows
      * Import `PathWithLaneId DefaultFixedGoalPlanner::modifyPathForSmoothGoalConnection` from the following `autoware.universe` code
          https://github.com/autowarefoundation/autoware.universe/blob/a0816b7e3e35fbe822fefbb9c9a8132365608b49/planning/behavior_path_planner/autoware_behavior_path_goal_planner_module/src/default_fixed_goal_planner.cpp#L74-L104
      * Also import all related functions from the `autoware.universe` side

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
Copy link

github-actions bot commented Feb 26, 2025

Thank you for contributing to the Autoware project!

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

Please ensure:

@sasakisasaki sasakisasaki added run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) component:planning Route planning, decision-making, and navigation. (auto-assigned) labels Feb 26, 2025
@sasakisasaki sasakisasaki self-assigned this Feb 26, 2025
@xmfcx xmfcx added the type:new-feature New functionalities or additions, feature requests. label Feb 28, 2025
Copy link

codecov bot commented Feb 28, 2025

Codecov Report

Attention: Patch coverage is 0% with 125 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (4cb18f5) to head (04596a1).
Report is 58 commits behind head on main.

Files with missing lines Patch % Lines
planning/autoware_path_generator/src/utils.cpp 0.00% 120 Missing ⚠️
planning/autoware_path_generator/src/node.cpp 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #227       +/-   ##
==========================================
- Coverage   78.75%   0.00%   -78.76%     
==========================================
  Files          11      87       +76     
  Lines         193    8714     +8521     
  Branches       73     889      +816     
==========================================
- Hits          152       0      -152     
- Misses         11    8714     +8703     
+ Partials       30       0       -30     
Flag Coverage Δ
differential 0.00% <0.00%> (?)
total ?

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

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
Comment on lines 53 to 54
const double seg_dist =
autoware::motion_utils::calcSignedArcLength(input.points, seg_idx, seg_idx + 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

this is just calc_distance2d ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for this informative comment. I also guessed this is just a calculation for 2-dimensional distance between two points. If this is not my misunderstanding, the calcSignedArcLength() function calculates the total path length by summing up the 2D distances between consecutive points, as shown in the following code:

Ref. to code:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps we need to ensure which method should we use.

sasakisasaki and others added 7 commits February 28, 2025 18:11
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>

Co-authored-by: Kosuke Takeuchi <kosuke.tnp@gmail.com>
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>

Co-authored-by: Kosuke Takeuchi <kosuke.tnp@gmail.com>
  * Enhance error handlings
  * Remove unused variables
  * Simplify the code
  * Improve readability a little bit

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
…sasakisasaki/autoware.core into feat-embed-smooth-path-as-alpha-quality

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
sasakisasaki added a commit to sasakisasaki/autoware_launch that referenced this pull request Mar 4, 2025
  * This fix is for the following PR:
      autowarefoundation/autoware.core#227

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
  * This comment is wrote because of my misunderstanding

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
@sasakisasaki
Copy link
Contributor Author

Fixing error in some CI checks.

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
sasakisasaki and others added 6 commits March 5, 2025 15:01
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
…sasakisasaki/autoware.core into feat-embed-smooth-path-as-alpha-quality

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
@sasakisasaki sasakisasaki marked this pull request as ready for review March 5, 2025 06:20
@sasakisasaki sasakisasaki requested a review from kosuke55 March 5, 2025 06:27
@sasakisasaki
Copy link
Contributor Author

Oh, it seems the test is failing. Let me investigate and fix it. Sorry.

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.

Please add unit tests starting with only the simple cases.

@sasakisasaki
Copy link
Contributor Author

@youtalk Thank you so much for your proposal! Yes, that must be a practical approach for the improvement of the code quality! I'll add some small, but would be effective tests.

@kosuke55 In this PR's domain, I could detect the following hidden issue (expected package does not exist) thanks to the already existing tests. That's why I'm welcome adding the tests in this PR. Do you have any proposal what kind of tests would be effective? 👀

Breakpoint 1 (PlanningModuleInterfaceTest_NodeTestWithExceptionTrajectory) pending.
(gdb) run
Starting program: /***/build/autoware_path_generator/test_autoware_path_generator --gtest_filter=PlanningModuleInterfaceTest.NodeTestWithExceptionTrajectory
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Running main() from /opt/ros/humble/src/gtest_vendor/src/gtest_main.cc
Note: Google Test filter = PlanningModuleInterfaceTest.NodeTestWithExceptionTrajectory
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from PlanningModuleInterfaceTest
[ RUN      ] PlanningModuleInterfaceTest.NodeTestWithExceptionTrajectory
[New Thread 0x7ffff51ff640 (LWP 88717)]
[New Thread 0x7ffff49fe640 (LWP 88718)]
[New Thread 0x7ffff41fd640 (LWP 88719)]
[New Thread 0x7ffff39fc640 (LWP 88720)]
[New Thread 0x7ffff31fb640 (LWP 88721)]
[New Thread 0x7ffff28f9640 (LWP 88722)]
[New Thread 0x7ffff1ff7640 (LWP 88723)]
[New Thread 0x7ffff16f5640 (LWP 88724)]
[New Thread 0x7ffff0ef4640 (LWP 88725)]
[Thread 0x7ffff0ef4640 (LWP 88725) exited]
[Thread 0x7ffff1ff7640 (LWP 88723) exited]
[Thread 0x7ffff28f9640 (LWP 88722) exited]
[Thread 0x7ffff16f5640 (LWP 88724) exited]
[Thread 0x7ffff31fb640 (LWP 88721) exited]
[Thread 0x7ffff49fe640 (LWP 88718) exited]
[Thread 0x7ffff41fd640 (LWP 88719) exited]
[Thread 0x7ffff39fc640 (LWP 88720) exited]
unknown file: Failure
C++ exception with description "package 'autoware_test_utils' not found, searching: [/opt/ros/humble]" thrown in the test body.
[  FAILED  ] PlanningModuleInterfaceTest.NodeTestWithExceptionTrajectory (594 ms)
[----------] 1 test from PlanningModuleInterfaceTest (594 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (595 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] PlanningModuleInterfaceTest.NodeTestWithExceptionTrajectory

 1 FAILED TEST
[Thread 0x7ffff6211300 (LWP 88696) exited]
[Thread 0x7ffff51ff640 (LWP 88717) exited]
[New process 88696]
[Inferior 1 (process 88696) exited with code 01]
(gdb) 

sasakisasaki and others added 9 commits March 5, 2025 17:31
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
  * Sorry, I was forgetting to do this on my local env.

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
…sasakisasaki/autoware.core into feat-embed-smooth-path-as-alpha-quality

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:planning Route planning, decision-making, and navigation. (auto-assigned) run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) type:new-feature New functionalities or additions, feature requests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants