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

refactor(goal_planner): refactor PullOverPlannseBase to instantiate only valid path #8983

Merged

Conversation

soblin
Copy link
Contributor

@soblin soblin commented Sep 30, 2024

Description

  • move all member of PullOverPlannerBase to private
  • access to them by parking_path(), not getParkingPath()
  • delete the constructor of PullOverPath, and use PullOverPath::create to create not NONE PullOverPath
  • divide PullOverPlannerBase to source file

Related links

Parent Issue:

How was this PR tested?

https://evaluation.tier4.jp/evaluation/reports/babee270-4645-50a2-bbe0-a971956166f7?project_id=prd_jt

Notes for reviewers

None.

Interface changes

None.

Effects on system behavior

None.

@github-actions github-actions bot added the component:planning Route planning, decision-making, and navigation. (auto-assigned) label Sep 30, 2024
Copy link

github-actions bot commented Sep 30, 2024

Thank you for contributing to the Autoware project!

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

Please ensure:

@soblin soblin changed the title feat(goal_planner): refactor PullOverPlannseBase to instantiate only valid path refactor(goal_planner): refactor PullOverPlannseBase to instantiate only valid path Sep 30, 2024
Signed-off-by: Mamoru Sobue <[email protected]>
Signed-off-by: Mamoru Sobue <[email protected]>
Signed-off-by: Mamoru Sobue <[email protected]>
Signed-off-by: Mamoru Sobue <[email protected]>
@soblin soblin force-pushed the feat/goal-planner/path-validity branch from acd3045 to 3f75765 Compare September 30, 2024 05:53
Signed-off-by: Mamoru Sobue <[email protected]>
partial_paths, path_shifter.getShiftLines().front().start,
path_shifter.getShiftLines().front().end);
pull_over_path.pairs_terminal_velocity_and_accel.push_back(std::make_pair(pull_over_velocity, 0));
auto pull_over_path_opt = PullOverPath::create(
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
auto pull_over_path_opt = PullOverPath::create(
const auto pull_over_path_opt = PullOverPath::create(

Comment on lines +141 to +147
auto pull_over_path_opt = PullOverPath::create(
getPlannerType(), goal_id, id, partial_paths, current_pose, goal_pose,
pairs_terminal_velocity_and_accel);
if (!pull_over_path_opt) {
return {};
}
auto & pull_over_path = pull_over_path_opt.value();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
auto pull_over_path_opt = PullOverPath::create(
getPlannerType(), goal_id, id, partial_paths, current_pose, goal_pose,
pairs_terminal_velocity_and_accel);
if (!pull_over_path_opt) {
return {};
}
auto & pull_over_path = pull_over_path_opt.value();
const auto pull_over_path_opt = PullOverPath::create(
getPlannerType(), goal_id, id, partial_paths, current_pose, goal_pose,
pairs_terminal_velocity_and_accel);
if (!pull_over_path_opt) {
return {};
}
const auto & pull_over_path = pull_over_path_opt.value();

Comment on lines +75 to +81
auto pull_over_path_opt = PullOverPath::create(
getPlannerType(), goal_id, id, planner_.getPaths(), planner_.getStartPose(),
planner_.getArcEndPose(), planner_.getPairsTerminalVelocityAndAccel());
if (!pull_over_path_opt) {
return {};
}
auto & pull_over_path = pull_over_path_opt.value();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
auto pull_over_path_opt = PullOverPath::create(
getPlannerType(), goal_id, id, planner_.getPaths(), planner_.getStartPose(),
planner_.getArcEndPose(), planner_.getPairsTerminalVelocityAndAccel());
if (!pull_over_path_opt) {
return {};
}
auto & pull_over_path = pull_over_path_opt.value();
const auto pull_over_path_opt = PullOverPath::create(
getPlannerType(), goal_id, id, planner_.getPaths(), planner_.getStartPose(),
planner_.getArcEndPose(), planner_.getPairsTerminalVelocityAndAccel());
if (!pull_over_path_opt) {
return {};
}
const auto & pull_over_path = pull_over_path_opt.value();

Copy link
Contributor

Choose a reason for hiding this comment

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

(nits): return pull_over_path_opt.value();

Signed-off-by: Mamoru Sobue <[email protected]>
@soblin soblin added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Oct 1, 2024
@soblin soblin enabled auto-merge (squash) October 1, 2024 02:06
@soblin soblin merged commit 90ba42b into autowarefoundation:main Oct 1, 2024
34 of 36 checks passed
@soblin soblin deleted the feat/goal-planner/path-validity branch October 1, 2024 02:18
Copy link

codecov bot commented Oct 1, 2024

Codecov Report

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

Project coverage is 26.03%. Comparing base (21f547d) to head (8ae3466).
Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
...e/src/pull_over_planner/pull_over_planner_base.cpp 0.00% 67 Missing ⚠️
...th_goal_planner_module/src/goal_planner_module.cpp 0.00% 58 Missing ⚠️
...odule/pull_over_planner/pull_over_planner_base.hpp 0.00% 13 Missing ⚠️
...r_module/src/pull_over_planner/shift_pull_over.cpp 0.00% 7 Missing ⚠️
...r_path_goal_planner_module/goal_planner_module.hpp 0.00% 5 Missing ⚠️
...are_behavior_path_goal_planner_module/src/util.cpp 0.00% 4 Missing ⚠️
...dule/src/pull_over_planner/freespace_pull_over.cpp 0.00% 3 Missing ⚠️
...dule/src/pull_over_planner/geometric_pull_over.cpp 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8983      +/-   ##
==========================================
- Coverage   26.05%   26.03%   -0.03%     
==========================================
  Files        1315     1320       +5     
  Lines       98503    98593      +90     
  Branches    40113    40142      +29     
==========================================
  Hits        25668    25668              
- Misses      70117    70194      +77     
- Partials     2718     2731      +13     
Flag Coverage Δ *Carryforward flag
differential 1.22% <0.00%> (?)
total 26.09% <ø> (+0.03%) ⬆️ Carriedforward from 21f547d

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

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

prakash-kannaiah pushed a commit to prakash-kannaiah/autoware.universe that referenced this pull request Oct 9, 2024
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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants