-
Notifications
You must be signed in to change notification settings - Fork 53
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(path_generator): add path_generator package #138
feat(path_generator): add path_generator package #138
Conversation
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
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.
Since it’s a porting task, I’m keeping the review to a minimum.
b3c36b5
to
e3b27e6
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.
Thank you for your revision. LGTM
@takayuki5168
can we use tier4 msgs in core? |
@kosuke55 |
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.
PathWithLaneId
has been ported. Please use it instead.
autowarefoundation/autoware_internal_msgs#42
<depend>generate_parameter_library</depend> | ||
<depend>rclcpp</depend> | ||
<depend>rclcpp_components</depend> | ||
<depend>tier4_planning_msgs</depend> |
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.
<depend>tier4_planning_msgs</depend> | |
<depend>autoware_internal_planning_msgs</depend> |
Please make the same changes to the other sections as well.
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.
The package uses autoware_trajectory internally, and it depends on tier4_planning_msgs::msg::PathPointWithLaneId
. Should I ask the maintainer to migrate to autoware_internal_planning_msgs
?
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.
@mitsudome-r It seems that the autoware_motion_utils
and autoware_trajectory
packages also need to be ported, but is any work being done on them somewhere?
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.
I found #171.
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.
|
||
#include "autoware/path_generator/common_structs.hpp" | ||
|
||
#include <tier4_planning_msgs/msg/path_with_lane_id.hpp> |
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.
#include <tier4_planning_msgs/msg/path_with_lane_id.hpp> | |
#include <autoware_internal_planning_msgs/msg/path_with_lane_id.hpp> |
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.
using tier4_planning_msgs::msg::PathPointWithLaneId; | ||
using tier4_planning_msgs::msg::PathWithLaneId; |
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.
using tier4_planning_msgs::msg::PathPointWithLaneId; | |
using tier4_planning_msgs::msg::PathWithLaneId; | |
using autoware_internal_planning_msgs::msg::PathPointWithLaneId; | |
using autoware_internal_planning_msgs::msg::PathWithLaneId; |
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.
<depend>generate_parameter_library</depend> | ||
<depend>rclcpp</depend> | ||
<depend>rclcpp_components</depend> | ||
<depend>tier4_planning_msgs</depend> |
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.
@mitsudome-r It seems that the autoware_motion_utils
and autoware_trajectory
packages also need to be ported, but is any work being done on them somewhere?
<depend>generate_parameter_library</depend> | ||
<depend>rclcpp</depend> | ||
<depend>rclcpp_components</depend> | ||
<depend>tier4_planning_msgs</depend> |
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.
I found #171.
@mitukou1109 The |
c44202b
to
427fb8c
Compare
It seems |
93f11f2
to
e5d8e89
Compare
Plus, it needs #170 to be merged due to its dependence on |
4d0f7b8
to
538e248
Compare
@mitukou1109 (CC: @mitsudome-r @kosuke55) Thank you for working on adding features! Sorry for making you wait for a while. Let me share the current situation. Currently we are working on resolving package dependencies. What we doing/found:
We'll continuously put the update here not to miss the ongoing updates. Again, thank you so much for your contribution! |
planning/autoware_path_generator/include/autoware/path_generator/utils.hpp
Outdated
Show resolved
Hide resolved
852bff1
to
3ea23c5
Compare
Signed-off-by: mitukou1109 <[email protected]>
Signed-off-by: mitukou1109 <[email protected]>
Signed-off-by: mitukou1109 <[email protected]>
Signed-off-by: mitukou1109 <[email protected]>
Signed-off-by: mitukou1109 <[email protected]>
Signed-off-by: mitukou1109 <[email protected]>
Signed-off-by: mitukou1109 <[email protected]>
Signed-off-by: mitukou1109 <[email protected]>
Signed-off-by: mitukou1109 <[email protected]>
2b8ba7f
to
e118a66
Compare
I rebased on origin/main to include vehicle_info_utils. (sorry to make reviews outdated |
Signed-off-by: kosuke55 <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #138 +/- ##
==========================================
- Coverage 78.75% 0.00% -78.76%
==========================================
Files 11 86 +75
Lines 193 8564 +8371
Branches 73 856 +783
==========================================
- Hits 152 0 -152
- Misses 11 8564 +8553
+ Partials 30 0 -30
☔ View full report in Codecov by Sentry. |
works well with
todo : need implement additional features in other PRs
|
* add tests Signed-off-by: mitukou1109 <[email protected]> * adapt test to new test manager Signed-off-by: mitukou1109 <[email protected]> * migrate to autoware_internal_planning_msgs Signed-off-by: mitukou1109 <[email protected]> * use intersection map for unit tests Signed-off-by: mitukou1109 <[email protected]> --------- Signed-off-by: mitukou1109 <[email protected]>
reverted because autoware_test_utils has not been ported |
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! thanks a lot!
Signed-off-by: kosuke55 <[email protected]>
Signed-off-by: kosuke55 <[email protected]>
unit test require to port autoware_test_utils to core. so I reverted unit test commits
unit test branch is here, will merge after porting ↑ |
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.
CI passed LGTM again!
@youtalk cloud you check again? |
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 finally! Great work.
evaluator check is also ok, I will merge cc @mitsudome-r |
Description
This PR adds
path_generator
package, a simplified alternative ofbehavior_path_planner
.Related links
How was this PR tested?
Psim on a lanelet2 map with waypoints
Screencast.from.2024.11.29.16.11.24.webm
Notes for reviewers
None.
Interface changes
None.
Effects on system behavior
None.