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

added cmake support #23

Merged
merged 1 commit into from
Jan 25, 2021
Merged

added cmake support #23

merged 1 commit into from
Jan 25, 2021

Conversation

berndpfrommer
Copy link
Contributor

added support for building under cmake

Copy link
Contributor

@foehnx foehnx left a comment

Choose a reason for hiding this comment

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

Thanks a lot, this is nice.
I quickly tested it and seems to work with CMake (3.16.3), gcc (9.3.0) and nvcc (11.2.67).

It would be nice if you could add quick building/installing instructions to the README, and how it can be used/included this way in other projects.

Have you run a runtime comparison to check if the compiler flags differ and/or have impact with respect to the makefile-built version?

cmake/vilibConfig.cmake.in Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
cmake/vilibConfig.cmake.in Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@berndpfrommer
Copy link
Contributor Author

Thanks for reviewing.
I have not done any runtime performance testing, nor verified that the compiler flags are identical. I added the cmake support to use vilib from within a ROS2 wrapper package, and was very happy with the performance I got there, so I stopped at that.

Traveling right now with a laptop that has no nvidia card. Will do the testing and make the changes you requested when returning in about 2 weeks.

@foehnx
Copy link
Contributor

foehnx commented Jan 6, 2021

awesome, thank you.

If your ROS2 project is open source, feel free to mention it in the README and link to it, if you like.

@berndpfrommer
Copy link
Contributor Author

berndpfrommer commented Jan 21, 2021

So far I had only used the cmake package in my ROS2 package and it worked fine there. When creating a stand-alone package I discovered some transitive dependencies were not propagated correctly.
This has now been fixed. Will squash the commits and push again.

Here is a performance comparison between original and cmake library tests
vilib_orig.txt
vilib_cmake.txt

@berndpfrommer
Copy link
Contributor Author

I tested once more on Ubuntu 18.04 (it compiles, but not run tested) and 20.04 (compiles and run tested).
A section with instructions on how to use has been added to the README file.
Please have a look at it again.

Copy link
Contributor

@foehnx foehnx 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!
Thanks a lot for your contribution!

@foehnx foehnx merged commit bd5ff71 into uzh-rpg:master Jan 25, 2021
@foehnx foehnx mentioned this pull request Jan 25, 2021
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.

2 participants