refactor(emergency_handler, mrm_handler) change MRM operators to classes inside Handler Nodes

[veqcc]

Description

Delete MRM operators and implemented those operators inside Handlers (Emergency Handler, MRM Handler) as classes based on #6539 .

Related links

#6539

Tests performed

Not yet.

Notes for reviewers

Please review especially in terms of:

• namespace structure
• update_rate of handlers
  • Previously, it was 10 Hz in emergency_handler, while 33 Hz in emergency_stop_operator.
• how to pass parameters to each operator
• When to publish emergency commands in emergency_stop operator class
  • only in operate() or both operate() and onTimer() ?
• Are commands gear and hazard_light used only when emergency_stop is enabled?
  • If so, should they included in emergency_stop class, not in handlers?

Interface changes

We cannot access MRM operators directly.
All inputs/outputs of operators are included in the Handler's interfaces.

Effects on system behavior

Handlers do not have to call ROS 2 services anymore to invoke mrm oprations.
It changes to function calls and thus mrm operations can work more quickly in terms of:

• no need to wait next timer callback
• no need to wait ROS 2 scheduler to schedule

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

• I've confirmed the contribution guidelines.
• The PR follows the pull request guidelines.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

• The PR follows the pull request guidelines.
• The PR has been properly tested.
• The PR has been reviewed by the code owners.

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.
• The PR is ready for merge.

After all checkboxes are checked, anyone who has write access can merge the PR.

[isamu-takagi]

namespace structure

Do you mean comfortable_stop_operator in emergency_handler? It looks fine.
FYI: It can be written as namespace emergency_handler::comfortable_stop_operator

update_rate of handlers
Previously, it was 10 Hz in emergency_handler, while 33 Hz in

Since it changes the publishing rate of control commands, please check whether it has any effect on vehicle control. Also, it is better to treat the topic rate and processing rate separately.

emergency_stop_operator.
how to pass parameters to each operator

Where is this about? I think it would be better to use declare_parameter in each operator class, as is the current implementation.

When to publish emergency commands in emergency_stop operator class
only in operate() or both operate() and onTimer() ?

I think it's better to always publish if there are no performance problems.

Are commands gear and hazard_light used only when emergency_stop is enabled?
If so, should they included in emergency_stop class, not in handlers?

Yes. At the meeting the other day, we decided to publish a command for each MRM, so please move it in the operator class. It should be needed for comfortable_stop as well.

[TetsuKawa]

As soon as this node is started up, the process dies.
I don't know where the bug is, but you should look into the code.

[veqcc]

@TetsuKawa
Thank you for your comment.
If the error message is what(): Statically typed parameter 'hoge' must be initialized. or something like this, then adding some parameters to the top-level configuration file (autoware/src/launcher/autoware_launch/autoware_launch/config/system/emergency_handler/emergency_handler.param.yaml) works well.

[TetsuKawa]

@veqcc
I'm sorry. You are right, the higher level parameter file was loaded.

[veqcc]

@isamu-takagi
Thank you for your helpful comments!

namespace structure

Do you mean comfortable_stop_operator in emergency_handler? It looks fine.
FYI: It can be written as namespace emergency_handler::comfortable_stop_operator

Yes I meant it. I have fixed the namespace following it.

[veqcc]

emergency_stop_operator. how to pass parameters to each operator

Where is this about? I think it would be better to use declare_parameter in each operator class, as is the current implementation.

Yes I meant the parameters in configuration files.
Thank you for your review.

[veqcc]

@isamu-takagi

Are commands gear and hazard_light used only when emergency_stop is enabled? If so, should they included in emergency_stop class, not in handlers?

Yes. At the meeting the other day, we decided to publish a command for each MRM, so please move it in the operator class. It should be needed for comfortable_stop as well.

I see. I will move these parameters and functions to operators from handlers.
One question here is, is it neccesary to modify vehicle_cmd_gate to enable these commands even when comfortable_stop?
It seems the vehicle_cmd_gate chooses emergency commands only when emergency_stop is being operated.
If so, I will make another Pull Request for it after this PR is merged.

[isamu-takagi]

One question here is, is it neccesary to modify vehicle_cmd_gate to enable these commands even when comfortable_stop?
It seems the vehicle_cmd_gate chooses emergency commands only when emergency_stop is being operated.
If so, I will make another Pull Request for it after this PR is merged.

@veqcc That's right. So for this PR, it is enough to move the command publishers to the emergency_stop operator class.

[veqcc]

update_rate of handlers
Previously, it was 10 Hz in emergency_handler, while 33 Hz in

Since it changes the publishing rate of control commands, please check whether it has any effect on vehicle control. Also, it is better to treat the topic rate and processing rate separately.

I have changed following your advice. Now emergency_handler works in 10Hz, while emergency_stop_operator works in 30Hz as before.

[veqcc]

When to publish emergency commands in emergency_stop operator class. Only in operate() or both operate() and onTimer() ?

I think it's better to always publish if there are no performance problems.

Both the current implementaion and my modification always publish them.
There are no performance problem in Psim on my computer, though I don't know in full Autoware on actual vehicles.
Until any performance problem happens, I will keep the current one.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity.

[veqcc]

I willl close it.