ros2_controllers icon indicating copy to clipboard operation
ros2_controllers copied to clipboard

diff_drive_controller generates wrong odom and robot base frame names

Open lukicdarkoo opened this issue 2 years ago • 5 comments

Since the last sync (https://discourse.ros.org/t/new-packages-for-ros-2-humble-hawksbill-2022-12-16/28806) there are two issues:

  1. Default frame name of the robot base frame is odom. This used to be base_link and it was a meaningful name. I don't think we should change defaults within the single ROS distro. I guess the problem is here: https://github.com/ros-controls/ros2_controllers/blob/42f6c1470508ca2480b954eead58a87e7e827434/diff_drive_controller/src/diff_drive_controller_parameter.yaml#L49
  2. Namespaces are passed as prefixes to the odom and robot base frame names. I think I read somewhere that adding prefixes to frame names in multi-robot systems is a bad practice. Anyways, like in the first issue, the behavior shouldn't change within a single ROS distro. Even adding a leading slash to the frame names didn't make prefixes go away. My temporary solution is to use the snapshot version:
    wget -O /tmp/diff_drive_controller.deb http://snapshots.ros.org/humble/2022-11-23/ubuntu/pool/main/r/ros-humble-diff-drive-controller/ros-humble-diff-drive-controller_2.12.0-1jammy.20221108.202153_amd64.deb && \
    sudo apt install -y /tmp/diff_drive_controller.deb && \
    rm -f /tmp/diff_drive_controller.deb
    

It looks like the changes are made on purpose in https://github.com/ros-controls/ros2_controllers/pull/461. I am not sure whether it is a good decision, especially considering that the change breaks the previous behavior.

lukicdarkoo avatar Dec 18 '22 13:12 lukicdarkoo

@lukicdarkoo you are right. I don't know how I missed this "/" issue with frames. I think we should use "_" as separator between prefix and frame name and use it only if there is controller namespace provided.

@delihus maybe you would like to open another PR 😉

destogl avatar Jan 11 '23 18:01 destogl

@lukicdarkoo sorry, this change slipped through into humble unfortunately…

destogl avatar Jan 11 '23 18:01 destogl

@destogl I don't think the namespace should affect frame names. So far I haven't seen any package doing it in ROS 2.

Instead, there should be a parameter, like frame_prefix. Or, simply not allowing a prefix is also fine. A user can add the prefix when defining frame names anyways.

For example in Nav2, a namespace adds a prefix to every topic, including tf and tf_static. It makes transforms isolated so there is no need to add prefixes to frame names.

In short, I believe #461 should be reverted.

lukicdarkoo avatar Jan 12 '23 08:01 lukicdarkoo

Hi, I also agree that #461 should be reverted. I think that having a prefix option could be a way of doing it for those who only have 1 /tf topic and want to add differentiation. In my situation I use remapping to make a new /tf topic for each namespace as described in this issue on Geometry2. This is also how they do it on the Nav2 project, so having nothing would be best for those who use that method.

bobbleballs avatar Feb 24 '23 18:02 bobbleballs

hi, anyone following this problem? I guess I'm struggling with same issue .

I applied a namsapce on the node. But /tf publishes frame id with namespace as a prefix.

All I need is just remove the namespace on frame id for TF.

hyunseok-yang avatar Nov 09 '23 07:11 hyunseok-yang