Skip to content

conversions for Eigen::Isometry3d in ROS 2 - re-iteration#113

Closed
vmayoral wants to merge 16 commits intoros2:ros2from
AcutronicRobotics:isometry3d-new
Closed

conversions for Eigen::Isometry3d in ROS 2 - re-iteration#113
vmayoral wants to merge 16 commits intoros2:ros2from
AcutronicRobotics:isometry3d-new

Conversation

@vmayoral
Copy link
Member

@tfoote tfoote added the in review Waiting for review (Kanban column) label May 16, 2019
@dirk-thomas dirk-thomas requested a review from tfoote May 16, 2019 20:49
@vmayoral
Copy link
Member Author

So I went ahead and manually cherry-picked all relevant changes from #93 in here.

The diff now is actually rather silly so it seems that most changes are already in the ros2 branch for some reason.

@vmayoral
Copy link
Member Author

vmayoral commented May 16, 2019

All right, I can see most of these changes got already in https://github.com/ros2/geometry2/pull/90/commits. Maybe @dirk-thomas you can squash all of them into a single small commit with "minor adjustments to tf2_eigen" or should we simply discard these changes?

@clalancette
Copy link
Contributor

So I went ahead and manually cherry-picked all relevant changes from #93 in here.

Cool, thanks.

The diff now is actually rather silly so it seems that most changes are already in the ros2 branch for some reason.

Right. What's left here is actually formatting changes, and the removal of one method. Do we need this PR at all anymore?

return msg;
}

/** \brief Convert a Eigen::Vector3d type to a geometry_msgs::msg::Vector3.
Copy link
Member Author

Choose a reason for hiding this comment

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

This shouldn't be removed. Needed by collision_detection submodule of moveit2.

@vmayoral
Copy link
Member Author

Closing here, it modifies things that shouldn't be modified. Let me know @vmayoral or @dirk-thomas if you miss anything but I'd say we're good now for moveit2.

@vmayoral vmayoral closed this May 16, 2019
@tfoote tfoote removed the in review Waiting for review (Kanban column) label May 16, 2019
@vmayoral
Copy link
Member Author

@ahcorde can you advise if 6dccebe is still needed please?

@ahcorde
Copy link
Contributor

ahcorde commented May 16, 2019

Yes, otherwise

The negative w values are completely valid quaternions. I'd rather not just normalize it here. This breaks the expectation that fromMsg(toMsg(...)) will return the same value.

@vmayoral
Copy link
Member Author

Ok, can you make a PR against ros2 then with only that last commit of yours or should I make it myself?

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.

5 participants