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

Build system fixes: use include/library paths from pkg-config, and strip leading 'v' from librtlsdr version #138

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

agrif
Copy link

@agrif agrif commented Apr 28, 2022

This PR fixes two issues with the CMake build scripts I encountered on my Mac:

  1. CMake now uses the include and library paths reported by pkg-config, which lets it build successfully even when (e.g.) librtlsdr is installed in a nonstandard place.
  2. The bias tee version check for librtlsdr now looks for a leading 'v' on the version, and strips it if necessary. Some versions of librtlsdr include this 'v' (as when building from source), and some do not (as in the Debian package). If the version includes a 'v' without being stripped first, the bias tee check fails and I cannot use a bias tee even though my version of librtlsdr supports it. This change supports both version formats.

I'm happy to make any changes necessary to help get this merged!

I also have a few EMWIN handler fixes. I'll clean those up and probably open a separate PR once they're ready.

Copy link
Owner

@pietern pietern left a comment

Choose a reason for hiding this comment

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

Thanks for submitting the PR!

Are you able to build on macOS with these changes? If so, what do you use for the dependencies (e.g. homebrew, ports)?

@@ -22,10 +24,16 @@ if(NOT RTLSDR_FOUND)
message(WARNING "Unable to find librtlsdr")
else()
add_library(rtlsdr_source rtlsdr_source.cc)
# some rtlsdr versions begin with an extraneous 'v', remove it
Copy link
Owner

Choose a reason for hiding this comment

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

Can you specify for which versions this is the case in the comment here?

The next person looking at these build files will be thankful :-)

@agrif
Copy link
Author

agrif commented May 26, 2022

Yes! I'm building this successfully on MacOS using librtlsdr and opencv from macports. I've only used goesrecv but it seems to work fine.

It looks like target_link_directories is too new for the targeted cmake version. There's a couple of solutions:

I'll try absolute paths first, but I'll have to test it out on my pi and elsewhere to make sure it doesn't break any builds.

agrif added 2 commits May 27, 2022 19:47
If the libraries discovered by pkg-config are installed in a
nonstandard location, these are required to include their
headers and link with their libraries.
Some versions of librtlsdr (including versions <= 0.5.4 built directly
from source) have a leading 'v' on their versions as reported by
pkg-config. This causes the version check for bias tees to yield
incorrect results, preventing the use of a bias tee even if the
underlying librtlsdr supports it. Removing the 'v' avoids this.
@agrif
Copy link
Author

agrif commented May 27, 2022

I've updated the pkg-config sections to use absolute library paths from ${FOO_LINK_LIBRARIES}, as recommended by the CMake documentation. This should make the changes compatible with cmake < 3.13. I've tested these changes on my mac, x64 debian, and raspbian. Unfortunately this means linking with all of OpenCV instead of selectively, but I don't think that hurts much.

The pkgconfig file for airspy puts the libairspy directory in the include path, so I've changed the source to include airspy.h directly. This was also tested and works on all three systems I have access to.

I've also noted which versions of librtlsdr have the pesky leading 'v'.

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