pothosware / SoapyAirspy

Soapy SDR plugin for the Airspy
https://github.com/pothosware/SoapyAirspy/wiki
MIT License
25 stars 13 forks source link

No error checking / validation of sample rates #25

Open mutability opened 3 years ago

mutability commented 3 years ago

If you request a sample rate that is not supported by the hardware, then no error is reported but the sample rate is not used.

There's no error checking or validation done at either the point of calling setSampleRate() or when airspy_set_samplerate is called here: https://github.com/pothosware/SoapyAirspy/blob/master/Streaming.cpp#L197

Callers can verify the sample rate against the values returned by listSampleRates() / getSampleRateRange() before deciding to use a sample rate, but the problem is then that other drivers don't support that API well (notably, rtlsdr doesn't implement getSampleRateRange() so working around the airspy problem then limits what you can do with rtlsdr unless you special-case one or the other based on the driver in use)

guruofquality commented 3 years ago

And I think because of how the library is, the call cannot be made during setSampleRate() because that would be an issue while streaming.

One solution might be to simply check the rate passed into setSampleRate() against the values returned by airspy_get_samplerates. I believe the airspy code basically replicates airspy_get_samplerates inside of airspy_set_samplerate anyway.

Another solution might be to move this whole chunk of code into the setSampleRate() and handle the error code there

if (airspy_is_streaming(dev))
{
        airspy_stop_rx(dev);
        r = airspy_set_samplerate(dev, sampleRate);
        airspy_start_rx(dev, &_rx_callback, (void *) this);
 }
else  r = airspy_set_samplerate(dev, sampleRate);
//throw if r is bad

If you have the hardware available, do you want to try out a fix and make a PR? I dont think this is a change that can be done blindly (ie it just compiles).

mutability commented 3 years ago

I do have hardware, I'll have an experiment.

(From looking through the library/firmware, I think that maybe it's safe to call airspy_set_samplerate while streaming data; the firmware seems to have the right logic to stop/retune/restart; I'll try that too)