pothosware / SoapySDRPlay2

Soapy SDR plugin for SDRPlay
MIT License
51 stars 11 forks source link

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

Closed dlaw closed 9 months ago

dlaw commented 4 years ago

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
vsonnier commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

vsonnier commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

Cheers!

fventuri commented 2 years ago

@olliehaffenden - thanks for your comment and hard work.

I started a similar discussion for the current version of the SoapySDRPlay module here: https://github.com/pothosware/SoapySDRPlay3/issues/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 commented 2 years ago

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!