Skip to content
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

diff_drive_controller generates wrong odom and robot base frame names #482

Closed
lukicdarkoo opened this issue Dec 18, 2022 · 6 comments · Fixed by #495 or #498
Closed

diff_drive_controller generates wrong odom and robot base frame names #482

lukicdarkoo opened this issue Dec 18, 2022 · 6 comments · Fixed by #495 or #498

Comments

@lukicdarkoo
Copy link

lukicdarkoo commented Dec 18, 2022

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:
  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 #461. I am not sure whether it is a good decision, especially considering that the change breaks the previous behavior.

@destogl
Copy link
Member

destogl commented Jan 11, 2023

@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
Copy link
Member

destogl commented Jan 11, 2023

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

@lukicdarkoo
Copy link
Author

lukicdarkoo commented Jan 12, 2023

@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.

@bobbleballs
Copy link
Contributor

bobbleballs commented Feb 24, 2023

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.

@hyunseok-yang
Copy link

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.

@christophfroehlich
Copy link
Contributor

I think this was solved with #533

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment