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

Transform force-torque into tcp frame. #237

Merged
merged 2 commits into from Nov 10, 2021

Conversation

livanov93
Copy link
Contributor

@livanov93 livanov93 commented Nov 5, 2021

Closes #235
Sets the rotation of the wrench message into the tool center point.

Copy link
Contributor

@destogl destogl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. Please test it on hardware and then it can be merged.

@gavanderhoorn
Copy link
Contributor

Isn't this basically a copy from the ROS 1 driver?

@destogl
Copy link
Contributor

destogl commented Nov 5, 2021

Isn't this basically a copy from the ROS 1 driver?

Yes. Shouldn't it be?

@schornakj can you please test this on HW?

@schornakj
Copy link

@destogl I tested this on my robot and the wrench measurements are now correctly published relative to the tool0 frame that was specified when configuring the force/torque controller.

I'm testing this using:

ros2 launch ur_bringup ur_control.launch.py headless_mode:=True use_fake_hardware:=False fake_sensor_commands:=False robot_ip:="[ip_address]" ur_type:="ur5e" launch_rviz:=True

@gavanderhoorn
Copy link
Contributor

gavanderhoorn commented Nov 6, 2021

Isn't this basically a copy from the ROS 1 driver?

Yes. Shouldn't it be?

there was no mention of that anywhere. Not in the commit, nor in the PR description.

Seeing as it was already part of the ROS 1 driver, and the ROS 2 version here is a continuation of that development, but this functionality somehow got removed, it would be a good idea to add a note where this came from to the commit. As-is, it's almost like this functionality is new, while in reality it was (likely inadvertently) removed.

Among other things, noting where it came from and that it's reinstated functionality would link this back to the ROS 1 driver, connecting it to the history there as well, which may help in the future with maintenance tasks.

@destogl
Copy link
Contributor

destogl commented Nov 9, 2021

@livanov93 can you please add link to the ROS1 driver when squash-merging into commit?

Also, it would make sense to add comment in the code too. Then you can merge this since @schornakj confirmed the functionality.

@livanov93
Copy link
Contributor Author

livanov93 commented Nov 10, 2021

@destogl @gavanderhoorn I have added original source of the code in the comments. On merge, I will leave another link to the ROS1 driver.

@livanov93 livanov93 merged commit 7c1e9b1 into UniversalRobots:main Nov 10, 2021
livanov93 added a commit to livanov93/Universal_Robots_ROS2_Driver that referenced this pull request Nov 15, 2021
livanov93 added a commit to livanov93/Universal_Robots_ROS2_Driver that referenced this pull request Nov 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Force/torque sensor broadcaster publishes wrench with incorrect orientation
4 participants