gqrx-sdr / gqrx

Software defined radio receiver powered by GNU Radio and Qt.
http://gqrx.dk
GNU General Public License v3.0
3.12k stars 546 forks source link

Funcube Dongle Pro not working on OS X 10.11.4 #358

Closed csete closed 8 years ago

csete commented 8 years ago

Looks like something goes wrong with in gr-fcd even with a fix for issue #357

geozsj commented 8 years ago

Seems to be working well here.

George W9ZSJ

PS I just got an SDRPlay unit and I am impressed. However, for the Mac it only provides API for CubicSDR, another nice project. I'd much rather be using GQRX.

dl9sau commented 8 years ago

My FDC on OSX stopped working ~1,5 years ago: segfault of gqrx. Had no problems with an rtl-sdr device.

Looks an index out of bounds error during parsing the "Device String" (fcd,type=1,device='FUNcube Dongle V1_0').

I'm not sure if it's a gqrx bug or a bug in libgnuradio-fcd. Hoped other people have the same problem, but after waiting over various gqrx- and gnuradio-versions, I decided to show the problem today.

I testet with current gqrx and gnuradio, as well with gqrx-devel and gnuradio-devel. My package source is macports (2.3.4; port selfupdate initiated today, before I started debuging).

Here's the crash trace: 2016-04-20--gqrx-fdc-crash-osx.txt

csete commented 8 years ago

@dl9sau Thanks for the info.

@geozsj Can you give us more info about your setup, in particular, are you using the latest version of OS X?

a1k0n commented 8 years ago

Looks to me like a bug in gnuradio gr-fcd; it's trying to get the "Transport" property from each HID device with the appropriate vendor/product ID, OR any device with vendor/product ID of 0. That goes fine, but then it tries to read characters 0 through 32 of a 3-character string into a 32-byte buffer via CFStringGetBytes: https://github.com/gnuradio/gnuradio/blob/master/gr-fcd/lib/hid/hidmac.c#L297

The fix looks to me like to change range.length to call CFStringGetLength instead of assuming a length of 32.

Anyway, this is just drive-by debugging. :) If I had to guess what changed, some new HID device has shown up with a 0/0 vendor/product id.

dl9sau commented 8 years ago

Thank you Andy for your hint and your good eye on this. In fact, it is not a gqrx issue.

From the documentation of CFStringGetBytes(): https://developer.apple.com/library/ios/documentation/CoreFoundation/Reference/CFStringRef/#//apple_ref/c/func/CFStringGetBytes

    range
    The range of characters in theString to process. The specified range must not exceed the length of the string.

=> range.length is set to the len of buf we got by the caller of our function, instead of the length of the string we got from IOHIDDeviceGetProperty(). range.length has to be the length of that source string, not that of the destination buffer.

If we look at the funcubedongle-pro-plus fork at https://github.com/dl1ksv/gr-fcdproplus/blob/master/lib/hid/hidmac.c we see that dl1ksv's hidmac.c differs from that in the the gnuradio-fcd tree. If we look in the header of the file, there's the reference to the original authors (https://github.com/signal11/hidapi/blob/master/mac/hid.c). Btw, it has fixed only 4,5 years ago (https://github.com/signal11/hidapi/commit/f4e138ac7a4d063fe1d87ad21cf68dadcce77b88#diff-5cbb2d1918d13a205b0b45fa24166f42).

It's a classical case where of fork-and-forget.

This fixes the problem for me (but you may better use the original hidmac.c code mentioned above).

-- hidmac.c.orig        2016-04-14 23:04:50.000000000 +0200
+++ hidmac.c.new        2016-04-24 12:56:46.000000000 +0200
@@ -270,7 +270,7 @@
        if (str) {
                CFRange range;
                range.location = 0;
-               range.length = len;
+               range.length = CFStringGetLength(str);
                CFIndex used_buf_len;
                CFStringGetBytes(str,
                        range,
@@ -297,7 +297,7 @@
        if (str) {
                CFRange range;
                range.location = 0;
-               range.length = len;
+               range.length = CFStringGetLength(str);
                CFIndex used_buf_len;
                CFStringGetBytes(str,
                        range,

For a pretty-beauty fix, I'll suggest to the gr-fcd maintainers to update their old fork.

Hint for everyone who is interested but not so familiar in compiling a macports package:

  port fetch gnuradio-devel
  port extract gnuradio-devel

  cd /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_science_gnuradio
  cp /tmp/hidmac.c.new ./gnuradio-devel/work/gnuradio-bdf85171b8a35004cdbf634f48ff696787b5fbde/gr-fcd/lib/hid/hidmac.c

  port build
  port uninstall -f gnuradio-devel
  port -k -s install gnuradio-devel

Then start gqrx and have fun.

vy 73,

csete commented 8 years ago

Thanks for finding the issue @a1k0n and @dl9sau

Thomas, I think the quickest way to fix this is to submit a pull request to https://github.com/gnuradio/gnuradio here on github with the updated hidapi files (update all files while at it). Let me know if you do it, otherwise I will do it myself when I get to it.

dl9sau commented 8 years ago

Hello Alexandru,

Am 24.04.2016 um 15:29 schrieb Alexandru Csete notifications@github.com:

Thanks for finding the issue @a1k0n and @dl9sau

Thomas, I think the quickest way to fix this is to submit a pull request to gnuradio/gnuradio here on github with the updated hidapi files (update all files while at it). Let me know if you do it, otherwise I will do it myself when I get to it.

If you are in touch with gnuradio-developer, I'd prefer it if you could help.

They forked core from the original authors (https://github.com/signal11/hidapi/blob/master/mac/hid.c), made a few cosmetic changes, and never touched their fork again. I'm not sure whom to contact at gnuradio, and it may also be worth they'd look which other issues the signal11 team has fixed over the years. But then, larger testing may be required so that nothing breaks (as well as a review, if gnuradio team has made fixes that should not get lost). => I'd feel better if the gnuradio team would look at the issue and decides, for either merging the entire current hid code, or only the problematic functions in hidmac.c.

vy 73,

csete commented 8 years ago

I'm not particularly in touch with gnuradio developers but I am familiar with their workflow since I used to contribute, so I will do that, no problem.

However, I think you misunderstand the situation and perhaps I should clarify.

I am the original author of gr-fcd. In the beginning, it was a separate block like gr-fcdproplus is, but in time we agreed to include it in gnuradio. Back then there was no hidapi library and certainly none that was included in the distributions, so the only way to use it was to include the source in your own package.

This is by definition not a fork, rather a snapshot. From the FCD point of view it was feature complete so there was no reason to update it, unless of course it becomes broken or bugs are discovered, which is the case now.

dl9sau commented 8 years ago

However, I think you misunderstand the situation and perhaps I should clarify.

I did misunderstand because I was searching where the code may have been come from, found hidapi project (with the code history where the problem has been patched), and their hid.c header starts with

"/*** HIDAPI - Multi-Platform library for communication with HID devices. Alan Ott Signal 11 Software 2010-07-03 Copyright 2010, All Rights Reserved. "

After I had read this, I guessed the code came from them.

You say the code comes from you => Their copyright info is misleading?

I do not like extend this as a copyright discussion and mention this just for completeness.

Unfortunately, your private e-mail address is hidden => this discussion is public readable in github, which I dislike, because I pointed fingers to others and I was wrong. I hope I could delete that part from the issue thread afterwards.

vy 73,

csete commented 8 years ago

I see my attempts to clarify have only caused a lot more confusion, so let me try again :)

I am the original author of the gr-fcd module. The module uses hidapi from Alan Ott / Signal 11 Software, which is why you see his copyright in those files.

The rest of the gr-fcd module is copyright FSF, which is a prerequisite for contributing to gnuradio. I have signed written agreement to reassign copyright when I agreed to include gr-fcd in gnuradio.

So there are no problems anywhere wrt. license and copyright.

The only reason I mentioned that I am the original author of gr-fcd module was to clarify that I am familiar with the code and there is no reason to start looking for somebody else who can make the change and submit a patch.

So, all is fine, we just need to fix the technical issues :)

g0oan commented 8 years ago

Hi Guys,

I have tried a couple of time over the last few months to report this bug to the GNURadio developers using http://gnuradio.org/redmine/projects/gnuradio/wiki/FAQ#How-do-I-report-a-bug but the site Register system doesn't appear to be working correctly.

No matter what I've tried I can't seem to get a confirmation email allowing me to finish registration and post the bug.

Can anyone help or am I simply missing something really obvious?

Thanks in advance for any help.

Sean.

csete commented 8 years ago

Hi Sean,

Sorry for not doing anything about this issue for a long time. I have been very busy and didn't work much on gqrx anyway.

I expect to make a new gqrx release in a few weeks. When that happens it will be time to create a new bundle for Mac OS and I will patch gnuradio in the bundle. If the patch works I will submit it to the gnuradio project.

You are of course welcome to file a bug report in advance. It will not interfere with what I'm doing. I don't know about registration on the bug tracker; I would ask on their mailing list.

Alex

g0oan commented 8 years ago

Hi Alex,

Thank you so much for the feedback and indeed all of the effort in getting this working in the first place.

I did try to following Thomas's instructions, given above, regarding the patch this morning but failed too get it working :( I'm not a programmer so have no doubt that I did something very silly.

More than happy to wait for the next updated.

Thanks again for all of your time.

Kind regards,

Sean.

csete commented 8 years ago

Good news, I have now tried updating the HID API in GNU Radio / gr-fcd and the Funcube Dongle works again. I will include this fix in the 2.6 release in a few weeks and also send a patch upstream.

g0oan commented 8 years ago

Hi Alexandru,

That’s fantastic news, thank you so much for that.

Really appreciate all of the hard work and effort you put into GQRX, anything I can do in return please do let me know.

Look forward to the release of 2.6.

Kind regards,

Sean.

On 17 Sep 2016, at 22:43, Alexandru Csete <notifications@github.com mailto:notifications@github.com> wrote:

Good news, I have now tried updating the HID API in GNU Radio / gr-fcd and the Funcube Dongle works again. I will include this fix in the 2.6 release in a few weeks and also send a patch upstream.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/csete/gqrx/issues/358#issuecomment-247809916, or mute the thread https://github.com/notifications/unsubscribe-auth/ATC6eJVqvzS1C1_v3oyXV6FBdnuLs2FZks5qrF7ygaJpZM4H-h0Y.

csete commented 8 years ago

Released.

geozsj commented 8 years ago

Good news. Can't wait to try it.