Unified the unit of `cmd_vel_timeout`

[Narukara]

Describe the bug

Several controllers have a cmd_vel_timeout parameter. But their units are inconsistent. Specifically, I found these:

parameter	unit	type
tricycle_controller_parameter.yaml	milliseconds	int
omni_wheel_drive_controller_parameters.yaml	seconds	double
diff_drive_controller_parameter.yaml	seconds	double

This is a bit confusing. Maybe we should unify their units.

Additionally, two controllers are using reference_timeout. This appears to be the same as cmd_vel_timeout. Maybe the parameter names should be unified as well?

reference_timeout: steering_controllers_library.yaml, mecanum_drive_controller.yaml

Expected behavior
Consistent units and types.

Environment:

• Version: master branch

[christophfroehlich]

Thanks for the overview. Fyi: Tricycle controller will be removed/integrated in the steering controllers_library, but I haven't finished the PR yet. #1695

You are right about the naming. reference_timeout would be more generic, but I'm not sure at the moment if this logic is also used if the controllers are in chained mode.

[github-actions]

This issue is being labeled as stale because it has been open 45 days with no activity. It will be automatically closed after another 45 days without follow-ups.

[vedh1234]

I would like to take this up.
I will investigate if the timeout logic is active during chained mode and will decide accordingly on the naming convention.
Can you assign it to me ?

[ielson]

Hey @vedh1234 are you still working on this issue? If not, I´d like to take that. @christophfroehlich if he´s not working on it anymore, could you assign this issue to me?

[vedh1234]

Hi @ielson
you can take this up. I am currently working on another issue.

[ielson]

@christophfroehlich I'm already working on this issue, could you please assign it to me?

[bmagyar]

Thanks for volunteering!

[ielson]

So I have checked here and from my research, these parameters are used for the same kind of behavior:. The main difference todayis API naming:

• cmd_vel_timeout is used by controllers subscribing to ~/cmd_vel
• reference_timeout is used by controllers subscribing to ~/reference
  The types in them are the same(geometry_msgs/msg/TwisStamped)

None of them are used when on chained mode, so following @christophfroehlich answer I would call them all reference_timeout

You are right about the naming. reference_timeout would be more generic, but I'm not sure at the moment if this logic is also used if the controllers are in chained mode.

Proposed plan:

• Standardize timeout units and type across all affected controllers to seconds as double
• Unify the parameter name across controllers to be reference_timeout
• Update parameter definition files, code paths, tests, example configs, and user documentation together

But more than that, can't we unify the /cmd_vel and the /reference topics at the controllers in the future?

[Amronos]

I personally think the parameter should be called cmd_vel_timeout. I don't have a good reasoning for this but it just feels better. Similarly the topic should be ~/cmd_vel.

Also, cmd_vel_timeout should work in chained mode. The reason for this is that cmd_vel_timeout can be set to 0.0 if one wants to disable it.

Another point: This common functionality should then probably be moved to control_toolbox.