-
Notifications
You must be signed in to change notification settings - Fork 341
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.
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
Deprecate non-stamped twist for tricycle_controller and steering_controllers #1093
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Just a small remark, it will not be possible to send directly a command to a robot using DiffDrive controller from a console.
That was nice to switch easily from turtlesim to a real robot from a pedagogical point of view. But the auto completion in ROS-2 with pub topic is not working super nicely from my experience. If anybody has a nice way to have a simple example for ros2 topic pub /cmd_vel
, it will be nice to document it.
You mean because it is not possible to auto-fill the time-stamp in the header? Or that the syntax of the nested msg is tedious? |
Actually both. (BTW thanks again for the documentation effort this is great !) With the auto completion I never managed to have the nice example you gave in the docs. I do not remember if there is a default acceptable value for the time stamp like 0 for all the fields. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Do we want to completely remove it in Jazzy or leave it with a deprecation warning?
I agree with @olivier-stasse. I believe it should be fine to publish with 0 timestamp as it is usually considered as the command sent with the current timestamp
Yes, I think we will completely remove it. I'm not sure if there will be a rolling release before jazzy, but I thought at least we can backport the deprecation warning to iron.
Is this a feature of ROS 2 CLI or does the controller just have to "ignore" zero timestamp? I'd have to test that |
🙃
I also struggle always there. Sometimes it's faster to ask my AI friend for the syntax than trying around.. |
I think the controller is the one who has to consider the zero timestamps as the recent one. Upon checking the code, this is already handled. In fact in most of the ROS/ROS2 systems it is considered like that ros2_controllers/diff_drive_controller/src/diff_drive_controller.cpp Lines 344 to 351 in 689efe7
|
I think the clunkiness comes from the CLI auto complete offering a one-lined yaml as opposed to the JSON format (dict with curly brackets) |
First step of #989.