Skip to content

protonect: added install target and use pkg-config to find dependancis #90

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

Closed
wants to merge 1 commit into from

Conversation

goldhoorn
Copy link
Contributor

The install targets for protonect was missing, now the lib and all headers
got installes.

Also make it configurable wether the internal-find algorithm should
be used, or if it is assumed that the depending libraries (libusb/glfw/glew)
are installed in the system. This needs a fix of the include of libusb itself
in the headers.

All changes should be backward compatible.

The install targets for protonect was missing, now the lib and all headers
got installes.

Also make it configurable wether the internal-find algorithm should
be used, or if it is assumed that the depending libraries (libusb/glfw/glew)
are installed in the system. This needs a fix of the include of libusb itself
in the headers.

All changes should be backward compatible.
@goldhoorn
Copy link
Contributor Author

pong

@christiankerl
Copy link
Contributor

the install feature has been included with an older PR

regarding the dependency search. it is probably sensible to use distribution/system packages, but there need to be fallbacks if the Find... scripts don't work (e.g. for windows). Another problem is that we need the patched libusb version on Linux...

@@ -27,7 +27,7 @@
#ifndef TRANSFER_POOL_H_
#define TRANSFER_POOL_H_

#include <libusb.h>
#include <libusb-1.0/libusb.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason why you have to include libusb-1.0 - what happend if the library is named otherwise eventually :) ?

My path to libusb.h is "C:\libraries\libusbx\libusb", so that wouldn't work on my install.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On 18.11.2014 09:29, Lars wrote:

In examples/protonect/include/libfreenect2/usb/transfer_pool.h:

@@ -27,7 +27,7 @@
#ifndef TRANSFER_POOL_H_
#define TRANSFER_POOL_H_

-#include <libusb.h>
+#include <libusb-1.0/libusb.h>

Any reason why you have to include libusb-1.0 - what happend if the
library is named otherwise eventually :) ?

My path to libusb.h is "C:\libraries\libusbx\libusb", so that
wouldn't work on my install.

In the past the libusb-1.0 was fixed given in the CMakeLists.
The package-config returns the patch to /usr/include and not to
/usr/include/libusb-1.0/
(which is the corretct way).

For compatibility i could extend the include-path in the CMakeLists
instead inside the headers, but this is not the way as headers should be
handeled or installed.


Reply to this email directly or view it on GitHub
https://github.com/OpenKinect/libfreenect2/pull/90/files#r20491420.

@goldhoorn
Copy link
Contributor Author

On 18.11.2014 09:28, Christian Kerl wrote:

the install feature has been included with an older PR

Then this overlapped, sorry

regarding the dependency search. it is probably sensible to use
distribution/system packages, but there need to be fallbacks if the
Find... scripts don't work (e.g. for windows). Another problem is that
we need the patched libusb version on Linux...

The patched libusb will (hopefully) go in the future, still i would
prefer to install it manually and globally in my system.
The generic way to find packages (by pkg-config or find-scripts) is
better and more portable.


Reply to this email directly or view it on GitHub
#90 (comment).

@goldhoorn
Copy link
Contributor Author

will continue and combine the work in #68

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.

3 participants