pothosware / SoapyRTLSDR

SoapySDR RTL-SDR Support Module
https://github.com/pothosware/SoapyRTLSDR/wiki
MIT License
124 stars 29 forks source link

If you request an invalid sample rate, the internal member variable in Settings.cpp will be updated #50

Closed crroush closed 3 years ago

crroush commented 3 years ago

Currently there is no way to determine if a sample rate was valid or not. You can certainly check the ranges and not allow invalid rates to go through as a workaround, but it seems that the function calls should do the right thing.

The error should be handled by either throwing an exception or keeping the sample rate the same, so the user can query "getSampleRate" to get the actual rate that is being used.

Since the actual rate that gets set can be slightly different than the rate requested, getSampleRate should query the device vs using the member variable sampleRate.

void SoapyRTLSDR::setSampleRate(const int direction, const size_t channel, const double rate)
{
    long long ns = SoapySDR::ticksToTimeNs(ticks, sampleRate);
    sampleRate = rate;
    resetBuffer = true;
    SoapySDR_logf(SOAPY_SDR_DEBUG, "Setting sample rate: %d", sampleRate);
    rtlsdr_set_sample_rate(dev, sampleRate);
    ticks = SoapySDR::timeNsToTicks(ns, sampleRate);
}
guruofquality commented 3 years ago

I think that the call should throw if rtlsdr_set_sample_rate() returns an error. Can you make a PR?

zuckschwerdt commented 3 years ago

As far as I see rtlsdr_get_sample_rate() doesn't involve hardware. We should be fine to just always proxy the get. At least we should query the real rate in the constructor and in setSampleRate() as suggested. I can implement these changes if needed.

guruofquality commented 3 years ago

As far as I see rtlsdr_get_sample_rate() doesn't involve hardware. We should be fine to just always proxy the get. At least we should query the real rate in the constructor and in setSampleRate() as suggested.

I almost dont really see the need for even caching "sampleRate" in SoapyRTL, but I suppose if rtlsdr_get_sample_rate() ever changes, its for the best.

I can implement these changes if needed.

Sounds good, looks like the changes are:

crroush commented 3 years ago

We also should look at doing the same thing for setting frequency. There is no error that is thrown when it is outside the range.

On Wed, Oct 28, 2020 at 10:35 AM Josh Blum notifications@github.com wrote:

As far as I see rtlsdr_get_sample_rate() doesn't involve hardware. We should be fine to just always proxy the get. At least we should query the real rate in the constructor and in setSampleRate() as suggested.

I almost dont really see the need for even caching "sampleRate" in SoapyRTL, but I suppose if rtlsdr_get_sample_rate() ever changes, its for the best.

I can implement these changes if needed.

Sounds good, looks like the changes are:

  • throw error if rtlsdr_set_sample_rate actually fails
  • set cached sample rate based on rtlsdr_get_sample_rate() after tuning

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pothosware/SoapyRTLSDR/issues/50#issuecomment-718056832, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACMFB6Z4AUPMBHTR47NUARLSNBB63ANCNFSM4TCNDLNA .

crroush commented 3 years ago

As far as I see rtlsdr_get_sample_rate() doesn't involve hardware. We should be fine to just always proxy the get. At least we should query the real rate in the constructor and in setSampleRate() as suggested. I can implement these changes if needed.

it does recalculate it and logs an error if your desired Fs is different, but you can use the member variable from the rtl device. Querying it would be suggested in case the functionality is ever changed on the librtl side.