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(bpp): use pluginlib to load scene module #5771

Merged

Conversation

satoshi-ota
Copy link
Contributor

@satoshi-ota satoshi-ota commented Dec 5, 2023

Description

🤖[deprecated] Generated by Copilot at 4ff450e

This pull request refactors the behavior path planner node and its scene modules to use pluginlib for dynamic plugin loading and unloading. It also simplifies the launch and configuration of the node and its modules, and fixes some minor bugs and issues. It modifies several files, including launch files, CMakeLists.txt, header and source files, package.xml, and plugins.xml.

Tests performed

Confirmed that bpp loaded all scene modules correctly.

[INFO] [component_container_mt-29]: process started with pid [145130]                                                                                                                                           
[component_container_mt-29] [INFO 1701737049.463438690] [planning.scenario_planning.lane_driving.behavior_planning.behavior_planning_container]: Load Library: /home/satoshi/pilot-auto/install/behavior_path_planner/lib/libbehavior_path_planner_lib.so
[component_container_mt-29] [INFO 1701737049.570749397] [planning.scenario_planning.lane_driving.behavior_planning.behavior_planning_container]: Found class: rclcpp_components::NodeFactoryTemplate<behavior_path_planner::BehaviorPathPlannerNode>
[component_container_mt-29] [INFO 1701737049.576008845] [planning.scenario_planning.lane_driving.behavior_planning.behavior_planning_container]: Instantiate class: rclcpp_components::NodeFactoryTemplate<behavior_path_planner::BehaviorPathPlannerNode>
[component_container_mt-29] [INFO 1701737049.722422275] [planning.scenario_planning.lane_driving.behavior_planning.behavior_path_planner]: The scene plugin 'behavior_path_planner::AvoidanceModuleManager' is loaded.
[component_container_mt-29] [INFO 1701737049.728016083] [planning.scenario_planning.lane_driving.behavior_planning.behavior_path_planner]: The scene plugin 'behavior_path_planner::DynamicAvoidanceModuleManager' is loaded.
[component_container_mt-29] [INFO 1701737049.731040344] [planning.scenario_planning.lane_driving.behavior_planning.behavior_path_planner]: The scene plugin 'behavior_path_planner::SideShiftModuleManager' is loaded.
[component_container_mt-29] [INFO 1701737049.768085413] [planning.scenario_planning.lane_driving.behavior_planning.behavior_path_planner]: The scene plugin 'behavior_path_planner::StartPlannerModuleManager' is loaded.
[component_container_mt-29] [INFO 1701737049.945528895] [planning.scenario_planning.lane_driving.behavior_planning.behavior_path_planner]: The scene plugin 'behavior_path_planner::GoalPlannerModuleManager' is loaded.
[component_container_mt-29] [INFO 1701737050.164803561] [planning.scenario_planning.lane_driving.behavior_planning.behavior_path_planner]: The scene plugin 'behavior_path_planner::LaneChangeRightModuleManager' is loaded.
[component_container_mt-29] [INFO 1701737050.190003251] [planning.scenario_planning.lane_driving.behavior_planning.behavior_path_planner]: The scene plugin 'behavior_path_planner::LaneChangeLeftModuleManager' is loaded.
[component_container_mt-29] [INFO 1701737050.236931556] [planning.scenario_planning.lane_driving.behavior_planning.behavior_path_planner]: The scene plugin 'behavior_path_planner::ExternalRequestLaneChangeRightModuleManager' is loaded.
[component_container_mt-29] [INFO 1701737050.411370194] [planning.scenario_planning.lane_driving.behavior_planning.behavior_path_planner]: The scene plugin 'behavior_path_planner::ExternalRequestLaneChangeLeftModuleManager' is loaded.
[component_container_mt-29] [INFO 1701737050.600689989] [planning.scenario_planning.lane_driving.behavior_planning.behavior_path_planner]: The scene plugin 'behavior_path_planner::AvoidanceByLaneChangeModuleManager' is loaded.

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 component:planning Route planning, decision-making, and navigation. (auto-assigned) component:launch Launch files, scripts and initialization tools. (auto-assigned) labels Dec 5, 2023
@satoshi-ota satoshi-ota added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Dec 5, 2023
@satoshi-ota satoshi-ota marked this pull request as ready for review December 5, 2023 00:50
Copy link

codecov bot commented Dec 5, 2023

Codecov Report

Attention: 1212 lines in your changes are missing coverage. Please review.

Comparison is base (765a596) 15.32% compared to head (147db3e) 12.68%.
Report is 154 commits behind head on main.

Files Patch % Lines
...anner/src/utils/avoidance/shift_line_generator.cpp 13.19% 435 Missing and 65 partials ⚠️
.../scene_module/goal_planner/goal_planner_module.cpp 0.58% 168 Missing and 2 partials ⚠️
...er/src/scene_module/avoidance/avoidance_module.cpp 10.48% 111 Missing and 17 partials ⚠️
...ule/dynamic_avoidance/dynamic_avoidance_module.cpp 1.56% 63 Missing ⚠️
..._path_planner/src/marker_utils/avoidance/debug.cpp 0.00% 61 Missing ⚠️
...cene_module/start_planner/start_planner_module.cpp 8.47% 54 Missing ⚠️
...ath_planner/src/scene_module/avoidance/manager.cpp 18.86% 0 Missing and 43 partials ⚠️
...th_planner/src/scene_module/lane_change/normal.cpp 0.00% 28 Missing ⚠️
..._planner/src/scene_module/goal_planner/manager.cpp 12.00% 2 Missing and 20 partials ⚠️
...h_planner/src/scene_module/lane_change/manager.cpp 40.54% 1 Missing and 21 partials ⚠️
... and 19 more
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #5771       +/-   ##
===========================================
- Coverage   15.32%   12.68%    -2.65%     
===========================================
  Files        1721      107     -1614     
  Lines      118559    14917   -103642     
  Branches    37995     8291    -29704     
===========================================
- Hits        18169     1892    -16277     
+ Misses      79657    10136    -69521     
+ Partials    20733     2889    -17844     
Flag Coverage Δ
differential 12.68% <13.36%> (?)
total ?

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

SideShiftParameters p{};

std::string ns = "side_shift.";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
std::string ns = "side_shift.";
const std::string ns = "side_shift.";

Copy link
Contributor

Choose a reason for hiding this comment

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

and others

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kosuke55 Fixed in 81332c9.

Comment on lines +68 to +84
void PlannerManager::removeScenePlugin(rclcpp::Node & node, const std::string & name)
{
auto it = std::remove_if(manager_ptrs_.begin(), manager_ptrs_.end(), [&](const auto plugin) {
return plugin->name() == name;
});

if (it == manager_ptrs_.end()) {
RCLCPP_WARN_STREAM(
node.get_logger(),
"The scene plugin '" << name << "' is not found in the registered modules.");
} else {
manager_ptrs_.erase(it, manager_ptrs_.end());
processing_time_.erase(name);
RCLCPP_INFO_STREAM(node.get_logger(), "The scene plugin '" << name << "' is unloaded.");
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

just a question.
this is not currently used, but is this a mechanism to dynamically disable modules in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

YES.

@satoshi-ota satoshi-ota force-pushed the refactor/bpp-use-pluginlib branch from 4ff450e to 81332c9 Compare December 5, 2023 03:10
Copy link
Contributor

@kosuke55 kosuke55 left a comment

Choose a reason for hiding this comment

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

LGTM! thanks!"

Signed-off-by: satoshi-ota <[email protected]>
@satoshi-ota satoshi-ota force-pushed the refactor/bpp-use-pluginlib branch from 81332c9 to 147db3e Compare December 5, 2023 04:21
@satoshi-ota satoshi-ota merged commit c6788cf into autowarefoundation:main Dec 5, 2023
@satoshi-ota satoshi-ota deleted the refactor/bpp-use-pluginlib branch December 5, 2023 06:04
danielsanchezaran pushed a commit to tier4/autoware.universe that referenced this pull request Dec 15, 2023
…#5771)

* refactor(bpp): use pluginlib

Signed-off-by: satoshi-ota <[email protected]>

* refactor(tier4_planning_launch): update launcher

Signed-off-by: satoshi-ota <[email protected]>

* refactor(avoidance): support pluginlib

Signed-off-by: satoshi-ota <[email protected]>

* refactor(lane_change): support pluginlib

Signed-off-by: satoshi-ota <[email protected]>

* refactor(dynamic_avoidance): support pluginlib

Signed-off-by: satoshi-ota <[email protected]>

* refactor(goal_planner): support pluginlib

Signed-off-by: satoshi-ota <[email protected]>

* refactor(side_shift): support pluginlib

Signed-off-by: satoshi-ota <[email protected]>

* refactor(start_planner): support pluginlib

Signed-off-by: satoshi-ota <[email protected]>

* refactor(bpp): move interface

Signed-off-by: satoshi-ota <[email protected]>

* fix(bpp): add const

Signed-off-by: satoshi-ota <[email protected]>

---------

Signed-off-by: satoshi-ota <[email protected]>
karishma1911 pushed a commit to Interplai/autoware.universe that referenced this pull request Jun 3, 2024
…#5771)

* refactor(bpp): use pluginlib

Signed-off-by: satoshi-ota <[email protected]>

* refactor(tier4_planning_launch): update launcher

Signed-off-by: satoshi-ota <[email protected]>

* refactor(avoidance): support pluginlib

Signed-off-by: satoshi-ota <[email protected]>

* refactor(lane_change): support pluginlib

Signed-off-by: satoshi-ota <[email protected]>

* refactor(dynamic_avoidance): support pluginlib

Signed-off-by: satoshi-ota <[email protected]>

* refactor(goal_planner): support pluginlib

Signed-off-by: satoshi-ota <[email protected]>

* refactor(side_shift): support pluginlib

Signed-off-by: satoshi-ota <[email protected]>

* refactor(start_planner): support pluginlib

Signed-off-by: satoshi-ota <[email protected]>

* refactor(bpp): move interface

Signed-off-by: satoshi-ota <[email protected]>

* fix(bpp): add const

Signed-off-by: satoshi-ota <[email protected]>

---------

Signed-off-by: satoshi-ota <[email protected]>
Signed-off-by: karishma <[email protected]>
satoshi-ota added a commit to tier4/autoware.universe that referenced this pull request Jun 6, 2024
…#5771)

* refactor(bpp): use pluginlib

Signed-off-by: satoshi-ota <[email protected]>

* refactor(tier4_planning_launch): update launcher

Signed-off-by: satoshi-ota <[email protected]>

* refactor(avoidance): support pluginlib

Signed-off-by: satoshi-ota <[email protected]>

* refactor(lane_change): support pluginlib

Signed-off-by: satoshi-ota <[email protected]>

* refactor(dynamic_avoidance): support pluginlib

Signed-off-by: satoshi-ota <[email protected]>

* refactor(goal_planner): support pluginlib

Signed-off-by: satoshi-ota <[email protected]>

* refactor(side_shift): support pluginlib

Signed-off-by: satoshi-ota <[email protected]>

* refactor(start_planner): support pluginlib

Signed-off-by: satoshi-ota <[email protected]>

* refactor(bpp): move interface

Signed-off-by: satoshi-ota <[email protected]>

* fix(bpp): add const

Signed-off-by: satoshi-ota <[email protected]>

---------

Signed-off-by: satoshi-ota <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:launch Launch files, scripts and initialization tools. (auto-assigned) 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.

2 participants