chili-epfl / ros_markers

ROS wrapper for the EPFL's chilitags library
BSD 3-Clause "New" or "Revised" License
7 stars 18 forks source link

Use a catkin wrapper instead of PkgConfig #7

Open nicolov opened 8 years ago

nicolov commented 8 years ago

Hi, thank you very much for open-sourcing the code, there's a lot to learn from.

In ROS, a common pattern to handle external pure-cmake projects is to create a catkin wrapper that compiles the shared libraries and exports the includes. By doing this, any other ROS package can just REQUIRED COMPONENTS the wrapper and the library gets linked in automatically.

IMHO, this is neater than having people install chilitags system-wide using CMake, as it plays nicer with the rest of the ROS ecosystem (.deb packaging, cleaning, multiple workspaces,..)

I've created such a wrapper here and pushed the (little) changes needed to use it in my fork.

Is this something you would be interested in merging?

severin-lemaignan commented 8 years ago

Is this something you would be interested in merging?

Definitely! Thanks a lot!

I'm away until September, so if I forget to merge it at that point, please ping the thread!


[http://www.plymouth.ac.uk/images/email_footer.gif]http://www.plymouth.ac.uk/worldclass

This email and any files with it are confidential and intended solely for the use of the recipient to whom it is addressed. If you are not the intended recipient then copying, distribution or other use of the information contained is strictly prohibited and you should not rely on it. If you have received this email in error please let the sender know immediately and delete it from your system(s). Internet emails are not necessarily secure. While we take every care, Plymouth University accepts no responsibility for viruses and it is your responsibility to scan emails and their attachments. Plymouth University does not accept responsibility for any changes made after it was sent. Nothing in this email or its attachments constitutes an order for goods or services unless accompanied by an official order form.

skohlbr commented 8 years ago

Ping :) Just ran also into difficulties building, looks like the proposed package could make that more convenient.

nicolov commented 8 years ago

Install these first:

Then, you can use my fork of this package

Let me know how that goes, I've only tested on ROS Kinetic.

severin-lemaignan commented 8 years ago

@nicolov Alright, I had a look + I've forked chilitags_catkin. I'm happy to merge a pull request to support the wrapper, but I'm less keen on forcing people to use it (as it means one more bit of package to install).

Would it be possible to bundle chilitags_catkin directly with ros_markers so that it is almost transparent for the users?

nicolov commented 8 years ago

Thanks again, I've used the detector for some drone precision landing application, and it worked great!

We can have two folders/catkin packages in this repo, one with the wrapper, and one with the current code. Ros_markers would depend on the wrapper, so they would both get built.

If you're ok with that, I can prepare a PR.

On Friday, 9 September 2016, Séverin Lemaignan notifications@github.com wrote:

@nicolov https://github.com/nicolov Alright, I had a look + I've forked chilitags_catkin https://github.com/chili-epfl/chilitags_catkin. I'm happy to merge a pull request to support the wrapper, but I'm less keen on forcing people to use it (as it means one more bit of package to install).

Would it be possible to bundle chilitags_catkin directly with ros_markers so that it is almost transparent for the users?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/chili-epfl/ros_markers/issues/7#issuecomment-245932528, or mute the thread https://github.com/notifications/unsubscribe-auth/AKO4B1Vy_X70TKMpqZjE6zs5LL_PsqeAks5qoW9XgaJpZM4JfbWT .

severin-lemaignan commented 8 years ago

Sounds good! Looking forward the PR.