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.

Sign up for GitHub

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

Jump to bottom

Additional tf2 Type Conversions #292

Merged
merged 1 commit into from
Apr 26, 2018
Merged

Additional tf2 Type Conversions #292

merged 1 commit into from
Apr 26, 2018

Conversation

IanTheEngineer
Copy link
Contributor

@IanTheEngineer IanTheEngineer commented Apr 9, 2018 •
edited
Loading

  • adds non-stamped Eigen to Transform function
  • converts Eigen Matrix Vectors to and from geometry_msgs::Twist
  • adds to/from message for geometry_msgs::Pose and KDL::Frame

This PR is not ready for merge, but I figured I'd open it to get the discussion started. MoveIt! utilizes many different data type conversions in the old tf stack from eigen_conversions, kdl_conversions, and tf_conversions, and I'm attempting to convert everything to tf2.

Not all of the conversions from tf were present in geometry2, so I added those that I needed during the MoveIt migration. Caveats: I understand that the Twist type has representation issues, and that conversions are meant to be to/from Stamped types, but it seemed these conversions could still be useful.

That said, I am open to feedback on how to work with geometry2, so please let me know if I am off base in these additions.

@IanTheEngineer
Adds additional conversions for tf2, KDL, Eigen
2982de0 
- adds non-stamped Eigen to Transform function
- converts Eigen Matrix Vectors to and from geometry_msgs::Twist
- adds to/from message for geometry_msgs::Pose and KDL::Frame
@IanTheEngineer IanTheEngineer mentioned this pull request Apr 9, 2018
Merged
8 tasks
IanTheEngineer
IanTheEngineer commented Apr 9, 2018
View reviewed changes
tf2_eigen/include/tf2_eigen/tf2_eigen.h
return msg;
}

/** \brief Convert a Pose message transform type to a Eigen Affine3d.
Copy link
Contributor Author

@IanTheEngineer IanTheEngineer Apr 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Update comment to read Twist message, and Eigen Affine3d should read Eigen 6x1 Matrix

@IanTheEngineer IanTheEngineer mentioned this pull request Apr 26, 2018
Closed
@dirk-thomas
Copy link
Member

@tfoote Can you please take a look at this since it is necessary for the migration of MoveIt to tf2. Thanks.

tfoote
tfoote approved these changes Apr 26, 2018
View reviewed changes
Copy link
Member

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

The changes look good. The PR failure looks unrelated. I'll try retriggering it.

@tfoote
Copy link
Member

tfoote commented Apr 26, 2018

@ros-pull-request-builder retest this please.

@tfoote tfoote merged commit 1a972d7 into ros:melodic-devel Apr 26, 2018
IanTheEngineer added a commit to IanTheEngineer/moveit that referenced this pull request May 17, 2018
@IanTheEngineer
tf2 migration, resolves issue moveit#745
f4ebdc6 
- All type conversions now depend on geometry2 ROS packages, rather
  than geometry (see ros/geometry2#292 and
  ros/geometry2#294 for details of the new
  conversions)
- Removes all boost::shared_ptr<tf::TransformListener> from the API,
  and replaced them with std::shared_ptr<tf2_ros::Buffer>'s
- Piped tf2_ros::Buffer's everywhere where tf::TransformListeners were
  used
- Utilizes the new tf2 API in the tf::Transformer library to access
  the internal tf2::Buffer object used by RViz
  (see ros/geometry#163 for details of the
  new API)
- Removes the prepending of forward slashes ('/') for Transforms frames
  as this is deprecated in tf2
@rhaschke
Copy link
Contributor

@tfoote @dirk-thomas Can you please consider these changes for cherry-picking to Kinetic?
MoveIt requires these changes for its melodic branch. However, we decided that this branch should also compile on Kinetic.

@rhaschke rhaschke mentioned this pull request May 18, 2018
Closed
@IanTheEngineer
Copy link
Contributor Author

IanTheEngineer commented May 18, 2018 •
edited
Loading

As I mentioned in moveit/moveit#902 (comment), the MoveIt maintainers need to discuss the decision to compile on both Melodic & Kinetic. There are several Melodic API changes that the MoveIt tf2 migration depends on:

geometry2: #292 & #294
ros/geometry#163
ros-visualization/rviz#1236

While I don't have a preference on what is backported (or not), I want to make sure we are all on the same page before making a decision.

@rhaschke
Copy link
Contributor

I think all these changes target enabling of tf2. I think this is a valid goal not only for Melodic but also for Kinetic. Thus I ask you to consider backporting all of them. Instead of deprecating old functionality, it would suffice to add new functionality. I'm looking forward to your decision.

rhaschke pushed a commit to moveit/moveit that referenced this pull request May 18, 2018
@IanTheEngineer @rhaschke
MoveIt! tf2 migration (#830)
736251d 
migration from tf to tf2 API, resolves issue #745

- All type conversions now depend on geometry2 ROS packages, rather than geometry
  (see ros/geometry2#292 and
  ros/geometry2#294 for details of the new conversions)
- Removes all boost::shared_ptr<tf::TransformListener> from the API,
  and replaced them with std::shared_ptr<tf2_ros::Buffer>'s
- Utilize new tf2 API in the tf::Transformer library to access the internal tf2::Buffer of RViz
  (see ros/geometry#163 for details of the new API)
- Removes prepending of forward slashes ('/') for transforms frames as this is deprecated in tf2
- Replaced deprecated tf2 _getLatestCommonTime
@v4hn
Copy link
Contributor

v4hn commented May 23, 2018

Ping @tfoote @dirk-thomas

Can you please consider these changes [Ian pull-requested in your packages] for cherry-picking to Kinetic?
MoveIt requires these changes for its melodic branch. However, we decided that this branch should also compile on Kinetic.

@tfoote
Copy link
Member

tfoote commented May 24, 2018

Since this is just additions or modifications to inline functions we can plan to backport this to kinetic.

@IanTheEngineer IanTheEngineer mentioned this pull request May 29, 2018
Closed
tfoote pushed a commit that referenced this pull request Jul 10, 2018
@IanTheEngineer @tfoote
Adds additional conversions for tf2, KDL, Eigen (#292)
0ea7580 
- adds non-stamped Eigen to Transform function
- converts Eigen Matrix Vectors to and from geometry_msgs::Twist
- adds to/from message for geometry_msgs::Pose and KDL::Frame
@tfoote tfoote mentioned this pull request Jul 10, 2018
Closed
tfoote pushed a commit that referenced this pull request Jul 10, 2018
@IanTheEngineer @tfoote
Adds additional conversions for tf2, KDL, Eigen (#292)
26f1694 
- adds non-stamped Eigen to Transform function
- converts Eigen Matrix Vectors to and from geometry_msgs::Twist
- adds to/from message for geometry_msgs::Pose and KDL::Frame
tfoote pushed a commit that referenced this pull request Jul 10, 2018
@IanTheEngineer @tfoote
Adds additional conversions for tf2, KDL, Eigen (#292)
15d0901 
- adds non-stamped Eigen to Transform function
- converts Eigen Matrix Vectors to and from geometry_msgs::Twist
- adds to/from message for geometry_msgs::Pose and KDL::Frame
@davetcoleman davetcoleman mentioned this pull request Mar 15, 2019
Closed
@jdlangs jdlangs mentioned this pull request Aug 26, 2019
Merged
jdlangs pushed a commit to jdlangs/geometry2 that referenced this pull request Aug 26, 2019
@IanTheEngineer @jdlangs
Adds additional conversions for tf2, KDL, Eigen
cae5fc0 
Originally PR ros2#292 at ros/geometry2#292

- adds non-stamped Eigen to Transform function
- converts Eigen Matrix Vectors to and from geometry_msgs::Twist
- adds to/from message for geometry_msgs::Pose and KDL::Frame
clalancette pushed a commit to ros2/geometry2 that referenced this pull request Sep 5, 2019
@IanTheEngineer @clalancette
Adds additional conversions for tf2, KDL, Eigen
70ca817 
Originally PR #292 at ros/geometry2#292

- adds non-stamped Eigen to Transform function
- converts Eigen Matrix Vectors to and from geometry_msgs::Twist
- adds to/from message for geometry_msgs::Pose and KDL::Frame
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tfoote tfoote tfoote approved these changes

@davetcoleman davetcoleman davetcoleman approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@IanTheEngineer @dirk-thomas @tfoote @rhaschke @v4hn @davetcoleman