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 eigen3_cmake_module #144

Merged
merged 4 commits into from
Aug 9, 2019
Merged

Use eigen3_cmake_module #144

merged 4 commits into from
Aug 9, 2019

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Aug 6, 2019

Requires ros2/eigen3_cmake_module#1
Requires ros2/eigen3_cmake_module#3

This makes tf2 packages use eigen3_cmake_module

  • tf2_eigen
    • Exports Eigen3 dependency instead of export Eigen3 include dir found at build time
  • tf2_sensor_msgs
    • This package appears to build nothing. I added eigen3_cmake_module since it was finding Eigen3 so it's ready when someone finishes porting this package later
  • tf2_kdl
    • This package was exporting Eigen3 without using it. This should be handled by the orocos_kdl package instead.

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz sloretz added the in progress Actively being worked on (Kanban column) label Aug 6, 2019
@sloretz sloretz self-assigned this Aug 6, 2019
@sloretz sloretz added this to Proposed in Dashing Patch Release 3 via automation Aug 6, 2019
@sloretz sloretz added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Aug 8, 2019
@sloretz sloretz moved this from Proposed to In progress in Dashing Patch Release 3 Aug 8, 2019
@sloretz
Copy link
Contributor Author

sloretz commented Aug 8, 2019

CI is green ros2/ros2#755 (comment)

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

I have one nit in here, but I'll approve anyway and let you decide what you want to do.

find_package(Eigen3 REQUIRED)
message(STATUS "Using Eigen3 include dirs: ${EIGEN3_INCLUDE_DIR}")
message(STATUS "Using Eigen3 include dirs: ${Eigen3_INCLUDE_DIRS}")
Copy link
Contributor

Choose a reason for hiding this comment

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

I know you didn't add it, but since you are touching it, I think we should just remove this line. While it is informational, you could say the same about literally any dependency that we use find_package on, so I'm not sure why Eigen is special.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in 8dfffa1

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz sloretz merged commit 3795e18 into ros2 Aug 9, 2019
@delete-merged-branch delete-merged-branch bot deleted the eigen3_cmake_module branch August 9, 2019 18:15
@sloretz sloretz moved this from In progress to Needs Backport in Dashing Patch Release 3 Aug 9, 2019
sloretz added a commit that referenced this pull request Aug 23, 2019
* Make tf2_sensor_msgs use eigen3_cmake_module

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

* Make tf2_eigen use eigen3_cmake_module

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

* Remove unused Eigen3 dependency from tf2_kdl

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

* Remove STATUS about Eigen

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
sloretz added a commit that referenced this pull request Aug 31, 2019
* Make tf2_sensor_msgs use eigen3_cmake_module

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

* Make tf2_eigen use eigen3_cmake_module

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

* Remove unused Eigen3 dependency from tf2_kdl

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

* Remove STATUS about Eigen

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in review Waiting for review (Kanban column)
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants