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_behavior_velocity_planner_common,autoware_behavior_velocity_stop_line_module): revert pr7710 and Check next_lane_id in createTargetPoint on stop line #7896

Conversation

yhisaki
Copy link
Contributor

@yhisaki yhisaki commented Jul 8, 2024

Description

  1. Revert this PR
  2. In the previous implementation, it was checked whether the lane_id of the intersection between the stop line and the path matched the lane_id held by the module. However, for points on the path near the end of a lane, the lane_id might become the next/previous lane_id of the original id due to the resampling of PathWithLaneId. Therefore, if the stop_line is near the end of a lane, the check for matching lane_ids at the intersection might fail. To address this, even if the lane_id at the intersection does not match the intended id of the stop_line_module, the next lane_id and the previous lane_id of the lane_id are allowed.
    The figure below illustrates the issue that occurs in the previous implementation. The red x represents the PathPointWithLaneId before resampling, the green x represents the PathPointWithLaneId after resampling, the blue lines represent the actual lane boundaries, and the purple line represents the stop_line. Ideally, the stop_line should intersect at a point with lane_id=30134, but due to the resampling effect, it is mistakenly determined to intersect at a point with lane_id=30322.
Untitled (1) (1)

Tests performed

tested on psim

yhisaki added 2 commits July 8, 2024 18:02
Signed-off-by: Y.Hisaki <yhisaki31@gmail.com>
@github-actions github-actions bot added the component:planning Route planning, decision-making, and navigation. (auto-assigned) label Jul 8, 2024
Copy link

github-actions bot commented Jul 8, 2024

Thank you for contributing to the Autoware project!

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

Please ensure:

Copy link

codecov bot commented Jul 8, 2024

Codecov Report

Attention: Patch coverage is 64.00000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 28.42%. Comparing base (f28ccd8) to head (f117c9d).
Report is 1476 commits behind head on main.

Files with missing lines Patch % Lines
...e_behavior_velocity_stop_line_module/src/scene.cpp 0.00% 6 Missing ⚠️
...avior_velocity_detection_area_module/src/scene.cpp 50.00% 1 Missing ⚠️
...topping_area_module/src/scene_no_stopping_area.cpp 0.00% 1 Missing ⚠️
...elocity_virtual_traffic_light_module/src/scene.cpp 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7896      +/-   ##
==========================================
- Coverage   28.43%   28.42%   -0.01%     
==========================================
  Files        1588     1592       +4     
  Lines      116175   116235      +60     
  Branches    49542    49549       +7     
==========================================
+ Hits        33032    33044      +12     
- Misses      74131    74195      +64     
+ Partials     9012     8996      -16     
Flag Coverage Δ *Carryforward flag
differential 16.05% <64.00%> (?)
total 28.43% <ø> (-0.01%) ⬇️ Carriedforward from 915c102

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

Copy link
Contributor

@takayuki5168 takayuki5168 left a comment

Choose a reason for hiding this comment

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

getNextLaneletWithinRoute should be used instead of getNextLanelets

@@ -51,8 +49,15 @@ bool StopLineModule::modifyPathVelocity(PathWithLaneId * path, StopReason * stop
stop_line_[0], stop_line_[1], planner_data_->stop_line_extend_length);

// Calculate stop pose and insert index
auto lane = this->planner_data_->route_handler_->getLaneletsFromId(lane_id_);
auto next_lanes = this->planner_data_->route_handler_->getNextLanelets(lane);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a previous lanes as well.

@@ -51,8 +49,15 @@ bool StopLineModule::modifyPathVelocity(PathWithLaneId * path, StopReason * stop
stop_line_[0], stop_line_[1], planner_data_->stop_line_extend_length);

// Calculate stop pose and insert index
auto lane = this->planner_data_->route_handler_->getLaneletsFromId(lane_id_);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a comment since this change is not straightforward.

Signed-off-by: Y.Hisaki <yhisaki31@gmail.com>
@yhisaki
Copy link
Contributor Author

yhisaki commented Jul 8, 2024

getNextLaneletWithinRoute should be used instead of getNextLanelets

It seems that there is a bug in the behavior_velocity_planner where the route is not subscribed, and the getNextLaneletsWithinRoute function cannot be used. In the future, I would like to fix this bug, but temporarily, I addressed it by using getNextLanelets.

Copy link

stale bot commented Sep 6, 2024

This pull request has been automatically marked as stale because it has not had recent activity.

@stale stale bot added the status:stale Inactive or outdated issues. (auto-assigned) label Sep 6, 2024
@mitsudome-r
Copy link
Member

@yhisaki @takayuki5168 Do you still need this modification? I suggest you to revert it quickly if you need to since it will bring more conflicts.

@stale stale bot removed the status:stale Inactive or outdated issues. (auto-assigned) label Dec 17, 2024
@yhisaki yhisaki closed this Dec 17, 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.

3 participants