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(lane_change): refactor getLaneChangePaths function #8909

Conversation

mkquda
Copy link
Contributor

@mkquda mkquda commented Sep 19, 2024

Description

The function getLaneChangePaths in LC module is the main part of LC code and is responsible for sampling different candidate paths and verifying the validity and safety of the candidate path. However the function is very big and complex, which makes it difficult to understand and modify.

This PR aims to refactor the getLaneChangePaths function to improve code readability and granularity and reduce its complexity.

Changes

  • Add struct PhaseMetrics to group relevant measurements (duration, length, velocity, lon. acc, lat. acc) for LC phases
  • Add function calc_prepare_phase_metrics to compute and return a list of sampled prepare phase metrics
  • Add function calc_shift_phase_metrics to compute and return a list of sampled lane changing phase metrics
  • Remove the necessity of computing target segment, and use reference path from target lane directly instead
  • Add function getCandidatePath to process LC info and construct the cadidate path
  • Add function checkCandidatePathSafety to verify LC candidate path

Flowchart of getLaneChangePaths function after refactoring
Untitled Diagram

Related links

None.

How was this PR tested?

  1. Build complete
  2. PSIM
  3. Internal TIER IV evaluator

Notes for reviewers

None.

Interface changes

None.

Effects on system behavior

None.

Signed-off-by: mohammad alqudah <[email protected]>
Signed-off-by: mohammad alqudah <[email protected]>
Signed-off-by: mohammad alqudah <[email protected]>
Signed-off-by: mohammad alqudah <[email protected]>
…ule' into RT1-7829-refactor-code-get-lane-change-paths
Signed-off-by: mohammad alqudah <[email protected]>
@github-actions github-actions bot added type:documentation Creating or refining documentation. (auto-assigned) component:planning Route planning, decision-making, and navigation. (auto-assigned) labels Sep 19, 2024
Copy link

github-actions bot commented Sep 19, 2024

Thank you for contributing to the Autoware project!

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

Please ensure:

@github-actions github-actions bot removed the type:documentation Creating or refining documentation. (auto-assigned) label Sep 19, 2024
Copy link

codecov bot commented Sep 19, 2024

Codecov Report

Attention: Patch coverage is 0.48309% with 206 lines in your changes missing coverage. Please review.

Project coverage is 28.45%. Comparing base (d25e9a1) to head (e89ef23).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...are_behavior_path_lane_change_module/src/scene.cpp 0.00% 116 Missing ⚠️
..._path_lane_change_module/src/utils/calculation.cpp 0.00% 55 Missing ⚠️
...ior_path_lane_change_module/utils/data_structs.hpp 5.55% 17 Missing ⚠️
...nning/autoware_route_handler/src/route_handler.cpp 0.00% 11 Missing ⚠️
...havior_path_lane_change_module/src/utils/utils.cpp 0.00% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8909      +/-   ##
==========================================
- Coverage   28.47%   28.45%   -0.02%     
==========================================
  Files        1314     1318       +4     
  Lines       98279    98318      +39     
  Branches    39961    39969       +8     
==========================================
- Hits        27985    27980       -5     
- Misses      70166    70209      +43     
- Partials      128      129       +1     
Flag Coverage Δ *Carryforward flag
differential 28.04% <0.48%> (?)
total 28.52% <ø> (+0.05%) ⬆️ Carriedforward from fa5a5fe

*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

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

I only checked the code and it looks good.
The CI failure is caused by a random test failure so re-running it should fix the issue. (the random failure is supposed to be fixed by #8799 but I guess not).

@github-actions github-actions bot added the type:documentation Creating or refining documentation. (auto-assigned) label Sep 20, 2024
Signed-off-by: mohammad alqudah <[email protected]>
@mkquda mkquda added the tag:deploy-docs Mark for deploy-docs action generation. (used-by-ci) label Sep 20, 2024
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.

Nice. LGTM!

@mkquda mkquda enabled auto-merge (squash) September 25, 2024 06:38
@mkquda mkquda merged commit 656a78e into autowarefoundation:main Sep 25, 2024
24 of 26 checks passed
@mkquda mkquda deleted the RT1-7829-refactor-code-get-lane-change-paths branch September 25, 2024 07:22
prakash-kannaiah pushed a commit to prakash-kannaiah/autoware.universe that referenced this pull request Oct 9, 2024
…foundation#8909)

* refactor lane change utility funcions

Signed-off-by: mohammad alqudah <[email protected]>

* LC utility function to get distance to next regulatory element

Signed-off-by: mohammad alqudah <[email protected]>

* don't activate LC module when close to regulatory element

Signed-off-by: mohammad alqudah <[email protected]>

* modify threshold distance calculation

Signed-off-by: mohammad alqudah <[email protected]>

* move regulatory element check to canTransitFailureState() function

Signed-off-by: mohammad alqudah <[email protected]>

* always run LC module if approaching terminal point

Signed-off-by: mohammad alqudah <[email protected]>

* use max possible LC length as threshold

Signed-off-by: mohammad alqudah <[email protected]>

* update LC readme

Signed-off-by: mohammad alqudah <[email protected]>

* refactor implementation

Signed-off-by: mohammad alqudah <[email protected]>

* update readme

Signed-off-by: mohammad alqudah <[email protected]>

* refactor checking data validity

Signed-off-by: mohammad alqudah <[email protected]>

* refactor sampling of prepare phase metrics and lane changing phase metrics

Signed-off-by: mohammad alqudah <[email protected]>

* add route handler function to get pose from 2d arc length

Signed-off-by: mohammad alqudah <[email protected]>

* refactor candidate path generation

Signed-off-by: mohammad alqudah <[email protected]>

* refactor candidate path safety check

Signed-off-by: mohammad alqudah <[email protected]>

* fix variable name

Signed-off-by: mohammad alqudah <[email protected]>

* Update planning/autoware_route_handler/src/route_handler.cpp

Co-authored-by: Zulfaqar Azmi <[email protected]>

* correct parameter name

Signed-off-by: mohammad alqudah <[email protected]>

* set prepare segment velocity after taking max path velocity value

Signed-off-by: mohammad alqudah <[email protected]>

* update LC README

Signed-off-by: mohammad alqudah <[email protected]>

* minor changes

Signed-off-by: mohammad alqudah <[email protected]>

* check phase length difference with previos valid candidate path

Signed-off-by: mohammad alqudah <[email protected]>

* change logger name

Signed-off-by: mohammad alqudah <[email protected]>

* change functions names to snake case

Signed-off-by: mohammad alqudah <[email protected]>

* use snake case for function names

Signed-off-by: mohammad alqudah <[email protected]>

* add colors to flow chart in README

Signed-off-by: mohammad alqudah <[email protected]>

---------

Signed-off-by: mohammad alqudah <[email protected]>
Co-authored-by: Zulfaqar Azmi <[email protected]>
Signed-off-by: prakash-kannaiah <[email protected]>
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) tag:deploy-docs Mark for deploy-docs action generation. (used-by-ci) tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) type:documentation Creating or refining documentation. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants