fix(mrm_handler): fix bugprone-branch-clone

[kobayu858]

Description

This is a fix based on clang-tidy bugprone-branch-clone error.

/home/emb4/autoware/autoware/src/universe/autoware.universe/system/mrm_handler/src/mrm_handler/mrm_handler_core.cpp:118:30: error: repeated branch in conditional chain [bugprone-branch-clone,-warnings-as-errors]
  if (is_emergency_holding_) {
                             ^
/home/emb4/autoware/autoware/src/universe/autoware.universe/system/mrm_handler/src/mrm_handler/mrm_handler_core.cpp:121:4: note: end of the original
  } else if (isEmergency() && param_.turning_hazard_on.emergency) {
   ^
/home/emb4/autoware/autoware/src/universe/autoware.universe/system/mrm_handler/src/mrm_handler/mrm_handler_core.cpp:121:67: note: clone 1 starts here
  } else if (isEmergency() && param_.turning_hazard_on.emergency) {
                                                                  ^
/home/emb4/autoware/autoware/src/universe/autoware.universe/system/mrm_handler/src/mrm_handler/mrm_handler_core.cpp:188:72: error: repeated branch in conditional chain [bugprone-branch-clone,-warnings-as-errors]
    if (!requestMrmBehavior(mrm_state_.behavior, RequestType::CANCEL)) {
                                                                       ^
/home/emb4/autoware/autoware/src/universe/autoware.universe/system/mrm_handler/src/mrm_handler/mrm_handler_core.cpp:190:6: note: end of the original
    } else if (requestMrmBehavior(current_mrm_behavior, RequestType::CALL)) {
     ^
/home/emb4/autoware/autoware/src/universe/autoware.universe/system/mrm_handler/src/mrm_handler/mrm_handler_core.cpp:192:12: note: clone 1 starts here
    } else {
           ^

Related links

Parent Issue:

• Link

How was this PR tested?

Notes for reviewers

None.

Interface changes

None.

Effects on system behavior

None.

[github-actions]

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

• You've checked our contribution guidelines.
• Your PR follows our pull request guidelines.
• All required CI checks pass before marking the PR ready for review.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.

Project coverage is 29.74%. Comparing base (e5243e4) to head (7b59903).
Report is 165 commits behind head on main.

Files with missing lines	Patch %	Lines
...m/mrm_handler/src/mrm_handler/mrm_handler_core.cpp	0.00%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9729      +/-   ##
==========================================
- Coverage   29.74%   29.74%   -0.01%     
==========================================
  Files        1443     1443              
  Lines      108681   108688       +7     
  Branches    42663    42663              
==========================================
- Hits        32330    32328       -2     
- Misses      73172    73181       +9     
  Partials     3179     3179              

Flag	Coverage Δ		*Carryforward flag
differential	0.00% <0.00%> (?)
total	29.74% <ø> (+<0.01%)	⬆️	Carriedforward from f06639e

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

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

[veqcc]

@mkuri @TetsuKawa
The original implementation looks wierd: Is the following correct?

if (param_.turning_hazard_on.emergency) {
  if (is_emergency_holding_ || isEmergency()) {
    msg.command = HazardLightsCommand::ENABLE;
  } else {
    msg.command = HazardLightsCommand::NO_COMMAND;
  }
} else {
  msg.command = HazardLightsCommand::NO_COMMAND;
}

[TetsuKawa]

@mkuri
I think that kambe san’ s code is better.
Is there a problem？

[veqcc]

In additiion to that, isEmergerncy() is implemented as follows:
https://github.com/autowarefoundation/autoware.universe/blob/d86c46a35882e75051a0abcba55853da78c1d21f/system/mrm_handler/src/mrm_handler/mrm_handler_core.cpp#L540C1-L544C2

bool MrmHandler::isEmergency() const
{
  return !operation_mode_availability_->autonomous || is_emergency_holding_ ||
         is_operation_mode_availability_timeout;
}

So, if (is_emergency_holding_ || isEmergency()) is still redundant and just if (isEmergency()) seems enough.

[mkuri]

@veqcc
I agree. I think the following is fine too.

if (param_.turning_hazard_on.emergency) {
  if (isEmergency()) {
    msg.command = HazardLightsCommand::ENABLE;
  } else {
    msg.command = HazardLightsCommand::NO_COMMAND;
  }
} else {
  msg.command = HazardLightsCommand::NO_COMMAND;
}