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

WIP: Make this a CMake-Package #53

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft

Conversation

skasperski
Copy link

Hi,

in order to use this package outside of ROS, I started transition towards a plain CMake-Package. Usually this works fine, as Catkin can handle CMake-Packages seemlessly, at least with ROS1. I assume it should work with ROS2 in a similar fashion, but needs testing. I few changes will also be required for dependent packages, I only did it for hdl_localization so far. I changed the package-name to "pclomp", because that is what was used in the headers.

Please let me know if you are generally interested in following up with this approach.

@skasperski skasperski marked this pull request as draft November 7, 2022 11:05
@koide3
Copy link
Owner

koide3 commented Nov 8, 2022

Thanks for your proposal. I would like to avoid directly merging these modifications because we need to test compatibility with several other packages, but building in non-ROS (or ROS2) environments is a very commonly demanded feature, and thus I would like to mention your repo in README so that users can easily find the information on non-ROS situations.

@skasperski
Copy link
Author

skasperski commented Nov 8, 2022

I absolutely agree, in its current state it should not be merged. I wanted to ask first, before putting more effort into this. Maybe you could test it in a ROS2 context and put the result here.

For reference: Here is the adapted version of hdl_localization. The required changes there are small: skasperski/hdl_localization@258d9ff

@koide3
Copy link
Owner

koide3 commented Nov 17, 2022

Sorry for the late reply. If it doesn't change the project name and namespaces, I would be happy to merge it.
Because there are some third parties using this package, I would like to keep the current interface.

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