search

borglab / GTDynamics

Full kinodynamics constraints for arbitrary robot configurations with factor graphs.
BSD 2-Clause "Simplified" License
39 stars 10 forks source link

CMake Packaging #335

Closed varunagrawal closed 2 years ago

varunagrawal commented 2 years ago

This PR adds support for making gtdynamics a cmake package so we can do find_package(gtdynamics) for downstream applications. Fixes #5.

I need this for my own LRSE project and it's been long overdue. To include gtdynamics, all I needed to do was find_package(gtdynamics REQUIRED) and when I print the version as ${gtdynamics_VERSION}, I get the following output:

-- GTSAM include directory:  /usr/local/lib/cmake/GTSAM/../../../include
-- Looking for ignition-math6 -- found version 6.9.2
-- Searching for dependencies of ignition-math6
-- GTSAM Version: 4.2a4
-- GTDynamics Version: 1.0.0

This is a CMake update and no C++ changes are made whatsoever. Moreover, it includes commands for configuring and installing mainly 3 files: gtdynamics-exports.cmake, gtdynamicsConfig.cmake and gtdynamicsConfigVersion.cmake. Thus, there should be no breakage in the normal process of installation from source. Even the CI should be evidence of this since we install from source.

Question: Should we stick to gtdynamics or convert the package name to GTDynamics? The former means we have CMake variables of the form gtdynamics_VERSION which is fine IMO, but we can do GTDynamics_VERSION instead if you'd like. GTSAM does GTSAM_VERSION but it is also a shorter name.

dellaert commented 2 years ago

Let’s stick to gtdynamics. Two questions: 1) how do you know it works? Add evidence in PR comment. 2) add some statement that makes me believe it does not affect anyone negatively Happy to approve after that - let me know by e-mail.