Skip to content

feat(goal_planner): check pull over lane crossing validity when triggering candidate generation thread #10784

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

soblin
Copy link
Contributor

@soblin soblin commented Jun 9, 2025

Description

If lane_change is running when goal_planner's thread is generating path candidates, lane_change can suddenly swerve the path. If ego is still driving on the preferred_lanelets before swerving, and goal_planner still happened to be IDLE, syncWithThreads() is not triggered, thus the backgroud thread keeps using old reference path, which is aligned on the old preferred lanelets. This is problematic when pull over planners process "previous_module_path" w.r.t reference path.

To deal with this issue:

  1. added the flag lane_change_status_changed_ to goal_planner to signal background thread
  2. each planner use get_reference_lanelets_for_pullover instead of getPullOverLanes

image

Related links

How was this PR tested?

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 Jun 9, 2025
Copy link

github-actions bot commented Jun 9, 2025

Thank you for contributing to the Autoware project!

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

Please ensure:

@soblin soblin force-pushed the feat/goal-planner/check-crossing-validity-background branch 5 times, most recently from 7404d0d to e56519f Compare June 13, 2025 08:30
@soblin soblin added run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) labels Jun 13, 2025
Copy link

codecov bot commented Jun 13, 2025

Codecov Report

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

Project coverage is 16.44%. Comparing base (5d5a211) to head (2a88e2d).

Files with missing lines Patch % Lines
...th_goal_planner_module/src/goal_planner_module.cpp 0.00% 22 Missing ⚠️
...are_behavior_path_goal_planner_module/src/util.cpp 0.00% 21 Missing ⚠️
...or_path_goal_planner_module/src/decision_state.cpp 0.00% 5 Missing ⚠️
...dule/src/pull_over_planner/geometric_pull_over.cpp 0.00% 2 Missing ⚠️
.../behavior_path_goal_planner_module/thread_data.hpp 0.00% 1 Missing ⚠️
..._module/src/pull_over_planner/bezier_pull_over.cpp 0.00% 1 Missing ⚠️
...r_module/src/pull_over_planner/shift_pull_over.cpp 0.00% 1 Missing ⚠️
...avior_path_goal_planner_module/src/thread_data.cpp 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10784      +/-   ##
==========================================
+ Coverage   15.80%   16.44%   +0.63%     
==========================================
  Files        1365     1373       +8     
  Lines      100678   101446     +768     
  Branches    32624    32938     +314     
==========================================
+ Hits        15914    16683     +769     
- Misses      72545    73070     +525     
+ Partials    12219    11693     -526     
Flag Coverage Δ *Carryforward flag
daily 17.17% <ø> (+<0.01%) ⬆️ Carriedforward from 5d5a211
daily-cuda 15.81% <ø> (+<0.01%) ⬆️ Carriedforward from 5d5a211
differential 8.25% <0.00%> (?)
total-cuda 15.79% <ø> (+<0.01%) ⬆️ Carriedforward from 5d5a211

*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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@soblin soblin force-pushed the feat/goal-planner/check-crossing-validity-background branch from 0c88d49 to 9292e11 Compare June 13, 2025 12:30
@soblin soblin marked this pull request as ready for review June 13, 2025 16:09
@soblin soblin enabled auto-merge (squash) June 13, 2025 16:09
@kosuke55 kosuke55 self-assigned this Jun 16, 2025
Comment on lines +255 to +260
/**
* @brief find the lanelet that has changed "laterally" from previous lanelet on the routing graph
* @return the lanelet that changed "laterally" if the path is lane changing, otherwise nullopt
*/
std::optional<lanelet::ConstLanelet> find_lane_change_completed_lanelet(
const PathWithLaneId & path, const lanelet::LaneletMapConstPtr lanelet_map,
Copy link
Contributor

@kosuke55 kosuke55 Jun 16, 2025

Choose a reason for hiding this comment

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

question:

A 1 | 3 | 6 | 7
B 2 | 4 | 5 | 8
For example changing lanes from 1→8
1 → 3, 4 → 5,6 → 8

this function returns 4 as lane_change_completed_lanelet ? I imagined 8 from the function name.

Comment on lines +1123 to +1124
return planner_data->route_handler->getLaneletSequence(
*lane_change_complete_lane, backward_length, forward_length);
Copy link
Contributor

Choose a reason for hiding this comment

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

this can't get lanes after goal?

@soblin soblin force-pushed the feat/goal-planner/check-crossing-validity-background branch from 9292e11 to 2a88e2d Compare June 19, 2025 04:16
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
Status: To Triage
Development

Successfully merging this pull request may close these issues.

2 participants