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

fix(route_handler): check all co-located lanes for generating path #4013

Conversation

beyzanurkaya
Copy link
Contributor

@beyzanurkaya beyzanurkaya commented Jun 19, 2023

Description

fixes: 3586

Tests performed

current route_handler package
consider co-located lanelet for generating path

Effects on system behavior

Not applicable.

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

  • There are no open discussions or they are tracked via tickets.

After all checkboxes are checked, anyone who has write access can merge the PR.

@github-actions github-actions bot added the component:planning Route planning, decision-making, and navigation. (auto-assigned) label Jun 19, 2023
@beyzanurkaya beyzanurkaya self-assigned this Jun 19, 2023
@beyzanurkaya beyzanurkaya added the type:bug Software flaws or errors. label Jun 19, 2023
@beyzanurkaya beyzanurkaya requested a review from mehmetdogru June 20, 2023 08:42
@codecov
Copy link

codecov bot commented Jun 20, 2023

Codecov Report

Patch coverage: 25.00% and project coverage change: -0.04 ⚠️

Comparison is base (b5de8a3) 14.28% compared to head (bd3146e) 14.25%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4013      +/-   ##
==========================================
- Coverage   14.28%   14.25%   -0.04%     
==========================================
  Files        1575     1575              
  Lines      108532   109723    +1191     
  Branches    31418    32302     +884     
==========================================
+ Hits        15507    15640     +133     
- Misses      76155    77080     +925     
- Partials    16870    17003     +133     
Flag Coverage Δ *Carryforward flag
differential 16.02% <25.00%> (?)
total 14.28% <ø> (-0.01%) ⬇️ Carriedforward from b5de8a3

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

Impacted Files Coverage Δ
planning/route_handler/src/route_handler.cpp 24.80% <25.00%> (+0.22%) ⬆️

... and 13 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@beyzanurkaya beyzanurkaya force-pushed the fix/chech-overlaped-lanes-for-generating-path branch 2 times, most recently from b2b25f3 to d06925c Compare June 21, 2023 10:02
@beyzanurkaya beyzanurkaya changed the title fix(route_handler)check all co-located lanes for generating path fix(route_handler): check all co-located lanes for generating path Jun 22, 2023
@maxime-clem maxime-clem self-requested a review July 5, 2023 07:21
Copy link
Contributor

@maxime-clem maxime-clem left a comment

Choose a reason for hiding this comment

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

The code looks good and the route planning when ego overlaps multiple lanelets is very good thanks for this PR.

simplescreenrecorder-2023-07-06_16.58.46.mp4

@beyzanurkaya beyzanurkaya force-pushed the fix/chech-overlaped-lanes-for-generating-path branch from d06925c to 2d7e347 Compare July 6, 2023 08:23
beyza and others added 2 commits July 6, 2023 17:55
@beyzanurkaya beyzanurkaya force-pushed the fix/chech-overlaped-lanes-for-generating-path branch 2 times, most recently from 36d61d6 to 9933aa2 Compare July 6, 2023 16:17
@beyzanurkaya
Copy link
Contributor Author

@maxime-clem There were some conflicts this PR. I solved it but I had to add new commits. So can you check and approve again?

@beyzanurkaya beyzanurkaya force-pushed the fix/chech-overlaped-lanes-for-generating-path branch 2 times, most recently from e54f534 to 206e4db Compare July 7, 2023 13:57
Signed-off-by: beyza <bnk@leodrive.ai>
@beyzanurkaya beyzanurkaya force-pushed the fix/chech-overlaped-lanes-for-generating-path branch from ce4bba4 to 076f73f Compare July 7, 2023 14:26
@beyzanurkaya
Copy link
Contributor Author

@maxime-clem I and @ahmeddesokyebrahim worked together on conflicts. I pushed the last version of this PR. Can you review it again?

Copy link
Contributor

@maxime-clem maxime-clem left a comment

Choose a reason for hiding this comment

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

Looks good

@beyzanurkaya
Copy link
Contributor Author

@zulfaqar-azmi-t4 @purewater0901 @kosuke55 @rej55 since you are the code owners can you check/review the PR and approve?

Copy link
Contributor

@mehmetdogru mehmetdogru left a comment

Choose a reason for hiding this comment

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

Looks nice, thank you

Copy link
Contributor

@zulfaqar-azmi-t4 zulfaqar-azmi-t4 left a comment

Choose a reason for hiding this comment

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

LGTM!

@mehmetdogru mehmetdogru merged commit 98b4f90 into autowarefoundation:main Jul 11, 2023
TakaHoribe added a commit to tier4/autoware.universe that referenced this pull request Jul 12, 2023
@beyzanurkaya beyzanurkaya deleted the fix/chech-overlaped-lanes-for-generating-path branch July 12, 2023 09:49
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) type:bug Software flaws or errors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants