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 a catkin wrapper instead of PkgConfig #7

Open
nicolov opened this issue Aug 8, 2016 · 6 comments
Open

Use a catkin wrapper instead of PkgConfig #7

nicolov opened this issue Aug 8, 2016 · 6 comments

Comments

@nicolov
Copy link
Contributor

nicolov commented Aug 8, 2016

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
Copy link
Member

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
Copy link

skohlbr commented Sep 9, 2016

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

@nicolov
Copy link
Contributor Author

nicolov commented Sep 9, 2016

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
Copy link
Member

@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
Copy link
Contributor Author

nicolov commented Sep 12, 2016

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
#7 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AKO4B1Vy_X70TKMpqZjE6zs5LL_PsqeAks5qoW9XgaJpZM4JfbWT
.

@severin-lemaignan
Copy link
Member

Sounds good! Looking forward the PR.

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

No branches or pull requests

3 participants