-
Notifications
You must be signed in to change notification settings - Fork 236
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
Add support for BladeRF 2.0 Micro. #37
base: master
Are you sure you want to change the base?
Conversation
--bladerf-biastee switch to enable the Micro's bias Tee on RX ports. Did not test that BladeRF 1 was still working, but it should be, if anyone has a BladeRF 1 to test with, please report.
…mmand line argument (--gain -10)
Does this build on stretch? The bladerf_frequency API changes have been a problem there in the past. |
Yes, it builds on Stretch, but with libbladerf2 - that one is not in stretch by default, but available on the Debian package repository. I suggest to deactivate compiling support for BladeRF by default on the rules file for Debian |
There is a strong requirement that we have bladeRF support enabled by default on both jessie and stretch, as FlightAware has many remotely deployed bladeRFs where the host runs jessie/stretch. Building and distributing updated bladerf packages for stretch is a possibility (we already do that for jessie), but I'd much rather just have any changes be version-aware and handle building against older bladerf out of the box. |
Unless I am mistaken, I'm afraid libbladerf1 only supports the older BladeRF boards, is that correct? So if we want to support all models, libbladerf2 is really the only option, which I agree is a pain... |
Have the build look at the headers to work out what bladerf library is installed (there are some versions defines, I think?) and build accordingly. (I mean preprocessor directives in the source) |
That should be possible - the API has changed a lot between the versions though, so it won't be super pretty :) |
The master does not build at all because of the frequency datatype mismatch. I've quickly hacked it here but this PR seems to be much more sophisticated approach so I am not even going to raise my own. Is there any reason why this cannot be merged straight away? |
That change breaks builds on stretch, which is the primary supported version. |
This is the OS version where I was building from scratch (everything including bladeRF):
And this is the system where I have tried to build it using the install script from jprochazka/adsb-receiver:
I believe it is a best practice to use the data type exposed by the library instead of hardcoding the currently used underlying type. Changing The only reason I am commenting here is because it looks like this PR is superset of my change. If it does not work as is, do you want me to raise another PR just for that one small change? |
So you're using a custom build of libbladerf, not the OS-provided version?
It breaks builds when the OS-provided version of libbladerf does not provide bladerf_frequency at all. You need to make it conditional on the libbladerf version. |
Current dev branch builds with no changes for me on xenial, using the xenial-supplied libbladerf (0.2016.01~rc1-3) |
OK, I see. There are two precompiled packages, one for 1.x and another for 2.x. |
What is source of the second precompiled package? I don't see a libbladeRF2 in anything other than Debian The change looks OK except you probably need to narrow down the API version (the type change dates back to at least https://github.com/Nuand/bladeRF/blob/a0d498e7232c6e856e1fbb9cf1a037a7bddd37c9/host/libraries/libbladeRF/include/libbladeRF.h which has an api version of |
OK, I have misunderstood -- they say the frequency internal data type has changed since 2.x.x but the abstract type has first appeared at 1.9.x. So the current code works but is not clean. Let me change it. The 2.x version of the library is taken from the PPA repositories published by Nuand: https://launchpad.net/~bladerf -- which is Ubuntu only, unfortunately. Does not mean the package cannot be installed but the repos are not compatible. Do you have your own CI/CD or you want me to keep the GitLab CI integration bit included with the PR? |
So just git-blamed the header and it looks like the type has appeared in 1.9.2 for the first time. But according to tags, 1.9.2 was not released and since the type is missing in 1.9.1 and is defined in 2.0.0, I would say 1.9.2 was just pre-2.0.0 changes. Are we ok to leave the conditional for >=2.0.0? |
I committed a minimal polyfill for building on both bladerf1 & bladerf2 in e419719 |
For the original PR, I'd lean towards a similar polyfill-like approach, i.e. assume that we're on libbladeRF2 and provide implementations of the missing functions if we're actually on the older API. Then at least we only need the one conditional block not ifdefs scattered throughout the code. |
Yup I was thinking of polyfill for exactly the same reason (not having ifdefs all over) but came to conclusion that for just one occurence, it would be more confusing than useful from readability perspective. |
Now that I've simplified the SDR interface, I'm leaning a bit more towards just having this as a separate SDR implementation (i.e. |
dump1090 also accepts a new --bladerf-biastee switch to enable the Micro's bias Tee on RX ports.
Did not test that BladeRF 1 was still working, but it should be, if anyone
has a BladeRF 1 to test with, please report.