Skip to content
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

Sample rate settings bug #68

Closed
mehdideveloper opened this issue Feb 16, 2020 · 24 comments
Closed

Sample rate settings bug #68

mehdideveloper opened this issue Feb 16, 2020 · 24 comments
Assignees
Labels
bug Something isn't working device-specific Device-specific issues

Comments

@mehdideveloper
Copy link
Collaborator

When I try to run LimeSDR Mini at a high bandwidth (say 20Msps) and start the SDR, the bandwidth and sampling rate are automatically set to 100Ksps and 100Khz!
There's no way I can set it to anything high (even 10Msps won't work)

By the way, in the Settings pane, the Bandwidth value's unit is Khz. I'm not sure if this is the best option. I mean, for 10MHZ, I have to set it to 10,000. This is personal. I think setting it to HZ, or having it automatically follow the sampling rate value would be better (like if I set 10Msps, then it also sets to 10MHZ, so I don't need to change them separately. Also if I need less bandwidth, I can set decimation, like SDRSharp?!)

@BatchDrake
Copy link
Owner

I experienced that as well. Probably a LimeSDR Mini bug: the device is rejecting 20 Msps and for some reason falls back to 100 Ksps (only happens with LimeSDR mini for some reason). If you go ahead and try 64 Msps it will accept that, although will probably deliver samples way slower than that. I'm not so sure about the right way to handle per-device behavior (from the UX perspective, I mean). The bandwidth sets back to 100 kHz because that's also the biggest sample rate the device is supporting.

Regarding setting the bandwidth, you are absolutely right, although I'd create a separate issue for that.

@BatchDrake
Copy link
Owner

CubicSDR also reports problem with this module cjcliffe/CubicSDR#716

@BatchDrake BatchDrake self-assigned this Feb 16, 2020
@BatchDrake BatchDrake added bug Something isn't working device-specific Device-specific issues labels Feb 16, 2020
@FFY00
Copy link

FFY00 commented Feb 16, 2020

I can reproduce in a normal LimeSDR. Would you consider using the same approach as they did? cjcliffe/CubicSDR#718

@BatchDrake
Copy link
Owner

Just looked into it, and I don't think it will help. Their problem was related to a streaming sample rate setting, while ours has to do with an improper sample rate setting. I think I know what may be going on, but I can't make further tests until tomorrow. Hope I can fix this soon.

@BatchDrake
Copy link
Owner

I've just made a little experiment. SoapySDR's Lime module reports only 2 allowed sample rates (100ksps and 65 Msps). If this is right, LimeSDR Mini would be unusable for streaming applications, and I don't think it is the case. Do you guys have any info about which are the supported rates?

@mehdideveloper
Copy link
Collaborator Author

mehdideveloper commented Feb 17, 2020

@BatchDrake In CubicSDR, you can only select between those 2 sampling rate numbers on the startup dialog, but then when you start the LimeSDR, you can manually enter a number.
Also in SDR Console it is possible to start from as low as 750Khz and go up. I can send you a screenshot of both if it helps

@BatchDrake
Copy link
Owner

I think the best approach here will be to keep the supported sample rate list in the device object and for SDR devices to provide a combo box (keeping the spinbox for file captures). I will redesign the config dialog once again.

@BatchDrake
Copy link
Owner

I have just added the aforementioned combobox. Pull from the following branches:

  • sigutils: feature/performance-improvements
  • suscan: feature/performance-improvements
  • SuWidgets: feature/ux-improvements
  • SigDigger: feature/ux-improvements

Compile and tell me what you think. In case you have a reduced number of available sample rates, I'm afraid you will have to play with the decimation value (please note that the decimator may be consuming too much CPU right now, don't worry: there is already a fix for that on the way)

@mehdideveloper
Copy link
Collaborator Author

There's nothing new on sigutils feature/performance-improvements and suwidgets feature/ux-improvements.
Their latest changes are made on 23/24 Feb

@mehdideveloper
Copy link
Collaborator Author

The combobox is a good idea 👍

@skgsergio
Copy link

skgsergio commented Apr 2, 2020

I've just compiled everything from develop (SuWidgets, sigutils, suscan and SigDigger) and I found that LimeSDR-mini performance is poor, it has cuts in the reception. Then I noticed that the sample rate in use is way too high, even playing with the decimation I don't manage to improve the situation. And with the other option available 100 ksps is not enough.

Hacking SigDigger a bit (exporting the profile, modifying it, and importing it again) and setting it to 5 Msps (<suscan:field name="samp_rate" value="5000000" />) works without major issues. Is there any reason not to allow a custom value like gqrx does?

I know that is due the fact that Lime's module for SoapySDR is reporting that:

[sconde@puck ~]$ SoapySDRUtil --probe | grep -E "(Channel |Sample rates)"
[INFO] Make connection: 'LimeSDR Mini [USB 3.0] 1D40EBEE41256B'
[INFO] Reference clock 40.00 MHz
[INFO] Device name: LimeSDR-Mini
[INFO] Reference: 40 MHz
[INFO] LMS7002M register cache: Disabled
-- RX Channel 0
  Sample rates: [0.1, 65] MSps
-- TX Channel 0
  Sample rates: [0.1, 65] MSps

Regarding the question of the supported rates I can't find anything useful more than the fact that the LimeSDR-mini version theoretical max rate is 30.72 Msps, and 61.44 Msps for the non mini version (at least this is what they advertise).

From my experience, LimeSDR allows anything arbitrary not in specific steps but it might do something more internally. I guess if we check the firmware (VHDL code) we could confirm this but... 🤷‍♂️

What is clear is that the module for SoapySDR just sends as sample rates some values they consider "reasonable" max and min limits (and they don't even set different values for the Mini and non-Mini versions):

//reasonable limits when advertising the rate
#define MIN_SAMP_RATE 1e5
#define MAX_SAMP_RATE 65e6

https://github.com/myriadrf/LimeSuite/blob/b14b34569bed8e6f9ee12b87a1a19799aff0443e/SoapyLMS7/Settings.cpp#L23-L25

std::vector<double> SoapyLMS7::listSampleRates(const int direction, const size_t channel) const
{
    return {MIN_SAMP_RATE, MAX_SAMP_RATE};
}

SoapySDR::RangeList SoapyLMS7::getSampleRateRange(const int direction, const size_t channel) const
{
    return { SoapySDR::Range(MIN_SAMP_RATE, MAX_SAMP_RATE) };
}

https://github.com/myriadrf/LimeSuite/blob/b14b34569bed8e6f9ee12b87a1a19799aff0443e/SoapyLMS7/Settings.cpp#L471-L479

The setSampleRate function is not more helpful: https://github.com/myriadrf/LimeSuite/blob/b14b34569bed8e6f9ee12b87a1a19799aff0443e/SoapyLMS7/Settings.cpp#L433-L462

@FFY00
Copy link

FFY00 commented Apr 2, 2020

Yeah, the LimeSDR software has never been great. I would like to redo the gateware and write better drivers, but I don't know if it is woth it. Either way, it's low on my priority list.

@BatchDrake
Copy link
Owner

The true reason is that, for some SDRs, forcing a value that is not theoretically supported, makes it deliver samples at some random rate while reporting an absolutely different rate, breaking things like audio playback (this is an issue that happened with RTL-SDR and motivated this change).

The thing is, there are two issues here:

  • We have to refactor the device abstraction in order to allow sample rate ranges and,
  • Provide some GUI to configure the sample rate based on those ranges.

This refactor is not difficult per se, just a bit of work and can be implemented in less than a weekend. What concerns me here is the UX: how do we provide information to the user about ranges? I was considering letting the user input whatever he wants, and, if the rate is invalid, automatically correct him to the nearest accepted range, or maybe change the background color of the spinbox, or even show a warning icon. However, the user should be informed about accepted / desirable ranges, and this is something we lose by removing the combo box.

In any case, the old SoapySDR API for sample rates (which is list-based, not range-based) is broken and does not provide accurate information about what the device is capable of. I'd love to have this fixed ASAP, but I still don't know what would be the best way to do it. Any ideas?

@FFY00
Copy link

FFY00 commented Apr 2, 2020

The true reason is that, for some SDRs, forcing a value that is not theoretically supported, makes it deliver samples at some random rate while reporting an absolutely different rate, breaking things like audio playback (this is an issue that happened with RTL-SDR and motivated this change).

That should not be supported by the driver, period. If they really wanted that, an extra API should be exposed. Letting users choose bad values in normal scenarios is just wrong.

@BatchDrake
Copy link
Owner

I totally agree with you. The problem is that the user is going to see something broken that I can't / don't want to fix. The only thing I can do about it is to constrain the accepted values in the most obvious way to reduce the chances to hit one of these undefined behaviors.

@skgsergio
Copy link

Regarding UX I'd keep the dropdown with the stuff that the drivers return, which should be the trusted values if drivers were ok. But I'd add a checkbox that when enabled it allows to override the values by disabling the dropdown and enabling an input. Another option is just making the dropdown to be a combo box and allowing user input.

In any case, you can print a warning or marking that is unsafe in some way but I'd keep the ability to set custom values somehow.

And then document somewhere that LimeSDR values are just broken and you need to set them manually.

@skgsergio
Copy link

Also, I saw the LMS API has a LMS_GetSampleRateRange function (https://docs.myriadrf.org/LMS_API/group___f_n___h_i_g_h___l_v_l.html#ga2be0ab821f7a13e137c807521bb0e467), I want to try this and if it works I'll try to PR upstream a fix for the getSampleRateRange function and maybe also fix listSampleRates returning steps in that range.

@BatchDrake
Copy link
Owner

Answering to you about UX, I think I'd prefer the first solution as there are already two stacked widgets (the combo and the spinbox) that are swapped according to the source type: SDR devices are set through the combo while file sources are set manually. Adding a checkbox that overrides this behavior is trivial.

Regarding that API fix, that would be extremely helpful! LimeSDR is still the black sheep of SigDigger and any improvement in that direction would be appreciated. Let's hope it works and they accept your changes soon!

@skgsergio
Copy link

skgsergio commented Apr 2, 2020

Now that I've been playing with the SoapySDR API and the LimeSDR module I can show some progress 🎉

Turns that in the past LimeSDR had some code for giving an actual list of sample rates, but they reworked all the code and now they return those shitty constants. However LMS_GetSampleRateRange works and I made some changes to return both the actual range in the range API and a list using the range and the min step returned by the LMS API function.

I want to clean the code a bit (I broke all the code style for the quick testing) and improve the returned list but so far it works and in the LimeSDR-Mini doesn't give values over the specs 🚀

Screenshot from 2020-04-02 22-37-57

Tested it at 30.72 Msps (theoretical max) and works great.

@BatchDrake
Copy link
Owner

BatchDrake commented Apr 3, 2020

That is good news! Hope they accept your changes soon. If for whatever reason they don't, or it is delayed after the release, I could merge your changes to my local copy of the SoapySDR LimeSDR module and go ahead with the release.

PS: are you capturing at 30.72 Msps and listening to audio at the same time?

@skgsergio
Copy link

Yes capturing and demoding FM, I had it for some minutes and 0 issues.

@skgsergio
Copy link

PR to LimeSDR: myriadrf/LimeSuite#305

@skgsergio
Copy link

The PR has been merged so eventually it will be version tagged and released 🚀

@BatchDrake
Copy link
Owner

This is good news, thank you! :D I'll keep my Lime libraries updated and ready for the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working device-specific Device-specific issues
Projects
None yet
Development

No branches or pull requests

4 participants