A few patches for gr-osmosdr

This is mostly for @joshblum but others are of course welcome to comment :slightly_smiling:

While working on Gqrx 2.6 I have accumulated a few patches for gr-osmosdr that I would like to include in our Ubuntu packages until upstream gets updated. The patches are available for review in my own copy of gr-osmosdr on Github in the rfscape and airspy branches respectively.

The RFSpace patches are necessary to make the CloudIQ receiver work at all. The first patch adds a 10 ms delay to the constructor allowing the device to become ready for commands. Without it, the connection will fail in at least nine out of ten times.

The second patch sends a periodic “get status” packets at 1 minute interval in order to keep the connection alive. Without it the device will stop sending IQ data after about 5 minutes.

The Airspy patch takes advantage of the possibility to upload custom filter taps to the IQ converter in libairspy. This can be used to reduce the CPU load of the IQ converter when e.g. the application uses input decimation anyway.

With this patch I have managed to get Gqrx running with the Airspy R2 and Airpy mini on the Raspberry Pi 2 and 3!

Admittedly, the Airspy patch is a hack but I would definitely like to have the RFSpace patches included. What do you think?

While working on Gqrx 2.6 I have accumulated a few patches for
gr-osmosdr that I would like to include in our Ubuntu packages until
upstream gets updated. The patches are available for review in my own
copy of gr-osmosdr on Github in the rfscape and airspy branches respectively.

Cool. I will do what I can, but usually you need to run the patches by someone over at osmocom. I can only merge stuff when I get permission. But I do have some comments since you asked :slightly_smiling:

The RFSpace patches are necessary to make the CloudIQ receiver work at all. The first patch
adds a 10 ms delay to the constructor allowing the device to become
ready for commands. Without it, the connection will fail in at least
nine out of ten times.

Whats the cause of the failure, is there just a race for the server to setup after its been commanded to start on a different port? I’m not super positive about the implementation details here, so I was just curious why there might be a race.

Rather than an arbitrary sleep, what about a loop that repeatedly tries to connect and get a reply. The loop could have a worst case timeout where it eventually throws and declares that there is no valid endpoint to communicate with.

The second patch
sends a periodic “get status” packets at 1 minute interval in order to
keep the connection alive. Without it the device will stop sending IQ
data after about 5 minutes.

Just a curiosity, but is the connection hang up something specific to the server? or is this just a TCP thing where the OS eventually closes the idle connection?

Is the status request inside the work function disruptive to the stream, since it blocks waiting for a reply? Maybe keepalive should be its own thread that mostly sleeps. Would there be a case where GQRX has an open handle to the server but is not currently streaming for a long period of time? Thats another good reason for a dedicated thread.

The Airspy patch
takes advantage of the possibility to upload custom filter taps to the
IQ converter in libairspy. This can be used to reduce the CPU load of
the IQ converter when e.g. the application uses input decimation anyway.

Its this something worthwhile adding to libairspy itself? Like an automatic bandwidth selection function based on sample rate. If you are not aware, there is now SoapyAirspy. And so the very useful kernels could be copied into each set bandwidth implementation or a similar patch for airspy itself (if that sounds practical) would be useful for a wider audience.

Hi Josh,

Thanks for the reply. I wasn’t aware of that you had commit access at osmocom, so I was only asking if you were OK with applying these patches to the Ubuntu packages in our PPA?

I do intend to take it up with Dmitri but that can take some time depending on how busy he is. Including the patch in PPA would give feedback from real world users in the meanwhile :slightly_smiling:

Whats the cause of the failure, is there just a race for the server
to setup after its been commanded to start on a different port? I’m not
super positive about the implementation details here, so I was just
curious why there might be a race.

I think what’s happening is that the processor in the radio, an STM32F4, needs time to initialize something before it is ready to accept commands on the newly established TCP connection. I intend to ask RFSpace about it since there is no mention of this in the interface document. They may not even be aware of this since that few milliseconds delay may occur naturally on a network, in particular when the host is running on Windows :wink:

Rather than an arbitrary sleep, what about a loop that repeatedly tries
to connect and get a reply. The loop could have a worst case timeout
where it eventually throws and declares that there is no valid endpoint
to communicate with.

Connection is already established at that point but for some reason the radio does not respond to the first command. I suspect the packets arrive at the radio but are dropped because the radio is busy. To make it worse, the radio needs to receive a first command within 5 seconds, otherwise it will terminate the connection.

So, the exact behavior is:

  1. Connection is established to the radio
  2. We send a command using the write() function which returns fine
  3. We read the response using the read() command which then hangs.
  4. After 5 seconds, the radio terminates the connection

I was looking at the serial console of the radio and it looks like it never receives the commands, hence my idea to wait for a few milliseconds. I think the extra 10 ms delay is an acceptable compromise until we have a better understanding of what is going on.

Just a curiosity, but is the connection hang up something specific to
the server? or is this just a TCP thing where the OS eventually closes
the idle connection?

That’s an interesting point, it could indeed be the OS terminating the connection because of inactivity.

Is the status request inside the work function disruptive to the stream,
since it blocks waiting for a reply? Maybe keepalive should be its own
thread that mostly sleeps.

I think you are right… I’ll see if I can add a separate thread to do this instead of doing it in the work() method.

Would there be a case where GQRX has an open
handle to the server but is not currently streaming for a long period of
time? Thats another good reason for a dedicated thread.

The radio uses a TCP connection for control channel and a separate UDP connection for IQ streaming. Therefore, the TCP connection becomes idle even when IQ data is streaming, unless periodic status requests or other commands are sent.

Its this something worthwhile adding to libairspy itself? Like an automatic bandwidth selection function based on sample rate.

Libairspy already does this for the sample rates supported by the devices, i.e 10/2.5 or 6/3 Msps depending on device type. We can ask Youssef if a new “set alias free bw” function could be added. Actually, he was the one who told me about this trick and gave me the kernels.

If you are not aware, there is now SoapyAirspy.
And so the very useful kernels could be copied into each set bandwidth
implementation or a similar patch for airspy itself (if that sounds
practical) would be useful for a wider audience.

Do you mean for devices other than Airspy?

I have re-implemented it using a dedicated thread. It is available in this new diff. I have also added a boost::mutex::scoped_lock to the function doing the TCP read/write to prevent the thread and the set_xyz() functions from interfering with each other.

Sorry with the delay, its not been my week. I like the patches, I think they should go into the PPA and ultimately upstream to gr-osmosdr source.

The only exception is that I think the airspy patch. If we can get an addition to libairspy itself, then we can just use the patch in the PPA for now and make gr-osmosdr airspy source use the new libairspy API call when its detected. Just my opinion.

Cool looks good. Good catch with the mutex.

If you are not aware, there is now SoapyAirspy.
And so the very useful kernels could be copied into each set bandwidth
implementation or a similar patch for airspy itself (if that sounds
practical) would be useful for a wider audience.

Do you mean for devices other than Airspy?

I was thinking C API users of airspy. And something like a “set alias free bw” function in libairspy itself seems like the right idea.

Hi Josh,

No problem with the delay. It wasn’t blocking anything.

I have updated the gr-osmosdr package to include the patches. They are applied at build time so they can be easily disabled if something turns out to be bad, which I do not expect will happen.

I will talk to Youssef about adding the extra function to libairspy, though he was the one who gave me the tip, so I assumed he didn’t want this in libairspy. But we will see.