pothosware / SoapyBladeRF

Soapy SDR plugin for the Blade RF
https://github.com/pothosware/SoapyBladeRF/wiki
23 stars 19 forks source link

Fixes for Cubic compatibility #27

Closed drahosj closed 5 years ago

drahosj commented 5 years ago

listSampleRates and listBandwidths have been re-added, since it seems most proper for the Soapy module to provide these lists as device-specific recommendations, rather than having any higher level applications derive recommendations from a range.

Additionally, hasGainMode really tries its best to figure out if AGC is available. If not, it returns false so that hasGainMode can be used as a safeguard before a (potentially failing) call to setGainMode.

guruofquality commented 5 years ago

So the getBandwidthRange() and getSampleRateRange() are actually returning a list of ranges. Due to the issue that hardware supports both continuous (virtually continuous ranges) and then at other times discrete values, the list (vector of doubles) API was deprecated in favor of the vector of ranges. I guess there is some precedent because this is how gr-osmosdr and uhd work as well.

So a few thoughts:

1) Are the list of numbers you provide specific to bladerf1? If so they should be conditional if there is one channel (or some other indicator).

2) This is absolutely perfect for a vector of ranges, its just 3 ranges, each w/ their own start, stop, step:

    for (double r = 160e3; r <= 200e3; r += 40e3) options.push_back(r);
    for (double r = 300e3; r <= 900e3; r += 100e3) options.push_back(r);
    for (double r = 1e6; r <= 40e6; r += 1e6) options.push_back(r);

3) Discrete points are basically just one range per point, where the start == the stop

    options.push_back(0.75);
    options.push_back(0.875);
    options.push_back(1.25);

4) How well does cubic support the vector of ranges API? Eventually I would like to delete the deprecated functions :-)

Its up to the GUI how to present things like this. So if one traversed the ranges, and the number of points is small, the GUI representation could be a drop-down menu. If its large, or if its just a single range with a very small step size, perhaps a slider is better. It really needs to adapt to match the hardware, which could be continuous or discrete anyway.

vsonnier commented 5 years ago

@drahosj @guruofquality Hello ! The following issue https://github.com/cjcliffe/CubicSDR/issues/513 is still open on Cubic because indeed it still only manages listSampleRates.
I had implemented a workaround to decimate the number of them in a very crude way (pick one every N) to limit them to 25 in the menu entry. The absolute Min and Max values are always included and there is the Manual entry where the user can choose anything between min and max.

How well does cubic support the vector of ranges API? Eventually I would like to delete the deprecated functions :-)

What would be possible in Cubic with not too much changes would be keep generating lists of discrete changes from the set of contnuous ranges, in a "smart" way to choose "meaningfull" discrete values in each range, also keeping the [min ; max] of each range. Would it be equivalent to the old listSampleRates then for all the traditionnal devices ?

Going beyond that would need changing the GUI by replacing this simple menu by something more sophisticated combining sliders and menus, or something else entirely.

guruofquality commented 5 years ago

What would be possible in Cubic with not too much changes would be keep generating lists of discrete changes from the set of contnuous ranges, in a "smart" way to choose "meaningfull" discrete values in each range, also keeping the [min ; max] of each range. Would it be equivalent to the old listSampleRates then for all the traditionnal devices ?

Lets just suppose that cubic will only need a min/max, a manual entry, or a drop-down selection with enumerated options, the following psudocode below could describe how to populate the drop-down.

Like if this for (double r = 160e3; r <= 200e3; r += 40e3) options.push_back(r); for (double r = 300e3; r <= 900e3; r += 100e3) options.push_back(r); for (double r = 1e6; r <= 40e6; r += 1e6) options.push_back(r); code has nothing to do with the bladerf, but its just useful for dropdown values that cubic users like, it should be in the gui logic then.

auto ranges = sdr->getSampleRateRange(dir, ch);

//you can get the same min and max from the list of ranges like this:
auto minVal = ranges.front().minimum();
auto maxVal = ranges.back().maximum();

//when to fake discrete options, its a single range, min/max, no step so its continuous
if (ranges.size() ==1 and ranges.front().step() == 0)
{
  //its a min/max range with no step, so I guess make fake enumerations that users will find useful
}

//when to do finite options
std::vector<double> options;
for (const auto &range : ranges)
{
  if (range.minimum() == range.maximum()) options.push_back(range.minimum());
  else if (range.step() == 0) continue; //continuous range, probably shouldn't happen on ranges.size() > 1
  else for (auto x = range.minimum(); x <= range.maximum(); x+= range.step())
    options.push_back(x);
}
if (options.size() < ITS TOO BIG)
{
  //a reasonable number of finite steps, use the drop down menu I guess
}
else
{
//once again, just fake enumerations that users will find useful
}

Going beyond that would need changing the GUI by replacing this simple menu by something more sophisticated combining sliders and menus, or something else entirely.

I think we can move forward using the range calls without changing cubic gui, just the logic. For just a sample rate, the drop down is probably fine anyway. How many sample rates and bandwidth values are really useful anyway.

vsonnier commented 5 years ago

I think we can move forward using the range calls without changing cubic gui, just the logic. For just a sample rate, the drop down is probably fine anyway. How many sample rates and bandwidth values are really useful anyway.

For Cubic, it is indeed a matter of changing the facade std::vector<long> SDRDeviceInfo::getSampleRates(int direction, size_t channel) to provide a list of sample rates from RangeList getSampleRateRange(const int direction, const size_t channel) with the following pseudo-code:

for each R in RangeList :
    insert Rmin and Rmax in the list (+ for each step value, if provided)

sort by std::sort
remove duplicates by std::unique

Note that Cubic don't use bandwidths, only sample rates so changing the API for the former is a non-issue.

I would not attempt to fill the gaps with special values though: Instead, I would keep the original list of discrete values as is encoded as ranges in each particular SoapySDR implementation, if need be:

RangeList = [val1, val2, step = 0], [val2, val3, step = 0 ],[val3, val4, step = 0] ...etc.

The reason is that sample rates / bandwidth ranges don't provide what the discrete lists did implicitly: particular values of interest for a given device. It is clear for SoapySDRPlay where the list of both bandwidths and sample rates indeed match sweet spots for low-level settings.

Another exemple may be SoapyRTLSDR listSampleRates that would match some "best working" sample rates ?

For really continuous devices, that would be up to each SoapySDR implem to provide values of interest the way above instead of having a single range.

A minor point where Cubic Manual Entry would be affected by disjoint sample rate ranges is that the dialog only automatically handle [min; max] value as user input. We should then list the available ranges in the dialog text, indicating that any frequency entered outside the ranges would be silently ignored, and validate the final value against the real range list.

That would be done by adding a RangeList SDRDeviceInfo::getSampleRateRange(int direction, size_t channel) facade that would re-interpret the RangeList getSampleRateRange(const int direction, const size_t channel) call by merging the actually adjacent ranges and other overlapping information.

guruofquality commented 5 years ago

I think its fine if the module provides some sort of list of magic values that happen to be useful. I just want to make sure it 1) matches up with the hardware and 2) uses the ranges api. I'm also trying to rectify this with what the authors of bladerf have implemented for gr-osmosdr:

1) bandwidth ranges are totally dropped here, its just the range returned by the libbladerf: https://git.osmocom.org/gr-osmosdr/tree/lib/bladerf/bladerf_common.cc#n636

2) sample rates have the same list of 3 ranges, but its modified by the range returned by libbladerf. I think this is important for the v2 support: https://git.osmocom.org/gr-osmosdr/tree/lib/bladerf/bladerf_common.cc#n517

3) I think cubic can use the ranges just the same as gqrx does, but in the meantime, can the change fill in the ranges with the same loopvalues mentioned in osmosdr::meta_range_t bladerf_common::sample_rates(bladerf_channel ch), but in addition, listSampleRates() still exists, it just calls getSampleRate() and loops through the ranges to fill in the vector of discrete rates.

Good compromise? That keeps cubic working in the meantime, regardless of changing to use ranges, and I think I want to do the same to other modules.

Thoughts?

guruofquality commented 5 years ago

I split this into two PRs, I will probably just merge them tomorrow, please leave any comments on the respective PRs if its not right. This should give the gain mode fix, and implemented the deprecated calls to match the gr-osmosdr values. In addition, the sample rate ranges call to implemented the gr-osmosdr ranges.