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(behavior_path_planner): unify rtc_interface_ptr in SceneModuleInterface #3242

Conversation

takayuki5168
Copy link
Contributor

@takayuki5168 takayuki5168 commented Apr 1, 2023

Description

Currently, by default, SceneModuleInterface has rtc_interface_ptr_ and its related functions such as lock/unlockRTCCommand.

However, some derived classes from SceneModuleInterface like avoidance have rtc_interface_ptr_left/right_ though they still have rtc_interface_ptr_, which makes the developers confused.
In addition, lock/unlockRTCCommand functions have to be modified following an implicit rule depending on the types of rtc_interface_ptr_ (e.g. only one rtc_interface_ptr, or left and right rtc_interface_ptrs)

To remove the implicit rule and redundant implementation for higher maintainability,

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 Apr 1, 2023
@takayuki5168 takayuki5168 force-pushed the refactor/behavior-path-rtc-interface2 branch from bec4e9a to 1954ac6 Compare April 1, 2023 07:53
@takayuki5168 takayuki5168 changed the title Refactor/behavior path rtc interface2 refactor(behavior_path_planner): unify rtc_interface_ptr in SceneModuleInterface Apr 1, 2023
@codecov
Copy link

codecov bot commented Apr 1, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.04 ⚠️

Comparison is base (442f9ac) 12.38% compared to head (8433338) 12.35%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3242      +/-   ##
==========================================
- Coverage   12.38%   12.35%   -0.04%     
==========================================
  Files        1363     1363              
  Lines       95194    95956     +762     
  Branches    27007    27592     +585     
==========================================
+ Hits        11793    11853      +60     
- Misses      71044    71661     +617     
- Partials    12357    12442      +85     
Flag Coverage Δ *Carryforward flag
differential 5.56% <0.00%> (?)
total 12.40% <ø> (+0.01%) ⬆️ Carriedforward from 442f9ac

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

Impacted Files Coverage Δ
...lanner/scene_module/avoidance/avoidance_module.hpp 0.00% <0.00%> (ø)
...er/scene_module/lane_change/lane_change_module.hpp 0.00% <0.00%> (ø)
..._planner/scene_module/pull_out/pull_out_module.hpp 0.00% <ø> (ø)
...lanner/scene_module/pull_over/pull_over_module.hpp 0.00% <ø> (ø)
...th_planner/scene_module/scene_module_interface.hpp 0.00% <0.00%> (ø)
...nner/scene_module/side_shift/side_shift_module.hpp 0.00% <ø> (ø)
...lanner/include/behavior_path_planner/utilities.hpp 29.16% <ø> (+5.35%) ⬆️
...er/src/scene_module/avoidance/avoidance_module.cpp 0.00% <0.00%> (ø)
...ane_change/external_request_lane_change_module.cpp 0.00% <0.00%> (ø)
...rc/scene_module/lane_change/lane_change_module.cpp 0.00% <0.00%> (ø)
... and 5 more

... and 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

Signed-off-by: Takayuki Murooka <takayuki5168@gmail.com>
Signed-off-by: Takayuki Murooka <takayuki5168@gmail.com>
Signed-off-by: Takayuki Murooka <takayuki5168@gmail.com>
Signed-off-by: Takayuki Murooka <takayuki5168@gmail.com>
Signed-off-by: Takayuki Murooka <takayuki5168@gmail.com>
Signed-off-by: Takayuki Murooka <takayuki5168@gmail.com>
@takayuki5168 takayuki5168 merged commit e664bc4 into autowarefoundation:main Apr 4, 2023
@takayuki5168 takayuki5168 deleted the refactor/behavior-path-rtc-interface2 branch April 4, 2023 06:43
ktro2828 pushed a commit to ktro2828/autoware.universe that referenced this pull request Apr 7, 2023
…leInterface (autowarefoundation#3242)

* refactor(behavior_path_planner): unify rtc_interface instance

Signed-off-by: Takayuki Murooka <takayuki5168@gmail.com>

* fix bug

Signed-off-by: Takayuki Murooka <takayuki5168@gmail.com>

* fix bug

Signed-off-by: Takayuki Murooka <takayuki5168@gmail.com>

* fix

Signed-off-by: Takayuki Murooka <takayuki5168@gmail.com>

* minor change

Signed-off-by: Takayuki Murooka <takayuki5168@gmail.com>

* fix build

Signed-off-by: Takayuki Murooka <takayuki5168@gmail.com>

---------

Signed-off-by: Takayuki Murooka <takayuki5168@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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants