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

[ros2-beta] Missing TFs in "/tf" topic when tf_publish_rate > 0 #2522

Closed
Ilia-Kovalev opened this issue Oct 28, 2022 · 10 comments
Closed

[ros2-beta] Missing TFs in "/tf" topic when tf_publish_rate > 0 #2522

Ilia-Kovalev opened this issue Oct 28, 2022 · 10 comments

Comments

@Ilia-Kovalev
Copy link

Ilia-Kovalev commented Oct 28, 2022

When parameter tf_publish_rate > 0, the node publishes all TFs from_static_tf_msgs.
This line clears static transforms saved in the RS node before adding transforms for each sensor. After all sensors initialization, _static_tf_msgs has only TFs for the last sensor.
So, dynamic transform publisher doesn't publish all TFs as static transform publisher does, but only TFs of the last initialized sensor.

It looks like a mistake, because _static_tf_msgs, as its name suggests, should have all static TFs inside it for the dynamic transform publisher.

@MartyG-RealSense
Copy link
Collaborator

MartyG-RealSense commented Oct 28, 2022

Hi @Tristis116 In Intel's ros2_beta wrapper documentation, it states for tf_publish_rate that "double, positive values mean dynamic transform publication with specified rate, all other values mean static transform publication. Default is 0".

The documentation also states that if the /static_tf topic is disabled then transformations will be published if tf_publish_rate is set to 1.0 in the launch file or on the command-line using -p tf_publish_rate:=1.0 and the topics /tf and /extrinsic/<stream>_to_<stream> are activated and read.

https://github.com/IntelRealSense/realsense-ros/tree/ros2-beta#limitations

@Ilia-Kovalev
Copy link
Author

Hi @MartyG-RealSense
I read about limitations. My point is: /tf doesn't have all transformations - only for a sensor initialized the last. And the documentation doesn't say anything about which transformations should be published and which shouldn't.
According to the code, it looks like it should duplicate /tf_static. But sensor initialization clears the list of transformations when it initializes every sensor.

@MartyG-RealSense
Copy link
Collaborator

I will highlight your query to the RealSense ROS team. Thanks very much for your patience!

@MartyG-RealSense
Copy link
Collaborator

Hi @Tristis116 Just an update to let you know that I have been discussing your case with the RealSense ROS team. At this time it is not known why you are experiencing the reported issue with /tf, unfortunately.

@Ilia-Kovalev
Copy link
Author

Hi @MartyG-RealSense
When I start d455 camera with ros2 launch realsense2_camera rs_launch.py -s, then /tf_static has transforms for color and depth:

transforms:
- header:
    stamp:
      sec: 1668070582
      nanosec: 112172855
    frame_id: camera_link
  child_frame_id: camera_depth_frame
  transform:
    translation:
      x: 0.0
      y: -0.0
      z: -0.0
    rotation:
      x: 0.0
      y: 0.0
      z: 0.0
      w: 1.0
- header:
    stamp:
      sec: 1668070582
      nanosec: 112172855
    frame_id: camera_depth_frame
  child_frame_id: camera_depth_optical_frame
  transform:
    translation:
      x: 0.0
      y: -0.0
      z: -0.0
    rotation:
      x: -0.5
      y: 0.4999999999999999
      z: -0.5
      w: 0.5000000000000001
- header:
    stamp:
      sec: 1668070582
      nanosec: 314827794
    frame_id: camera_link
  child_frame_id: camera_color_frame
  transform:
    translation:
      x: 0.00043846675544045866
      y: 0.059190280735492706
      z: 0.0001858784380601719
    rotation:
      x: 0.00023960287217050793
      y: 0.00014341560017783195
      z: -0.0007031101267784834
      w: 0.9999997019767761
- header:
    stamp:
      sec: 1668070582
      nanosec: 314827794
    frame_id: camera_color_frame
  child_frame_id: camera_color_optical_frame
  transform:
    translation:
      x: 0.0
      y: -0.0
      z: -0.0
    rotation:
      x: -0.5
      y: 0.4999999999999999
      z: -0.5
      w: 0.5000000000000001
---

/tf has only transformation for color:

transforms:
- header:
    stamp:
      sec: 1668070614
      nanosec: 989107270
    frame_id: camera_link
  child_frame_id: camera_color_frame
  transform:
    translation:
      x: 0.00043846675544045866
      y: 0.059190280735492706
      z: 0.0001858784380601719
    rotation:
      x: 0.00023960287217050793
      y: 0.00014341560017783195
      z: -0.0007031101267784834
      w: 0.9999997019767761
- header:
    stamp:
      sec: 1668070614
      nanosec: 989107270
    frame_id: camera_color_frame
  child_frame_id: camera_color_optical_frame
  transform:
    translation:
      x: 0.0
      y: -0.0
      z: -0.0
    rotation:
      x: -0.5
      y: 0.4999999999999999
      z: -0.5
      w: 0.5000000000000001
---

This behavior doesn't correspond to the tf_publish_rate description:

    'tf_publish_rate':
        Rate of publishing static_tf
        (default: '0.0') 

If I disable color stream, then /tf has only transforms for depth. /tf has only the last transform of /tf_static. And the last transform of /tf_static depends on enabled streams and output of rs2::device::query_sensors.
So there is an actual problem for users: ros tf functionality and rviz visualization may not work because of missing transforms. And a potential problem after disabling/enabling specific streams, switching to another camera model, or even firmware update, because published transform may change and break code that relied on previously published transform.

@MartyG-RealSense
Copy link
Collaborator

I have passed your latest information above to the RealSense ROS team. Thanks very much for the update.

@MartyG-RealSense
Copy link
Collaborator

MartyG-RealSense commented Nov 21, 2022

Hi @Tristis116 The issue is still under discussion with the RealSense ROS team, so I have added an Enhancement label to this case to signify that it should be kept open. Thanks very much for your patience!

@SamerKhshiboun
Copy link
Collaborator

SamerKhshiboun commented Jun 1, 2023

@MartyG-RealSense, PR #2676 was merged.
I think we can close this ticket if @Tristis116 confirms that.

@MartyG-RealSense
Copy link
Collaborator

Thanks so much, @SamerKhshiboun :)

@MartyG-RealSense
Copy link
Collaborator

Thanks very much @Tristis116 for the update!

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

No branches or pull requests

3 participants