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.

Sign up for GitHub

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

fix(lane_change): fix state transition in lane change module #7436

Merged
merged 5 commits into from
Jun 13, 2024
Merged

fix(lane_change): fix state transition in lane change module #7436

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ca13764
fix(lane_change): fix state transition in lane change module
rej55 Jun 11, 2024
94dc6e9
fix(lane_change): remove overridden run() function
rej55 Jun 11, 2024
e2021bb
fix(lane_change): fix RTC safety status
rej55 Jun 12, 2024
2e1e070
Merge branch 'main' into fix/state_transition_lane_change
rej55 Jun 13, 2024
37884ce
Merge branch 'main' into fix/state_transition_lane_change
rej55 Jun 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions ...r_path_lane_change_module/include/autoware_behavior_path_lane_change_module/interface.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@ class LaneChangeInterface : public SceneModuleInterface
LaneChangeInterface & operator=(LaneChangeInterface &&) = delete;
~LaneChangeInterface() override = default;

void processOnEntry() override;

void processOnExit() override;

bool isExecutionRequested() const override;
Expand Down Expand Up @@ -93,25 +91,6 @@ class LaneChangeInterface : public SceneModuleInterface

MarkerArray getModuleVirtualWall() override;

// TODO(someone): remove this, and use base class function
[[deprecated]] BehaviorModuleOutput run() override
{
updateData();

if (!isWaitingApproval()) {
return plan();
}

// module is waiting approval. Check it.
if (isActivated()) {
RCLCPP_DEBUG(getLogger(), "Was waiting approval, and now approved. Do plan().");
return plan();
} else {
RCLCPP_DEBUG(getLogger(), "keep waiting approval... Do planCandidate().");
return planWaitingApproval();
}
}

protected:
std::shared_ptr<LaneChangeParameters> parameters_;

Expand Down
26 changes: 12 additions & 14 deletions planning/autoware_behavior_path_lane_change_module/src/interface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,6 @@
logger_ = utils::lane_change::getLogger(module_type_->getModuleTypeStr());
}

void LaneChangeInterface::processOnEntry()
{
waitApproval();
}

void LaneChangeInterface::processOnExit()
{
module_type_->resetParameters();
Expand Down Expand Up @@ -79,7 +74,7 @@
module_type_->setPreviousModuleOutput(getPreviousModuleOutput());
module_type_->updateSpecialData();

if (isWaitingApproval()) {
if (isWaitingApproval() || module_type_->isAbortState()) {
module_type_->updateLaneChangeStatus();
}

Expand Down Expand Up @@ -116,14 +111,12 @@

updateSteeringFactorPtr(output);
if (module_type_->isAbortState()) {
waitApproval();
const auto candidate = planCandidate();
path_candidate_ = std::make_shared<PathWithLaneId>(candidate.path_candidate);
updateRTCStatus(
std::numeric_limits<double>::lowest(), std::numeric_limits<double>::lowest(), true,
State::ABORTING);
} else {
clearWaitingApproval();
const auto path =
assignToCandidate(module_type_->getLaneChangePath(), module_type_->getEgoPosition());
updateRTCStatus(
Expand Down Expand Up @@ -153,7 +146,7 @@

if (!module_type_->isValidPath()) {
updateRTCStatus(
std::numeric_limits<double>::lowest(), std::numeric_limits<double>::lowest(), true,
std::numeric_limits<double>::lowest(), std::numeric_limits<double>::lowest(), false,
State::FAILED);
path_candidate_ = std::make_shared<PathWithLaneId>();
return out;
Expand Down Expand Up @@ -209,11 +202,6 @@
}
}

if (module_type_->isAbortState() && module_type_->hasFinishedAbort()) {
log_debug_throttled("Abort process has completed.");
return true;
}

if (module_type_->hasFinishedLaneChange()) {
module_type_->resetParameters();
log_debug_throttled("Lane change process has completed.");
Expand Down Expand Up @@ -243,6 +231,16 @@
return false;
}

if (!module_type_->isValidPath()) {
log_debug_throttled("Transit to failure state due not to find valid path");
return true;
}

if (module_type_->isAbortState() && module_type_->hasFinishedAbort()) {
log_debug_throttled("Abort process has completed.");
return true;
}

Check warning on line 243 in planning/autoware_behavior_path_lane_change_module/src/interface.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ Getting worse: Complex Method 

LaneChangeInterface::canTransitFailureState increases in cyclomatic complexity from 15 to 18, 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.
if (module_type_->isCancelEnabled() && module_type_->isEgoOnPreparePhase()) {
if (module_type_->isStoppedAtRedTrafficLight()) {
log_debug_throttled("Stopping at traffic light while in prepare phase. Cancel lane change");
Expand Down
Loading