stereolabs / zed-ros2-wrapper

ROS 2 wrapper for the ZED SDK
https://www.stereolabs.com/docs/ros2/
Apache License 2.0
134 stars 139 forks source link

IMU-Camera TF Broadcasting #180

Closed anath93 closed 8 months ago

anath93 commented 8 months ago

Preliminary Checks

Description

IMU Camera TF broadcasting even if publish_tf is disabled, I have posted in community page of stereolabs.

Link: Click here

Steps to Reproduce

  1. Cleared the workspace and tried rebuilding the colcon workspace.
  2. Set publish_tf and imu_tf to false but still getting published
  3. When my Robot State publisher publishes, it causes problem as Zed is trying to publish and I am too with my robot urdf.
  4. But once disabled on my robot urdf, and enabled on zed side, it works fine, but still when disabled, it publishes the tf. ...

Expected Result

When publish_tf is disabled, IMU tf should not be displayed, otherwise I will not be able to add this to my robot URDF.

Actual Result

.

ZED Camera model

ZED2i

Environment

Ubuntu 20.04, Foxy, Nvidia Orin NX 64 GB

Anything else?

No response

Myzhar commented 8 months ago

IMU TF and localization TF are separate. You can disable IMU TF by setting this to false

publish_imu_tf: true # [usually overwritten by launch file] enable/disable the IMU TF broadcasting it's also a launch parameter

anath93 commented 8 months ago

I did that, if you refer to my screenshot in community link page, it still publishes it .

https://community.stereolabs.com/t/zed2i-imu-transform-to-add-to-robot-urdf/3009/12

anath93 commented 8 months ago

@Myzhar Any update on this thread ?

Myzhar commented 8 months ago

@anath93 the behavior currently reflects the value of the publish_tf launch parameter, thus you cannot disable the broadcasting of the IMU TF if odometry TF is active. I'm merging a fix that will add a new launch parameter for this. Example: $ ros2 launch zed_wrapper zed2i.launch.py publish_imu_tf:=False

anath93 commented 8 months ago

@Myzhar, I will try out the fix once out and close this issue once I have verified. Thank you !

Myzhar commented 8 months ago

Fix merged: https://github.com/stereolabs/zed-ros2-wrapper/blob/master/zed_wrapper/launch/include/zed_camera.launch.py#L205-L208

anath93 commented 8 months ago

@Myzhar So when I do publish_imu_tf: false, it does work but when I publish_tf to false as my odometry tf is coming from different source, it does not work. image

Myzhar commented 8 months ago

Sorry, this is not clear. Can you explain better what you are doing? What launch file are you using? What parameters are you using?

anath93 commented 8 months ago

Yes when I launch the zed_wrapper as below: ros2 launch zed_wrapper zed2i.launch.py publish_tf:=False (because I do not want base link --> odom be published, the imu_tf still exists. But as you suggested above ros2 launch zed_wrapper zed2i.launch.py publish_imu_tf:=False, then imu_tf does not get broadcasted.

Please let me know if you need more info like SDK version (which is same as posted on community page) or anything else . https://community.stereolabs.com/t/zed2i-imu-transform-to-add-to-robot-urdf/3009/12

anath93 commented 8 months ago

@Myzhar, did you had a chance to look at this ?

Myzhar commented 8 months ago

@anath93 yes, I could replicate this. It's a wrong behavior: when publish_tf is set to False the related publish_map_tf and publish_imu_tf are not processed and they use the default value that is true. I'm going to fix this wrong behavior and merge the fix before the end of the day

anath93 commented 8 months ago

@Myzhar Thank you, please let me know once its pushed !

Myzhar commented 8 months ago

@anath93 can you test the fix?

anath93 commented 8 months ago

@anath93 can you test the fix?

Yes ! I will get back to you shortly.

anath93 commented 8 months ago

@anath93 can you test the fix?

ros2 launch zed_wrapper zed2i.launch.py publish_tf:=False So now the odom and map frames are not getting published, which is good but still the zed2i_imu_link gets published, even if add argument of publish_imu_tf to false but same result.

Myzhar commented 8 months ago

Be aware that you must wait for the TF to time out when it's first published. You can use rqt and the Runtime Monitor plugin to be sure that TF are enabled/disabled

Disabled: image

IMU Enabled: image

All Enabled: image

Myzhar commented 8 months ago

This command disables the broadcasting of all the TFs: $ ros2 launch zed_wrapper zed2i.launch.py publish_tf:=False publish_imu_tf:=False

anath93 commented 8 months ago

This command disables the broadcasting of all the TFs: $ ros2 launch zed_wrapper zed2i.launch.py publish_tf:=False publish_imu_tf:=False

Yes this one worked ! Thank you.

One last question: Should'nt this should be set to 0's when publish_tf is set to False ? image

Myzhar commented 8 months ago

The TF broadcasting is disabled, but the Wrapper internally performs the processing for Positional Tracking to publish odom and pose related topics. You can turn off the module if you want, but the SDK requires Positional Tracking also for other type of processings (e.g. Depth Stabilization).

anath93 commented 8 months ago

Got it, that makes sense. Thank you again. I am going to close this topic !

anath93 commented 8 months ago

@Myzhar I think there could be another potential issue, when the depth_mode is turned off, the IMU frame still gets published as I am just using the stereo mode of this camera.

Myzhar commented 8 months ago

That's correct. Depth and IMU are not related, so you must explicitly disable it if you do not need it.

anath93 commented 8 months ago

I did disable imu_tf and publish_tf to false and depth to false too but the IMU link showed up ros2 launch zed_wrapper zed2i.launch.py publish_tf:=False publish_imu_tf:=False

anath93 commented 8 months ago

@Myzhar I manually added to components file and it worked, all the desired topics are up. But if you feel this could cause any unknown error in future, please let me know.

if (!mPublishImuTF || mDepthMode == sl::DEPTH_MODE::NONE) { return; }

Myzhar commented 8 months ago

The problem is related to the fact that when Depth is disabled the Positional Tracking is automatically disabled and the parameters are not processed, so mPublishImuTF keeps the default values that is true. The fix is to move the parameter publish_imu_tf from pos_tracking to sensors because it's more correct.

anath93 commented 8 months ago

I agree with you on that !

Myzhar commented 8 months ago

I thank you for catching this issue, it's really important to fix all the possible configurations of the node. I'm going to fix this tomorrow (evening now here in Europe), I will trigger you as soon as it's ready

anath93 commented 8 months ago

I thank you for catching this issue, it's really important to fix all the possible configurations of the node. I'm going to fix this tomorrow (evening now here in Europe), I will trigger you as soon as it's ready

Thank you Myzhar, everything looks good including params file which makes more sense now with segregation of sensor part. I work in USA and its nice to meet you.