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(avoidance_module): change implementation to lambda #486

Conversation

zulfaqar-azmi-t4
Copy link
Contributor

@zulfaqar-azmi-t4 zulfaqar-azmi-t4 commented Mar 7, 2022

In the generateExtendedDrivableArea function, the implementation to extend
drivable areas with reference to object is moved to route_handler library.

This will allow the function to be reused for some future planned updates.

The refactor doesn't affect the behavior of the avoidance_module.

Signed-off-by: Muhammad Zulfaqar Azmi zulfaqar.azmi@tier4.jp

Related Issue(required)

#485
#499

Description(required)

Refactoring the function so that the intent is much more clear, therefore it doensn't affect the behavior of the avoidance_module.
This also allows the function to be reused for debug visualization if necessary.

Review Procedure(required)

See build pass (no behavior change)
image

Related PR(optional)

#285
#365

Pre-Review Checklist for the PR Author

PR Author should check the checkboxes below when creating the PR.

If you are adding new package following items are required:

  • Documentation with description of the package is available
  • A sample launch file and parameter file are available if the package contains executable nodes

Checklist for the PR Reviewer

Reviewers should check the checkboxes below before approval.

  • Commits are properly organized and messages are according to the guideline
  • PR title describes the changes

Post-Review Checklist for the PR Author

PR Author should check the checkboxes below before merging.

  • All open points are addressed and tracked via issues or tickets

CI Checks

  • Build and test for PR / build-and-test-pr: Required to pass before the merge.
  • Build and test for PR / clang-tidy-pr: NOT required to pass before the merge. It is up to the reviewer(s). Found false positives? See the [guidelines][clang-tidy-guidelines].
  • Check spelling: NOT required to pass before the merge. It is up to the reviewer(s). See here if you want to add some words to the spell check dictionary.

@codecov
Copy link

codecov bot commented Mar 7, 2022

Codecov Report

Merging #486 (b6ae886) into main (5c9302d) will decrease coverage by 8.08%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             main    #486      +/-   ##
=========================================
- Coverage   10.94%   2.85%   -8.09%     
=========================================
  Files         704     110     -594     
  Lines       49690   10702   -38988     
  Branches     6655    1101    -5554     
=========================================
- Hits         5440     306    -5134     
+ Misses      39888   10252   -29636     
+ Partials     4362     144    -4218     
Impacted Files Coverage Δ
...er/src/scene_module/avoidance/avoidance_module.cpp 0.00% <0.00%> (ø)
planning/route_handler/src/route_handler.cpp 0.00% <0.00%> (ø)
...ene_module/occlusion_spot/occlusion_spot_utils.hpp 40.00% <0.00%> (-26.67%) ⬇️
...behavior_velocity_planner/src/utilization/util.cpp 32.43% <0.00%> (-1.62%) ⬇️
...vior_velocity_planner/include/utilization/util.hpp 12.00% <0.00%> (ø)
...ocity_planner/src/utilization/path_utilization.cpp 0.00% <0.00%> (ø)
..._planner/src/scene_module/occlusion_spot/debug.cpp 0.00% <0.00%> (ø)
...lanner/src/scene_module/occlusion_spot/manager.cpp 0.00% <0.00%> (ø)
...ene_module/occlusion_spot/scene_occlusion_spot.cpp 0.00% <0.00%> (ø)
... and 596 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c9302d...b6ae886. Read the comment docs.

zulfaqar-azmi-t4 and others added 7 commits March 10, 2022 11:13
In the generateExtendedDrivableArea function, the implementation to extend
drivable areas with reference to object is refactor to lambda.

This will allow the function to be used for some future planned updates.

The refactor doesn't affect the behavior of the avoidance_module.

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
By default, the parameter is true, therefore it doesn't affect the original
behavior.

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
The implementation is made into function and moved into route handler

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
@zulfaqar-azmi-t4 zulfaqar-azmi-t4 force-pushed the 485-refactor-code-to-lambda-to-allow-reuse branch from 50e717a to 65313d4 Compare March 10, 2022 02:13
Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
taikitanaka3
taikitanaka3 previously approved these changes Mar 11, 2022
Copy link
Contributor

@taikitanaka3 taikitanaka3 left a comment

Choose a reason for hiding this comment

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

LGTM

Also modified some avoidance_module to include the refactored functions

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
@zulfaqar-azmi-t4 zulfaqar-azmi-t4 merged commit a03ab1f into autowarefoundation:main Mar 13, 2022
@zulfaqar-azmi-t4 zulfaqar-azmi-t4 deleted the 485-refactor-code-to-lambda-to-allow-reuse branch March 13, 2022 07:55
boyali referenced this pull request in boyali/autoware.universe Sep 28, 2022
* refactor(avoidance_module): change implementation to lambda

In the generateExtendedDrivableArea function, the implementation to extend
drivable areas with reference to object is refactor to lambda.

This will allow the function to be used for some future planned updates.

The refactor doesn't affect the behavior of the avoidance_module.

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

* refactor: add default flag to extend_to_opposite side

By default, the parameter is true, therefore it doesn't affect the original
behavior.

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

* ci(pre-commit): autofix

* fix: rework function for better extension

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

* refactor(route handler, avoidance_module): move the implementation

The implementation is made into function and moved into route handler

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

* Refactor: convert lambdas to function

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

* chore: add comments for comments for documentation in the header file

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

* Refactor: local variable rename

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

* refactor: make a wrapper function to get furthest linestrings

Also modified some avoidance_module to include the refactored functions

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
boyali referenced this pull request in boyali/autoware.universe Oct 3, 2022
* refactor(avoidance_module): change implementation to lambda

In the generateExtendedDrivableArea function, the implementation to extend
drivable areas with reference to object is refactor to lambda.

This will allow the function to be used for some future planned updates.

The refactor doesn't affect the behavior of the avoidance_module.

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

* refactor: add default flag to extend_to_opposite side

By default, the parameter is true, therefore it doesn't affect the original
behavior.

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

* ci(pre-commit): autofix

* fix: rework function for better extension

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

* refactor(route handler, avoidance_module): move the implementation

The implementation is made into function and moved into route handler

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

* Refactor: convert lambdas to function

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

* chore: add comments for comments for documentation in the header file

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

* Refactor: local variable rename

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

* refactor: make a wrapper function to get furthest linestrings

Also modified some avoidance_module to include the refactored functions

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
boyali referenced this pull request in boyali/autoware.universe Oct 19, 2022
* refactor(avoidance_module): change implementation to lambda

In the generateExtendedDrivableArea function, the implementation to extend
drivable areas with reference to object is refactor to lambda.

This will allow the function to be used for some future planned updates.

The refactor doesn't affect the behavior of the avoidance_module.

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

* refactor: add default flag to extend_to_opposite side

By default, the parameter is true, therefore it doesn't affect the original
behavior.

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

* ci(pre-commit): autofix

* fix: rework function for better extension

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

* refactor(route handler, avoidance_module): move the implementation

The implementation is made into function and moved into route handler

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

* Refactor: convert lambdas to function

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

* chore: add comments for comments for documentation in the header file

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

* Refactor: local variable rename

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

* refactor: make a wrapper function to get furthest linestrings

Also modified some avoidance_module to include the refactored functions

Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
tkimura4 referenced this pull request in tier4/autoware.universe Oct 24, 2022
* add rviz helper for ad api

Signed-off-by: khtan <tkh.my.p@gmail.com>

* sync for init pose

Signed-off-by: khtan <tkh.my.p@gmail.com>

* add missing sync

Signed-off-by: khtan <tkh.my.p@gmail.com>

* add rviz adaptor

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* fix launch_dummy_localization

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* add dependency

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* add dependency

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* ci(pre-commit): autofix

Signed-off-by: khtan <tkh.my.p@gmail.com>
Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
Co-authored-by: Takagi, Isamu <isamu.takagi@tier4.jp>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
dmoszynski pushed a commit to RobotecAI/autoware.universe that referenced this pull request Jun 22, 2023
kyoichi-sugahara pushed a commit that referenced this pull request Sep 16, 2023
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants