Skip to content

Commit

Permalink
fix(behavior_path_planner): sort keep last modules considering priority
Browse files Browse the repository at this point in the history
Signed-off-by: kosuke55 <[email protected]>
  • Loading branch information
kosuke55 committed Jan 26, 2024
1 parent ccd852b commit 1a0c61b
Showing 1 changed file with 46 additions and 21 deletions.
67 changes: 46 additions & 21 deletions planning/behavior_path_planner/src/planner_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -630,32 +630,38 @@ BehaviorModuleOutput PlannerManager::runApprovedModules(const std::shared_ptr<Pl
return output;
}

const auto move_to_end = [](auto & modules, const auto & cond) {
auto itr = modules.begin();
while (itr != modules.end()) {
const auto satisfied_exit_cond =
std::all_of(itr, modules.end(), [&cond](const auto & m) { return cond(m); });

if (satisfied_exit_cond) {
return;
}
// move modules whose keep last flag is true to end of the approved_module_ptrs_.
// if there are multiple keep last modules, sort by priority
{
const auto move_to_end = [](auto & modules, const auto & module_to_move) {
auto itr = std::find(modules.begin(), modules.end(), module_to_move);

Check warning on line 637 in planning/behavior_path_planner/src/planner_manager.cpp

View check run for this annotation

Codecov / codecov/patch

planning/behavior_path_planner/src/planner_manager.cpp#L636-L637

Added lines #L636 - L637 were not covered by tests

if (cond(*itr)) {
if (itr != modules.end()) {
auto tmp = std::move(*itr);
itr = modules.erase(itr);
modules.insert(modules.end(), std::move(tmp));
} else {
itr++;
modules.erase(itr);
modules.push_back(std::move(tmp));
}
}
};
};

Check warning on line 644 in planning/behavior_path_planner/src/planner_manager.cpp

View check run for this annotation

Codecov / codecov/patch

planning/behavior_path_planner/src/planner_manager.cpp#L644

Added line #L644 was not covered by tests

// move modules whose keep last flag is true to end of the approved_module_ptrs_.
{
const auto keep_last_module_cond = [this](const auto & m) {
return getManager(m)->isKeepLast();
const auto get_sorted_keep_last_modules = [this](const auto & modules) {

Check warning on line 646 in planning/behavior_path_planner/src/planner_manager.cpp

View check run for this annotation

Codecov / codecov/patch

planning/behavior_path_planner/src/planner_manager.cpp#L646

Added line #L646 was not covered by tests
std::vector<SceneModulePtr> keep_last_modules;

std::copy_if(
modules.begin(), modules.end(), std::back_inserter(keep_last_modules),
[this](const auto & m) { return getManager(m)->isKeepLast(); });

Check warning on line 651 in planning/behavior_path_planner/src/planner_manager.cpp

View check run for this annotation

Codecov / codecov/patch

planning/behavior_path_planner/src/planner_manager.cpp#L651

Added line #L651 was not covered by tests

// sort by priority (low -> high)
std::sort(
keep_last_modules.begin(), keep_last_modules.end(), [this](const auto & a, const auto & b) {

Check warning on line 655 in planning/behavior_path_planner/src/planner_manager.cpp

View check run for this annotation

Codecov / codecov/patch

planning/behavior_path_planner/src/planner_manager.cpp#L654-L655

Added lines #L654 - L655 were not covered by tests
return getManager(a)->getPriority() < getManager(b)->getPriority();
});

return keep_last_modules;

Check warning on line 659 in planning/behavior_path_planner/src/planner_manager.cpp

View check run for this annotation

Codecov / codecov/patch

planning/behavior_path_planner/src/planner_manager.cpp#L659

Added line #L659 was not covered by tests
};
move_to_end(approved_module_ptrs_, keep_last_module_cond);

for (const auto & module : get_sorted_keep_last_modules(approved_module_ptrs_)) {
move_to_end(approved_module_ptrs_, module);
}

Check warning on line 664 in planning/behavior_path_planner/src/planner_manager.cpp

View check run for this annotation

Codecov / codecov/patch

planning/behavior_path_planner/src/planner_manager.cpp#L664

Added line #L664 was not covered by tests
}

// lock approved modules besides last one
Expand Down Expand Up @@ -768,6 +774,25 @@ BehaviorModuleOutput PlannerManager::runApprovedModules(const std::shared_ptr<Pl
* remove success module immediately. if lane change module has succeeded, update root lanelet.
*/
{
const auto move_to_end = [](auto & modules, const auto & cond) {

Check warning on line 777 in planning/behavior_path_planner/src/planner_manager.cpp

View check run for this annotation

Codecov / codecov/patch

planning/behavior_path_planner/src/planner_manager.cpp#L777

Added line #L777 was not covered by tests
auto itr = modules.begin();
while (itr != modules.end()) {
const auto satisfied_exit_cond =
std::all_of(itr, modules.end(), [&cond](const auto & m) { return cond(m); });

Check warning on line 781 in planning/behavior_path_planner/src/planner_manager.cpp

View check run for this annotation

Codecov / codecov/patch

planning/behavior_path_planner/src/planner_manager.cpp#L781

Added line #L781 was not covered by tests

if (satisfied_exit_cond) {
return;
}

if (cond(*itr)) {
auto tmp = std::move(*itr);
itr = modules.erase(itr);
modules.insert(modules.end(), std::move(tmp));
} else {
itr++;
}
}
};

Check warning on line 795 in planning/behavior_path_planner/src/planner_manager.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ Getting worse: Complex Method

PlannerManager::runApprovedModules increases in cyclomatic complexity from 13 to 15, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
const auto success_module_cond = [](const auto & m) {
return m->getCurrentStatus() == ModuleStatus::SUCCESS;
};
Expand Down

0 comments on commit 1a0c61b

Please sign in to comment.