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

Use pose_broadcaster to publish the TCP pose #1108

Merged
merged 9 commits into from
Nov 11, 2024

Conversation

fmauch
Copy link
Collaborator

@fmauch fmauch commented Sep 19, 2024

This uses the PoseBroadcaster currently written by @RobertWilbrandt. As he wants to contribute that to ros2_controllers, this will stay in a draft state until it is merged upstream.

Universal_Robots_ROS2_Driver.jazzy.repos Outdated Show resolved Hide resolved
Universal_Robots_ROS2_Driver.jazzy.repos Outdated Show resolved Hide resolved
Universal_Robots_ROS2_Driver.rolling.repos Outdated Show resolved Hide resolved
Universal_Robots_ROS2_Driver.rolling.repos Outdated Show resolved Hide resolved
@gavanderhoorn
Copy link
Contributor

High-level question: would it perhaps be better to have the hardware interface offer a quaternion instead of an Euler angle triplet?

That would seem cleaner to me, in the sense of it being more of an N-to-1 mathematical abstraction.

While they aren't necessarily very human friendly, quaternions have definite benefits mathematically, and making it the responsibility of the hardware interface/driver to convert whatever internal orientation representation it uses to this 'intermediary representation' would seem to make sense.

@fmauch
Copy link
Collaborator Author

fmauch commented Sep 26, 2024

High-level question: would it perhaps be better to have the hardware interface offer a quaternion instead of an Euler angle triplet?

That would seem cleaner to me, in the sense of it being more of an N-to-1 mathematical abstraction.

While they aren't necessarily very human friendly, quaternions have definite benefits mathematically, and making it the responsibility of the hardware interface/driver to convert whatever internal orientation representation it uses to this 'intermediary representation' would seem to make sense.

I was discussing that with @RobertWilbrandt, as well and I would agree. But I think that's a question to discuss once the PR on ros2_controllers is there.

@gavanderhoorn
Copy link
Contributor

Oh, just noticed #856 actually does it that way.

@RobertWilbrandt
Copy link
Collaborator

I don't have any strong opinions on this - Going through REP 103 i went through the options and preferred RPY because the values cannot represent any invalid state (while i would expect that a quaternion should be normalized). From an application point of view i agree, so i changed the representation in pose_broadcaster now.

# Conflicts:
#	ur_robot_driver/config/ur_controllers.yaml
#	ur_robot_driver/launch/ur_control.launch.py
#	ur_robot_driver/src/hardware_interface.cpp
This has now been merged to master.
Everything is merged upstream!
@fmauch fmauch requested a review from VinDp October 27, 2024 18:11
@fmauch fmauch marked this pull request as ready for review October 27, 2024 18:11
@fmauch
Copy link
Collaborator Author

fmauch commented Oct 27, 2024

Binary builds fail as the necessary changes in ros2_control have just been merged into master.

Copy link
Collaborator

@VinDp VinDp left a comment

Choose a reason for hiding this comment

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

Looks good to me and works as inteded.

@urfeex urfeex merged commit 660e363 into UniversalRobots:main Nov 11, 2024
8 of 12 checks passed
mergify bot pushed a commit that referenced this pull request Nov 11, 2024
Adds a broadcaster for the robot's TCP pose.

(cherry picked from commit 660e363)
mergify bot pushed a commit that referenced this pull request Nov 11, 2024
Adds a broadcaster for the robot's TCP pose.

(cherry picked from commit 660e363)
urfeex pushed a commit that referenced this pull request Nov 11, 2024
Adds a broadcaster for the robot's TCP pose.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants