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

Bring SDRPlay gain handling in line with SoapySDR abstractions. #61

Closed
wants to merge 4 commits into from
Closed

Conversation

dlaw
Copy link

@dlaw dlaw commented Nov 4, 2019

This PR solves #44 and #60. Summary of changes:

  1. Rename IFGR to IF, and negate it. (For example, an "Intermediate Frequency Gain Reduction" of +30 dB is now interpreted as an IF gain of -30 dB.)
  2. Remove RFGR from the list of gains. Instead, RFGR is adjusted using the equivalent "RF Gain Select" setting. (RF can be re-added to the list of gains via an optional build flag.)
  3. Override setGain and getGainRange so that setting an overall system gain will adjust the IF gain only. RF gain should not be adjusted implicitly.
  4. Rename the rfgain_sel setting to lna_state for semantic consistency.

The impact on CubicSDR clients will be the following:

  1. CubicSDR in manual gain mode will have an IF gain slider labeled -59 at the bottom and -20 at the top, with the greatest amplification obtained at the top of the slider. This replaces the old IFGR slider, which was backwards from usual (it was labeled 20 at the bottom and 59 at the top, with the greatest amplification at the lowest setting).
  2. The RFGR slider is no longer present in manual gain mode. The RF gain is settable in any mode by means of the "RF Gain Select" menu item. A build option (off by default) adds an RF gain slider in manual gain mode. RF gain values on this slider are negated from the previous RFGR slider (-9 through 0 instead of 0 through 9) for directional consistency with the other gain sliders.

This PR will require that any other applications which explicitly set IFGR, RFGR, or rfgain_sel by name must update their configuration files. It will introduce compatibility with any application that calls Device::setGain without a name.

I have built and tested this PR with an RSP1A, using CubicSDR (to verify manual and automatic gain control work properly) and trunk-recorder (to validate that Device::setGain with no name now has reasonable behavior).

References:

  1. Block diagram of RF and IF gains: SDRPlay application note
  2. Actual dB losses caused by RF Gain setting: SDRPlay API documentation, section 5.3

Settings.cpp Outdated Show resolved Hide resolved
@vsonnier
Copy link
Contributor

vsonnier commented Nov 4, 2019

Remove RFGR from the list of gains. Instead, RFGR is adjusted using the equivalent "RF gain" setting.
The RFGR slider is no longer present in manual gain mode. The RF gain is settable in any mode by means of the RF gain menu item.

Well, that was set precisely for Cubic because it was way more practical to use a "gain" slider than constantly going to the menus. The menu entry was kept simultaneously because in case of AGC on, the gain sliders were hidden in Cubic.

OK for the the rest of the changes, but I recomend keeping the setGain/getGain("RF") with a #ifdef, disabled by default in CMakeLists.txt if you wish.

@vsonnier
Copy link
Contributor

vsonnier commented Nov 4, 2019

Also you should put @SDRplay in the conversation about how relevant those changes are given those are for API v2 and there is already a working implementation of API v3 mentionned #58 on which it may be more relevant to work.

@dlaw
Copy link
Author

dlaw commented Nov 4, 2019

how relevant those changes are given those are for API v2 and there is already a working implementation of API v3 mentionned #58 on which it may be more relevant to work.

I'll be happy to port these changes to the v3 SDRPlay API once they are merged for v2. (Alternatively, if v3 is merged to master first, then I will rebase my PR.) For now, I'd just like to get the gain situation fixed.

@dlaw
Copy link
Author

dlaw commented Nov 4, 2019

it was way more practical to use a "gain" slider than constantly going to the menus. The menu entry was kept simultaneously because in case of AGC on, the gain sliders were hidden in Cubic.

For anyone else following along, this is how CubicSDR manual gain mode looks right now:
before

And this is how it would look with my proposed change:
after

There is definitely a tradeoff here between adherence to the SoapySDR abstraction, and ease of adjustment in CubicSDR's manual gain mode. Question for @vsonnier -- as someone who uses the RFGR slider in manual gain mode, would you prefer to have the existing "RFGR" slider which adjusts from 0 to 9 (with the highest gain at the bottom), or an "RF" slider which is inverted and runs from -9 to 0?

I still feel a bit uneasy about exposing a SoapySDR gain which is not in dB. (In fact, it's not even monotonic in all cases... on the RSP1, RFGR 0 -> 0 dB, RFGR 1 -> -24 dB, RFGR 2 -> -19 dB, and RFGR 3 -> -43 dB.) But a compiler flag to make it optional, especially one that's off by default, could be a solution.

In the long term, I'd love to figure out some way for CubicSDR to display UI widgets for arbitrary additional device settings. Then you could have an RF gain adjustment widget that would be visible all the time, in automatic or manual mode :-)

Cheers!
David

@vsonnier
Copy link
Contributor

vsonnier commented Nov 4, 2019

as someone who uses the RFGR slider in manual gain mode, would you prefer to have the existing "RFGR" slider which adjusts from 0 to 9 (with the highest gain at the bottom), or an "RF" slider which is inverted and runs from -9 to 0?

Neither :) Make it simple so that 0 means the lowest LNA gain (the old 9) and 9 the highest gain (the old 0)

But a compiler flag to make it optional, especially one that's off by default, could be a solution.

Yes, please.

In the long term, I'd love to figure out some way for CubicSDR to display UI widgets for arbitrary additional device settings. Then you could have an RF gain adjustment widget that would be visible all the time, in automatic or manual mode :-)

I wish it too, I studied the problem when I bought my RSP2 but gave up and has stayed on the easy side, exposing LNA state as "Gain".

@dlaw
Copy link
Author

dlaw commented Nov 4, 2019

Neither :) Make it simple so that 0 means the lowest LNA gain (the old 9) and 9 the highest gain (the old 0)

I am afraid that this would be too confusing for SoapySDR users who are not hands-on with the SoapySDRPlay source code. (Imagine: CubicSDR RFGR goes from 0 to 9, the SDRPlay documentation has RF gain performance tables that go from 0 to 9, but we have decided to shuffle around the meaning of 0 and 9!)

I'll wait a few days to hear from any other contributors or users who would like to weigh in on this design question, and will plan to update the PR this weekend.

@dlaw
Copy link
Author

dlaw commented Nov 4, 2019

Previous discussion about the RFGR gain slider vs menu item may be found in #35

@vsonnier
Copy link
Contributor

vsonnier commented Nov 4, 2019

the SDRPlay documentation has RF gain performance tables that go from 0 to 9, but we have decided to shuffle around the meaning of 0 and 9!)

Why not ? this PR is about reversing (GR)IF, is it not ? Whatever [-9 ; 0] is just as good, and user will have to adapt anyway to this convention so the same questions will inevitably araise in the long run among the existing users.

Previous discussion about the RFGR gain slider vs menu item may be found in #35

Indeed. The goal here would be to have "RF" presented as settings all the time (so removing the current #define RF_GAIN_IN_MENU), with an additional #ifdef RF_AS_GAIN (defaulting to "off" on CMake side) for instance bringing back RF as a Gain slider for Cubic users typically.

@dlaw
Copy link
Author

dlaw commented Nov 4, 2019

Why not ? this PR is about reversing (GR)IF, is it not ?

Well, I have two goals for redefinition of the gain ranges:

  1. Comply with standard SoapySDR abstractions, so that a higher gain number results in more amplification and the CubicSDR slider is not backwards.
  2. Maintain a direct correspondence with SDRPlay documentation (and other tools such as SDRUno), to avoid confusion among new users.

The nice thing about using the negative number range is that it (mostly) satisfies both goals.

@vsonnier, thanks for taking the time to talk through this with me! Looking back I can see you have done a lot of work for SoapySDRPlay and CubicSDR in the past. Your contributions are appreciated :-)

@dlaw
Copy link
Author

dlaw commented Nov 7, 2019

PR revised per discussion. I edited the description above to have an up-to-date summary of the changes.

Cheers!

dlaw added 4 commits August 15, 2020 12:03
SDRPlay's "RFGR" is actually a mode selector for the front-end LNA.
It does *not* have units of dB. (In fact, it is frequency-dependent.)
As a result, only IFGR should be used for adjusting the overall
system gain.
SoapySDR gains are expected to be positive for increased amplification.
The cleanest way to handle this is to treat the SDRPlay gain reduction
value as the negative of the SoapySDR IF gain.
The SDRPlay LNA state is a setting which is:
1. Not actually a gain in dB
2. Not adjusted by automatic gain control

We will always show it as a setting (e.g. a CubicSDR menu option).
Users can enable the LNA_STATE_AS_GAIN build option if they wish
for it to be duplicated as SoapySDR gain (the "RF gain").
For clients which call setGain() or getGainRange() without a name,
we will assume that they are getting or setting the IF gain.
@fventuri
Copy link
Contributor

@olliehaffenden - thanks for your comment and hard work.

I started a similar discussion for the current version of the SoapySDRPlay module here: pothosware/SoapySDRPlay3#35
I also created a branch for that module called new-gain-controls to experiment with some ideas on how to improve the gain controls for the SoapySDRPlay3 module, without affecting too heavily existing applications like CubicSDR and their users.

Please feel free to try using the different options offered by that branch of the SoapySDRPlay3 module, and let us know what you think.
I would suggest to use that discussion thread for your input in order to try to keep this important conversation in one place.

Franco

@olliehaffenden
Copy link

Thanks @fventuri for your quick response - I stumbled on the SoapySDRPlay3 development just after I'd posted and, realising I should have been posting there, I deleted my post here. Sorry to any other readers for the resulting confusion. I will take a look and make any comments - perhaps you've solved my problem already!

@dlaw dlaw closed this by deleting the head repository Jan 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants