Reconcile SDRPlay gains with SoapySDR abstractions

[dlaw]

1. Rebase the existing new-gain-controls branch onto latest master
2. Use negative gain values to represent gain reductions. For example, IF gain reduction of 20 to 59 is represented by an IF gain of -20 to -59. RF gain reduction of 0 to N is represented by an RF gain of 0 to -N.
3. Clean up the generic setGain implementation and add a corresponding generic getGainRange.

Let's use this PR to discuss whether this proposal represents the desired semantics for a change. I will be happy to make any additional updates once we come to a consensus.

This PR addresses #10, supersedes #26, and is related to the discussion in cjcliffe/CubicSDR#825.

Cheers,
David

[fventuri]

Thanks for opening this PR, David.

If I remember correctly (I had even forgotten about creating #10 a while ago. thanks for finding it), the values for the 'lnastates' and 'ifgains' tables came from @SDRplay , and I I think they are similar to the values used in the SDRuno application, which is the reference we used when designing and coding the SoapySDRPlay driver, to keep things consistent for the users.

Also on a different but related project, the new GNU Radio OOT module for SDRplay RSPs based on API version 3.X (https://github.com/fventuri/gr-sdrplay3), I used the tables from the SDRplay API specification guide (see here for instance: https://github.com/fventuri/gr-sdrplay3/blob/master/lib/rspduo_impl.cc#L265-L285), so that could be a possible approach too.

Franco

[dlaw]

That is correct, the tables of LNA and IF configurations for the "default" gains were provided by @SDRplay in the initial commit. I can't find them in the API documentation but presumably they are taken from SDRuno. I think this is great to have, and it is a nice way to override the default multi-stage gain allocation algorithm from SoapySDR setGain. As you mentioned, the LNA state to decibel tables are in section 5.3 of the API documentation.

The challenge of this PR is trying to keep things similar to SDRuno while at the same time keeping things similar to the other SoapySDR drivers. After all, SoapySDR is supposed to be an abstraction layer that hides some of the variation from radio to radio :-)

So, based on the previous discussion and new-gain-controls, I think we are agreed that we want to have two gain sliders named "RF" and "IF".

These are my remaining questions:

1. Should we keep the RF gain range going from 0 to -9 (or 0 to -8 or 0 to -3), or should we use the conversion tables to expose it as a decibel gain going from 0 to -64 (or 0 to -43, for RSP1)? The former is more similar to SDRuno and the latter is more similar to other SoapySDR drivers (for example, the HackRF amplifier is exposed as a gain which takes the discrete values of 0 or 14 dB).
2. Do we want to keep the default setGain range from 0 to 28 (resulting in ~3.7 dB gain per count), or should we expect an aggregate gain in decibels (as the sum of the RF gain in decibels and the IF gain in decibels)? In the latter case, valid values for setting the overall gain would range from e.g. 0 to -107 dB and we would divide by 3.7 to determine the index into the lookup table.
3. Should we implement a default getGain method to go with the default setGain? This is easy to do if we convert everything to decibels, but difficult with the present implementation of setGain (how do we convert an arbitrary LNA state and IF gain to a value of 0 to 28 which is consistent with the lookup tables). At present the default getGain will return the sum of the LNA state number and the IF gain, which is definitely wrong.

My preference is to convert everything to decibels (including the RF gain and the aggregate getGain and setGain methods). This adds some complexity to the driver but I think it is the best decision for the users (most functional and least confusing). I will wait a while for additional comments, then if there is some consensus here I will start implementing the changes.

[nmaster2042]

Would it be possible to add gain step in ::getGainRange() ?
Some app requires this, not all soapy drivers actually implement it.

[dlaw]

@nmaster2042 we can easily add 1 dB step size for IF gain and overall gain. The actual step spacing for RF gain is nonlinear, so I think we will need to expose it with a false 1 dB step size and then round to the closest value in the LNA state table during setGain.

[fventuri]

@dlaw
Now that I am done with the other issue about the selection of the RSP device using the serial number (#23), I am ready to address the improvements to the gain controls you propose.

Before we get started, I'd like to have some feedback from the usual folks (@SDRplay , @vsonnier , @guruofquality , @nmaster2042 , @jketterl ) on what they should look like based on your comment above (#27 (comment))

Just to make sure we are all on the same page, this is the current behavior of the SoapySDRPlay3 driver regarding gains/attenuations (based on what the source code does):

• listGains() (https://github.com/pothosware/SoapySDR/blob/master/include/SoapySDR/Device.hpp#L681) returns a list of two strings: 'IFGR', and 'RFGR'
• getGain(..., name) (https://github.com/pothosware/SoapySDR/blob/master/include/SoapySDR/Device.hpp#L740) returns the following:
  • if name == "IFGR", the value returned is chParams->tunerParams.gain.gRdB, which is a parameter expressing the base band gain reduction in dB; its values are typically in the range from 20 (lowest gain reduction, maximum gain) to 59 (highest gain reduction, minimum gain)
  • if name == "RFGR", the value returned is chParams->tunerParams.gain.LNAstate, which is a parameter expressing the so called 'LNA state', an integer value whose values control the attenuation of the LNA (typically higher values of the LNA state mean more attenuation, i.e. less gain). The range of values for the LNA state and their corresponding attenuation in dB vary depending on the RSP model and the selected frequency; they are fully described in a set of tables in appendix 5 of the SDRplay API Specification document
• getGainRange(..., name) (https://github.com/pothosware/SoapySDR/blob/master/include/SoapySDR/Device.hpp#L757) returns the following ranges:
  • if name == "IFGR", the range returned is [20, 59] (base band gain reduction in dB)
  • if name == "RFGR", the range returned is [0, N] (range of LNA state acceptable value), where N depends on the RSP mode (as mentioned above)
• setGain(..., name, value) (https://github.com/pothosware/SoapySDR/blob/master/include/SoapySDR/Device.hpp#L723) assigns the argument 'value' as follows:
  • if name == "IFGR", then chParams->tunerParams.gain.gRdB = value (i.e. the value gets assigned to the base band gain reduction in dB)
  • if name == "RFGR", then chParams->tunerParams.gain.LNAstate = value (i.e. the value gets assigned to the LNA state - see above)
• finally the current source code of the SoapySDRPlay3 driver does not have any implementation for a generic getGain() and setGain() (i.e. where the gain name is not specified)

With this information and the suggestion (and questions) from @dlaw I'd like to start a discussion on how to improve the gain/attenuation controls.

I also would like to limit as much possible any 'breaking' changes, that would affect client applications that use the driver (CubicSDR, gqrx, and many others, see here: https://github.com/pothosware/SoapySDR/wiki#platforms); for instance, as suggested above, we could leave the meaning and behavior of 'IFGR' and 'RFGR' unchanged, add two new gain controls called simply 'IF' and 'RF', and 'hide' the old controls (by having listGains() not return them), but I am not sure what the impact would be on the client applications, and the users using them.

Comments, ideas, objections are welcome.

Franco

[nmaster2042]

From a user point of view, I have high interest in @dlaw 's PR.

I totally agree to make SoapySDRPlay3 gain controls acts like other Soapy drivers instead of the gain reduction control and rename gains to RF and IF.

Regarding (#27 (comment)):

1. I would chose the to expose decibel gains
2. aggregate gain in decibels
3. implement a default getGain method to go with the default setGain

@fventuri regarding "breaking change" I agree it would be better to add 2 new sliders IF and RF, keeping the old ones.

To select the driver gain control behavior of the driver, what about adding a configurable feature CMakefile ?

Like the existing RF_GAIN_IN_MENU feature, a new RF_GAIN_REDUCTION (= 0 by default) could be added, so user can chose at build time if he wants IF/RF or IFGR/RFGR returned by listGains ?

[fventuri]

Thanks for your feedback @nmaster2042

I think you and @dlaw have a very strong point about the gains being exposed in dB.

Even more importantly, since this is a SoapySDR driver, I think it should conform to the SoapySDR driver gain API (see here and below: https://github.com/pothosware/SoapySDR/blob/master/include/SoapySDR/Device.hpp#L671).
In that file they explicitly say this regarding the value argument for instance in setGain() (https://github.com/pothosware/SoapySDR/blob/master/include/SoapySDR/Device.hpp#L721):

\param value the new amplification value in dB

Based on that I think that using the raw LNA state index for the RFGR gain violates that API (because it is not an "amplification value in dB").

Unfortunately this raises another tricky problem.

If you look at the 'Gain Reduction Tables' in chapter 5 of the SDRplay API specification document, you'll see that the gain reduction provided by a certain LNA state depends on the selected frequency. For instance for the RSPduo with LNAstate=4, you have a gain reduction of 37dB for rfHz in the range 0-60MHz, but that gain reduction drops to 20dB (i.e. 17db lower) in the range 60-420MHz.
So in the scenario where an RSPduo user is initially tuned to say 59MHz with an RF gain of -37dB (which is the value for LNAstate=4), and then they tune to say 61MHz, the code in the SoapySDRPlay driver would have to also change the LNAstate to say 7 (which has an RF gain of -38dB, the closest to the value before the frequency change).
Another issue would occur to an RSPduo user that is tuned to 61MHz and has selected an LNAstate of 9; if they move down to say 59Mhz, then the value of LNAstate=9 is no longer valid according to those tables (and it would probably throw some sort of error, I imagine).

I wonder if it is because of these 'quirks' that in SDRuno they decided to keep the RF gain/attenuation control as just a pure integer index (i.e. not as a value in dB). It is also possible that they do adjust that LNAstate index whenever the selected frequency changes range (for instance from 0-60MHz to 60-420MHz) to avoid sudden changes in attenuation, but I can also see how that could make the code more complicated (if you have Windows and SDRuno installed, it would be an interesting thing to find out).

Franco

[dlaw]

Based on this discussion, I would suggest that we express all gains in decibels and we do not perform any automatic change to the LNA state when the frequency is changed. This would mean that the RF gain in decibels may change when the frequency changes. However -- and I think this is important -- if you change back to your original frequency, you will also recover your original gain. I don't want to toggle from 61 MHz to 59 MHz then back to 61 MHz and find that some algorithm scrambled up my IF and RF gains.

I believe that CubicSDR already automatically refreshes the gain slider values whenever a different setting (such as the frequency) is changed. (This is why the existing RFGR slider automatically updates itself when a different LNA state is selected from the menu.)

Here is a specific proposal for what every function should do:

listGains() -- return IF and RF (not IFGR and not RFGR)

setGain(IF) and getGain(IF) -- value ranges from -20 dB to -59 dB, equal to the negative of the IF gain reduction

setGain(RF) and getGain(RF) -- frequency dependent -- value ranges from 0 dB to -64 dB, is converted to/from LNA state using the lookup tables in section 5.3 of the API documentation

setGain() -- frequency dependent -- automatically distribute the requested gain between RF and IF using the tables provided by SDRPlay in 1da639a

getGain() -- frequency dependent -- no change to default implementation (which returns getGain(RF) + getGain(IF))

setGain(IFGR) and getGain(IFGR) -- kept for backwards compatibility, value ranges from 20 to 59

setGain(RFGR) and getGain(RFGR) -- kept for backwards compatibility, value ranges from 0 to 9 and is equal to the LNA state

[fventuri]

@dlaw - thanks for your useful comment and ideas.

For this first part of my reply I am referring specifically to the RF gain/gain reduction (i.e. LNA state); more on the IF gain below.

The suggestion of having the RF gain frequency dependent has some consequences that need to be considered; imagine the scenario where the hypothetical user with his/her RSPduo goes from 59MHz to 61MHz, changes gain at 61MHZ, then they move up to 421MHz, change the gain to some other value there, then they go to 1001MHZ, change their gain again, finally they go back to 59MHz; what value of RF gain should the RSPduo set to to at this point? the original that they had at the beginning of their tuning around the bands? also what value of RF gain should the RSPduo have if then they switch to say 1500MHz? the same that they had when they were at 1001MHz? If this were the case, should the SoapySDRPlay code be able to remember and store the RF gain settings for each frequency (or range of frequencies) they visited?

Also if they were initially at say 61MHz with an RF gain reduction of 62 (LNAstate=9), and they change frequency to 59MHz (where LNAstate=9 is invalid), what should the SoapySDRPlay driver do?

As per the IF gain, there's also to take into account what to do if the AGC is enabled; what would be the meaning of the value of getGain() = getGain(IF) + getGain(RF) when the AGC is enabled and therefore the actual IF gain reduction could be significantly different from the value read from gRdB (perhaps they are listening to or tuned very close to a strong signal)?

Franco

[vsonnier]

Hello @fventuri , @dlaw and all.

I'm not repeating my arguments I wrote long before, but here is a quick summary of hopfully simple suggestions :

• Replace RF_GAIN_IN_MENU which should always be on anyway, by LNA_STATE_AS_GAIN (off by default) so that for those who like me prefer practicality over purity and don't loose sleep because the value displayed is 'not in dB' will continue to use CubicSDR efficiently. Call this pseudo-gain LNA to be clear. (Cubic like short acronyms in the Gains zone )
  For those who prefer toying with traditional buttons and menus like 90's HMI, the default LNA_STATE_AS_GAIN = 0 will content them, which means LNA state will only be available as a SoapySDR discrete setting using readSetting and writeSetting.

• Stop the "Gain reduction" nonsense, by simply reverting the IFGR of to be [Min gain .. Max gain] and labeled IF, where 20 would provide the smallest gain to the signal, and 59 the max for instance.

• Please don't introduce another nonsense with negative values in such increasing gain scales like [-20 dB to -59 dB]. Or the contrary. Whatever, you got my point :)

In other words :

• setGain(IF) and getGain(IF) : same source as IFGR of old, with an increasing scale [20, 59] maning an increasing gain effect
• Remove the conditional compile of RF_GAIN_IN_MENU, so the code is always there.
• Add LNA_STATE_AS_GAIN define : If LNA_STATE_AS_GAIN = 1 add : setGain(LNA) and getGain(LNA) using the reversed LNA state where 0 is the least aplification and N the value providing the max gain. Of course, include such gain in listGains.
• setGain(IFGR) and getGain(IFGR) -- kept for backwards compatibility, value ranges from 20 to 59, but not listed in listGains: agreed
• Don't keep setGain(RFGR) and getGain(RFGR) around at all because applications outside Cubic seemed to be confused by it anyway. Probably better to kill it entirely ?

@dlaw solution although interesting, seems complicated and not without drawbacks as @fventuri said.

And since @fventuri will probably be (too much) kind enough to try to implement all this, and worse support all the following bugs let's preserve his sanity :)

The incredible job he has done on the Duo support would have exhausted my patience long ago, so what about small quality of life changes this time ?

I believe that CubicSDR already automatically refreshes the gain slider values whenever a different setting (such as the frequency) is changed. (This is why the existing RFGR slider automatically updates itself when a different LNA state is selected from the menu.)

That was a hell to do it right : The gain sliders sort-of continously re-read the current gains by getGain(X) because indeed something, anything outside the sliders themselves could have change the actual gain value: in this case, the LNA state in the menu, but that could be litterally anything else because the code have to be generic.

The funny thing is if the gain sliders were not hidden when 'AGC on', you would probably see the displayed gain value change in realtime as adapted by the AGC algorithm.

[dlaw]

Thanks for all the thoughts @vsonnier. I agree, there is a lot to be said for putting LNA state in the menu and keeping it out of the gain sliders completely. That was actually my first attempt at solution for this problem (see #26).

I will be happy to do the work to implement any solution, as soon as there is some agreement about what I should do. So please all continue the discussion and let's try to find a consensus about what is best for the project! 😃

One area where I really disagree with @vsonnier is about the negative numbers. I think it is perfectly sensible for a positive gain reduction to equal a negative gain.

• IF gain reduction of 20 = IF gain of -20
• IF gain reduction of 59 = IF gain of -59

The alternative seems confusing to anyone who has used SDRPlay previously:

• IF gain reduction of 20 = IF gain of 59
• IF gain reduction of 59 = IF gain of 20 ???

At that point, since the shifting is arbitrary anyway, you might as well map it to an IF range from 0 to 39.

I don't think supporting setGain(RFGR) will confuse any applications as long as we keep RFGR out of the listGains() response.

So, here is a "simpler, quality of life" proposal based on @vsonnier suggestions:

listGains() -- return IF only (except also RF if compile time option is set)

setGain(IF) and getGain(IF) -- value ranges from -20 dB to -59 dB, equal to the negative of the IF gain reduction

setGain(RF) and getGain(RF) -- value ranges from 0 to 9, equal to LNA state (or negative LNA state? or 9 minus LNA state?)

setGain() -- set the IF gain only, leaving LNA state alone

getGain() -- return the IF gain only, ignoring LNA state

setGain(IFGR) and getGain(IFGR) -- kept for backwards compatibility, value ranges from 20 to 59

setGain(RFGR) and getGain(RFGR) -- kept for backwards compatibility (as long as it doesn't cause any issues with clients), value ranges from 0 to 9 and is equal to the LNA state

[vsonnier]

One area where I really disagree with @vsonnier is about the negative numbers. I think it is perfectly sensible for a positive gain reduction to equal a negative gain.
IF gain reduction of 20 = IF gain of -20
IF gain reduction of 59 = IF gain of -59
The alternative seems confusing to anyone who has used SDRPlay previously:
IF gain reduction of 20 = IF gain of 59
IF gain reduction of 59 = IF gain of 20 ???

If the gain is called IF this is a gain, not a Gain Reduction, like in IF**GR** for which we may keep the usual scale [20, 59] of Gain Reduction.

SDRUno now shows either "Gain Reduction" or "Gain" controls depending of the user preference.
If in "Gain" mode SDRUno indeed shows negative, increasing values for the scale then so be it let's align SoapySDR with that convention, else:

At that point, since the shifting is arbitrary anyway, you might as well map it to an IF range from 0 to 39.

Exactly.

Same arguments for the RF scale :)

Anyway I'm not that attached to any convention, but be assured that the first thing some users will ask is "We the hell the scale is negative numbers ?"