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(avoidance): the module ignored merging objects unexpectedly #6601

Merged
merged 6 commits into from
Mar 15, 2024

Conversation

satoshi-ota
Copy link
Contributor

@satoshi-ota satoshi-ota commented Mar 12, 2024

Description

Related PR: autowarefoundation/autoware_launch#923
Related ticket: https://tier4.atlassian.net/browse/RT1-202

Fix bug. The module ignored merging vehicle unexpectedly. Originally, I designed the module generates avoidance path for every objects that are NOT on ego lane. (Only for objects in intersection, the module sometimes ignores them.)

simplescreenrecorder-2024-03-14_16.26.27.mp4

And, this PR includes small refactoring as well.

Tests performed

webauto ci scenario run --project-id prd_jt --scenario-id f9e88ceb-0e06-48d5-9bad-be5ed4a244a0 --scenario-parameters '__tier4_modifier_npc_position_t=3'

Effects on system behavior

Nothing.

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 Mar 12, 2024
@satoshi-ota satoshi-ota force-pushed the fix/force-avoid-condition branch from 572b9f6 to 27b5d1c Compare March 14, 2024 02:22
Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>
Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>
@satoshi-ota satoshi-ota force-pushed the fix/force-avoid-condition branch 2 times, most recently from 62ee88f to d6ac70d Compare March 14, 2024 07:07
Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>
Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>
Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>
@satoshi-ota satoshi-ota force-pushed the fix/force-avoid-condition branch from d6ac70d to d14d2f3 Compare March 14, 2024 07:12
@satoshi-ota satoshi-ota marked this pull request as ready for review March 14, 2024 07:29
@satoshi-ota satoshi-ota changed the title fix(avoidance): avoid merging vehicle if it's NOT on ego lane fix(avoidance): the module ignore merging objects unexpectedly Mar 14, 2024
@shmpwk
Copy link
Contributor

shmpwk commented Mar 14, 2024

@Owen-Liuyuxuan Could you review this code? Ota-san will test scenario so please just check the code.

@Owen-Liuyuxuan
Copy link
Contributor

Thanks for the great response and contribution. The refactoring improves the readability of the codes; the codes update the target object filtering logic which should correctly implement the logic we hope to have.

I notice that overhang_lanelet array is added to the debug marker arrays, could there be additional materials/documentation on what we can see with the new debug markers?

@@ -188,10 +188,25 @@ MarkerArray createObjectInfoMarkerArray(const ObjectDataArray & objects, std::st
return msg;
}

MarkerArray createOverhangLaneletMarkerArray(const ObjectDataArray & objects, std::string && ns)
Copy link
Contributor

Choose a reason for hiding this comment

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

Will there be additional documentation or image showing the usage of this marker?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Owen-Liuyuxuan Thank you for your comment.

Unfortunatelly, there is no document about overhang lanelet. It is one of the ego driving lanelets (closest lanelet to the object) and used in the logic to check whether the object is on the ego lane.

Actually, it is difficult to understand without any comment. So, I'll add above brief explanation on code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Owen-Liuyuxuan I updated comment in code in 1344e6c.

@satoshi-ota
Copy link
Contributor Author

Please check autowarefoundation/autoware_launch#923 as well.

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>
@satoshi-ota satoshi-ota added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Mar 14, 2024
@satoshi-ota satoshi-ota changed the title fix(avoidance): the module ignore merging objects unexpectedly fix(avoidance): the module ignored merging objects unexpectedly Mar 14, 2024
Copy link

codecov bot commented Mar 15, 2024

Codecov Report

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

Project coverage is 14.79%. Comparing base (f9fbd39) to head (1344e6c).
Report is 2 commits behind head on main.

Files Patch % Lines
...nning/behavior_path_avoidance_module/src/utils.cpp 0.00% 14 Missing ⚠️
...nning/behavior_path_avoidance_module/src/debug.cpp 14.28% 6 Missing and 6 partials ⚠️
.../include/behavior_path_avoidance_module/helper.hpp 0.00% 10 Missing ⚠️
...nning/behavior_path_avoidance_module/src/scene.cpp 0.00% 9 Missing ⚠️
...path_avoidance_module/src/shift_line_generator.cpp 0.00% 7 Missing ⚠️
...th_avoidance_by_lane_change_module/src/manager.cpp 0.00% 0 Missing and 1 partial ⚠️
...ehavior_path_avoidance_module/parameter_helper.hpp 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6601      +/-   ##
==========================================
- Coverage   14.79%   14.79%   -0.01%     
==========================================
  Files        1920     1920              
  Lines      132379   132399      +20     
  Branches    39345    39360      +15     
==========================================
+ Hits        19584    19586       +2     
- Misses      90950    90963      +13     
- Partials    21845    21850       +5     
Flag Coverage Δ *Carryforward flag
differential 12.98% <3.57%> (?)
total 14.79% <ø> (+<0.01%) ⬆️ Carriedforward from f9fbd39

*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

@shmpwk shmpwk left a comment

Choose a reason for hiding this comment

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

LGTM

@satoshi-ota satoshi-ota merged commit c653236 into autowarefoundation:main Mar 15, 2024
34 of 37 checks passed
@satoshi-ota satoshi-ota deleted the fix/force-avoid-condition branch March 15, 2024 02:01
yhisaki pushed a commit to yhisaki/autoware.universe that referenced this pull request Mar 15, 2024
…warefoundation#6601)

* feat(avoidance): output overhang lanelet

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* fix(avoidance): avoid merging vehicle if it's NOT on ego lane.

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* fix(avoidance): add flag to identify ambiguous vehicle

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* refactor(avoidance): add helper function

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* refactor(avoidance): rename param

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* chore(avoidance): update comment

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

---------

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>
satoshi-ota added a commit to tier4/autoware.universe that referenced this pull request Mar 19, 2024
…warefoundation#6601)

* feat(avoidance): output overhang lanelet

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* fix(avoidance): avoid merging vehicle if it's NOT on ego lane.

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* fix(avoidance): add flag to identify ambiguous vehicle

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* refactor(avoidance): add helper function

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* refactor(avoidance): rename param

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* chore(avoidance): update comment

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

---------

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>
kaigohirao pushed a commit to kaigohirao/autoware.universe that referenced this pull request Mar 22, 2024
…warefoundation#6601)

* feat(avoidance): output overhang lanelet

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* fix(avoidance): avoid merging vehicle if it's NOT on ego lane.

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* fix(avoidance): add flag to identify ambiguous vehicle

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* refactor(avoidance): add helper function

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* refactor(avoidance): rename param

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* chore(avoidance): update comment

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

---------

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>
Signed-off-by: kaigohirao <kaigo.hirao@proxima-ai-tech.com>
satoshi-ota added a commit to tier4/autoware.universe that referenced this pull request Mar 28, 2024
…warefoundation#6601)

* feat(avoidance): output overhang lanelet

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* fix(avoidance): avoid merging vehicle if it's NOT on ego lane.

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* fix(avoidance): add flag to identify ambiguous vehicle

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* refactor(avoidance): add helper function

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* refactor(avoidance): rename param

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* chore(avoidance): update comment

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

---------

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>
satoshi-ota added a commit to tier4/autoware.universe that referenced this pull request Apr 2, 2024
…warefoundation#6601)

* feat(avoidance): output overhang lanelet

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* fix(avoidance): avoid merging vehicle if it's NOT on ego lane.

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* fix(avoidance): add flag to identify ambiguous vehicle

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* refactor(avoidance): add helper function

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* refactor(avoidance): rename param

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* chore(avoidance): update comment

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

---------

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>
satoshi-ota added a commit to tier4/autoware.universe that referenced this pull request Apr 8, 2024
…warefoundation#6601)

* feat(avoidance): output overhang lanelet

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* fix(avoidance): avoid merging vehicle if it's NOT on ego lane.

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* fix(avoidance): add flag to identify ambiguous vehicle

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* refactor(avoidance): add helper function

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* refactor(avoidance): rename param

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* chore(avoidance): update comment

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

---------

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>
karishma1911 pushed a commit to Interplai/autoware.universe that referenced this pull request Jun 3, 2024
…warefoundation#6601)

* feat(avoidance): output overhang lanelet

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* fix(avoidance): avoid merging vehicle if it's NOT on ego lane.

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* fix(avoidance): add flag to identify ambiguous vehicle

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* refactor(avoidance): add helper function

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* refactor(avoidance): rename param

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* chore(avoidance): update comment

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

---------

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>
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