search

pothosware / SoapySDRPlay3

Soapy SDR plugin for SDRPlay APIv3
https://github.com/pothosware/SoapySDRPlay3/wiki
MIT License
98 stars 15 forks source link

New gain controls #35

Open fventuri opened 3 years ago

fventuri commented 3 years ago

Given the diversity of opinions on how the gain controls should work (see discussion here: https://github.com/pothosware/SoapySDRPlay3/pull/27), I spent some time over the weekend thinking about a possible way of dealing with all the different tastes and preferences, and I just pushed my changes to the new-gain-controls branch.

Basically I created four different 'models' of gain controls, selectable via a line in the cmake config file CMakeLists.txt (https://github.com/pothosware/SoapySDRPlay3/blob/new-gain-controls/CMakeLists.txt#L43).

The four models are:

To make it easier to work with all the different gain control models (and perhaps to add more if we like to explore other options), I pulled out of Settings.cpp all the code for the gain API, and reorganized it in four different source files, one for each of these gain control models (plus one for some common code related to the gain reduction tables from the SDRplay API specification document).

One final note about the gains in dB: I spent some time yesterday afternoon playing around with SDRuno, switching the RF gain display mode back and forth between 'Gain' and 'Attenuation', and I noticed that while the dB values for RF attenuations in SDRuno match exactly the values in the gain reduction tables in the SDRplay API specification, the same is not true when the RF gain display mode is set to 'Gain': the dB values in this case have decimals, their spacing is different than shown in the tables (for instance the gain reduction tables might have 7dB difference between two successive LNA state values, while the SDRuno RF gain changes 6.5dB between the same LNA states); the minimum and maximum values depend on the frequency band, the RSP model, and possibly other factors. In other words I have the impression that SDRplay have done a good amount of work in figuring out the real gains in dB (and those are displayed in SDRuno when the RF gain control is set to 'Gain'), while the figures from the gain reduction tables are just rough approximations, and it is not obvious to me how the two are related, i.e. how to compute the real RF gain from the LNA state (I searched around for a bit, but I couldn't find much information about these RF gain values). As a consequence the RF "gains" in dB of the third gain control model (the one I called 'DB') are not the real gains, and shouldn't be treated as such; they are just some arbitrary values in dB that "approximate" the behavior of the real gain, in the sense that increasing the RF gain control by 10dB increases the signal by about 10dB, but the RF gain value shown is purely arbitrary (I hope this rambling makes sense).

One last caveat when using CubicSDR with the new gain controls: CubicSDR stores the last used config under $HOME/.CubicSDR/config.xml (on Linux; there's probably a similarly named file on Windows and Mac); in my case this file had the last used gains controls as IFGR and RFGR, and threw a few error messages about invalid gain range when starting the RSP, because those names are different in some of the gain control models; if you see similar error messages, you may want to recreate the CubicSDR config.xml file from scratch (or edit it to change the names and values of the gain controls, if you don't want to lose the rest of the CubicSDR configuration).

@SDRplay, @vsonnier, @dlaw, @nmaster2042, please give it a try and let me know what you think (I just ran a few quick tests here streaming a local FM station, and they worked, but there are probably still bugs in the code).

Franco

dlaw commented 3 years ago

Wow! That is quite an amazing piece of work! Thank you @fventuri !

I will give it a test run sometime over this next week, but in the meantime, a comment about the "generic" setGain and getGain functions:

SoapySDR provides a default implementation of the "generic" setGain and getGain functions which SoapySDRPlay inherits: https://github.com/pothosware/SoapySDR/blob/master/lib/Device.cpp#L292

In the case where all gains are in dB, the default implementation is already correct and we do not need to implement these functions ourselves. (Well, technically we may wish to tweak setGain a bit to change how the gain is split between IF and RF, but the default is certainly good enough.)

It is in the other three cases where we do need to implement our own generic setGain and getGain in order to prevent the inherited setGain from always setting the LNA state to 9 in a misguided attempt to distribute the gain.

dlaw commented 3 years ago

I think the IF gain range from 20 dB to 59 dB is a bit of an anti-pattern, and if you must avoid the negative numbers, a range from 0 dB to 39 dB would be better. Here's why:

An IF gain range of -59 dB to -20 dB will be confusing to a user who does not expect negative numbers, but at least the correspondence with IFGR value will be obvious for any user who uses SDRuno or is looking at SDRPlay documentation: IF -20 is IFGR 20, IF -30 is IFGR 30, etc.

An IF gain range of 20 dB to 59 dB eliminates the confusion with negative numbers, but a user could easily look at the range from 20 to 59 and conclude that this IF gain is exactly the same as the IFGR value from the datasheet! Whereas I think few users will readily deduce that e.g. an IF gain of 30 turns into an IFGR of 49.

So, for the case where you don't want to use negative numbers, I think it is better for IF gain to go from 0 to 39. This makes it obvious that the IF gain is something different than the IFGR and removes the possibility for confusion.

I still think the negative gains are cleanest of all, but :shrug:

I will test out the options shortly and report back to confirm functionality and share any notes on usability.

Cheers! David

fventuri commented 3 years ago

@dlaw - many thanks for your comments and for giving it a try so quickly.

A few comments:

Franco

SDRplay commented 3 years ago

I've been reading the gain thread with great interest. Fundamentally the RSPs have 2 gain systems with a single hardware AGC system. How this is all implemented in SoapySDRPlay isn't something that you need my input on. All I would do is point out some things that we have found in the development of SDRuno over the last few years...

  1. The way that 90+% people use the system is to leave the IFAGC enabled and just use the RF gain slider. The new AGC scheme in the API 3 is way better than the one used in API 2 and doesn't "bounce" like it could do before.
  2. When the IFAGC is enabled, setting the IFGR has no effect.
  3. Given 1 and 2, setting a single gain value for the entire system doesn't work when IFAGC is enabled.
  4. SDRuno will always display the gain sliders going from min to max for the gain type and then deal with any translation internally. For example IFGR is shown from 20 to 59 and IF gain is also shown as 20 to 59 - just that internally the IF gain value is inversed to convert to IFGR before sending to the API
  5. The gain value displayed in SDRuno comes back from the API - unlike the gain reduction value which can be calculated.
  6. Specifying RF gain as a dB value doesn't really work - for different RSPs and LO frequencies there are different gain steps and different gain ranges. 0 - 9 might work for the RSP1A, but the RSPdx supports 0-27 in some circumstances, so 0 - 9 would only allow about 50% of gain control range. The RSP1 only supports 0 - 3 so the API will error if 4 - 9 is used.

One thing that I looked at doing once for the TCP server was to map each RSP/LO frequency combination to an appropriate IFGR + RGFR value for a given single gain value. Of course this only works for when the IFAGC is off, but if the IFAGC is enabled, you could disable it, set the IFGR+RFGR and then enable the IFAGC - this would have the effect of putting the user close to where they wanted to be. If they want total gain control then the IFAGC would need to be disabled. There were some other issues that occurred in that piece of work that meant the single gain value work was not completed or implemented.

If you wanted to mimic how SDRuno works then you would need to have 2 independent gain control systems that could be individually set to have AGC or not. Then some code that internally could report back to the user on what GR or gain has actually been set.

Also, I think the key thing is to remember what the target audience is for this library. It's developers and not end users. I personally would look at popular end user software, see how they have been developed to work in gain and then work out the best way to interface to that with minimal impact.

Just my two penneth.

Best regards,

Andy

fventuri commented 3 years ago

@SDRplay - thanks for the interesting and useful thoughts, Andy

I have a few personal considerations about your last comment, where you say that the target audience is developers and not end users.

I suspect that a SoapySDR module like SoapySDRPlay3 is mostly used within client applications like CubicSDR or gqrx; these SDR receiver programs cater to a wider audience than just developers (in my opinion), as opposed to a software like GNU Radio, where the average user is probably someone more technical and more familiar with the concepts of SDRs. In addition both CubicSDR and gqrx (and most of the applications that build on the SoapySDR framework) are by their own nature pretty 'generic' (i.e. not developed with a specific set of devices in mind, like SDRuno), and therefore they have to try to accomodate the many different controls (and quirks) of a disparate set of devices, from the $20 USB dongles to say a USRP, that can cost several thousand dollars, and everything in between. Because of this variety of SDRs, many of these applications choose to expose to the end user the same gain controls that the SoapySDR (and therefore SoapySDRPlay3) shows them.

This whole preamble is to say that I think we have to put ourselves in the shoes of the end user when making decisions on these controls: which controls could be more useful to them, easier for them to understand, or just "make sense".

On the other hand, as a developer of a module like SoapySDRPlay3, I am bound to the contract that is the SoapySDR API; namely in this case to the description of the arguments of setGain() and return value of getGain() in Device.hpp (https://github.com/pothosware/SoapySDR/blob/master/include/SoapySDR/Device.hpp):

\param value the new amplification value in dB

\return the value of the gain in dB

In closing, what I am trying to say is that whatever we end up choosing will be some sort of trade-off, and there will always be someone who won't be 100% happy with it. I personally like to experiment and play around with the different options, and I am really looking forward to your and the others feedback.

Franco

SDRplay commented 3 years ago

Hi Franco,

What I meant by that is that of course the end user will need to be able to set frequency, sample rate, gain, etc. however they will not care how that is handled by the SoapySDRPlay library. It is the job of the developer to understand these concepts and present them to the end user.

For example when we point people to CubicSDR and the SoapySDR framework, almost all of them won't know or care what the SoapySDRPlay library does, just that they can control the hardware through the CubicSDR interface.

For the large part, and I'm not saying this is you, what I've seen here is "how do we get the SDRplay gain control to operate like a RTL device does" - if that's the end goal then that's one thing, but that's not to understand what benefits the RSP gain control system has.

We have many thousands of customers that use various software and I would say a lot less than 1% of them want, or have the coding knowledge, to understand or use the SoapySDR framework in anything other than an end application.

From our experience you guys are the exception rather than the rule. If you see what I mean.

Best regards,

Andy

fventuri commented 3 years ago

@SDRplay - Andy, thanks for your always welcome advice and for keeping me (and this group) grounded down to earth, i.e. to the reality of the actual end users.

I followed your suggestion of taking a look at what the other popular SDR applications are doing regarding the gain controls for the SDRplay RSPs; since I think that 90% of the SDR end users run Windows, I looked at what is the approach for gains in the most popular SDR applications for Windows:

I also googled terms like 'sdrconsole gain controls', and there seems to be some confusion among the end users on how these controls work (see for instance here: https://sdrplay.com/community/viewtopic.php?t=3524 and here: https://sdr-radio.groups.io/g/main/topic/16932930). I found similar results when googling 'hdsdr sdrplay gain controls' (for example: https://www.sdrplay.com/community/viewtopic.php?t=356).

Finally I implemented yet another gain control model, that I called 'NO_IF_GAIN_CONTROL_ON_AGC' for lack of a better term; it is essentially the same as the 'DB' gain control model, except that I changed listGains() not to return 'IF' whenever the AGC is enabled, and the other relevant functions (getGainRange(), getGain(), setGain()) to return an error message back to the application when invoked with the 'IF' gain control if the AGC is enabled. I ran a few quick tests with CubicSDR, but it seems to get confused when the list of available gain changes (i.e. when I start it , it displays correctly both 'IF' and 'RF'; then, if I turn AGC on, they both disappear; after that, if I turn AGC back off again, I only see the 'RF' gain control, but it does not seem to act properly when I try to change RF gain). I pushed this latest gain control model too to the new-gain-controls branch in the repo, so you can try it out if you like.

Franco

vsonnier commented 3 years ago

Thanks @fventuri for your dedication. I gave some try on your branch, and the most usable was the LNA one from me (Ha!) Here are my random remarks, in no particular order:

About your last message:

I ran a few quick tests with CubicSDR, but it seems to get confused when the list of available gain changes

Good heavens, CubicSDR certainly didn't expect the gain list to change dynamically. I'll have a look If it can be changed when I have some time.

if I turn AGC on, they both disappear

Standard procedure for Cubic, yes. It has been discussed in the past if we can add configurable behaviour for the Gain controls, i.e choosing which one you want to hide if AGC is on. It is certainly possible to do so, but I'm lacking the motivation to do it myself. Is it not a niche feature anyway ? The problem also originates from SoapySDR which has no concept of AGC on per-gain, only globally. That would solve the issue to have per-gain AGC and it could be implemented generically in Cubic but 🤷

I only see the 'RF' gain control, but it does not seem to act properly when I try to change RF gain).

See the points above :)

fventuri commented 3 years ago

@vsonnier - many thanks for your feedback and useful suggestions!

I finally found some time this weekend to fix the problem with the settings selection for the RF Gain (or RFGR, or LNA, depending on the gain control model); it is now back to a string type (instead of an integer), so it should be easier to use, and the avalable selections now match the correct options for each of the gain control models. I just pushed the code to the new-gain-controls branch; please give it a try and let me know what I missed (or what doesn't work as expected).

On the AGC front, I knew it was a very tall order for CubicSDR (and probably for any other SDR application out there) to be able to deal with a list of gains that changes dynamically, but I was just too curious to see what would happen - so don't worry about try to handle a gain list changing dynamically, since I doubt any other SDR client would be happy about it anyway (I removed that gain control model with this latest commit). Another option that I am exploring now is to leave the IF gain control even when the AGC is enabled, but have it return a gain range (in getGainRange()) of just one value (the current value of gRdB) when the AGC is enabled. To play around with this idea, tonight I created a new gain control model called LNA_AGC, which is just a copy of LNA with this one change for the IF gain range. I ran a couple of very quick tests with CubicSDR, enabling and then disabling AGC, and it seems that after the AGC is back to disabled (i.e. the IF gain control is available), the IF gain slider only displays one value (probably the one that was fixed when the AGC was on), but it seems to be still changing the IF gain value.

Franco

fventuri commented 3 years ago

Working on the RF control via a SoapySDR setting yesterday, gave me the idea of a possibly better approach for handling IF AGC; instead of using SoapySDR hasGainMode(), setGainMode(), and getGainMode() functions, I decided to try to implement enabling/disabling IF AGC as a boolean SoapySDR setting (ifagc_ctrl), and leave getGainRange() unchanged from the original.

With this approach, and a small change to the CubicSDR source code (see below), I was able IMHO to have a much better user experience using the IF AGC with CubicSDR.

This new approach can be enabled by setting the cmake variable GAIN_MODE_IF_AGC_AS_SETTING to True in line 45 of CMakeLists.txt, and it is available only for the new 'LNA', 'DB', and 'Normalized' gain control models. The new code is in the usual new-gain-controls branch.

As per the CubicSDR source code change mentioned above, I had to change lines 1028-1034 of AppFrame.cpp (https://github.com/cjcliffe/CubicSDR/blob/master/src/AppFrame.cpp#L1028-L1034) from this:

    agcMenuItem = nullptr;
    if (soapyDev->listGains(SOAPY_SDR_RX, 0).size()) {
        agcMenuItem = newSettingsMenu->AppendCheckItem(wxID_AGC_CONTROL, "Automatic Gain");
        agcMenuItem->Check(wxGetApp().getAGCMode());
    } else if (!wxGetApp().getAGCMode()) {
        wxGetApp().setAGCMode(true);
    }

to something like this:

    agcMenuItem = nullptr;
    if (soapyDev->hasGainMode(SOAPY_SDR_RX, 0)) {
        agcMenuItem = newSettingsMenu->AppendCheckItem(wxID_AGC_CONTROL, "Automatic Gain");
        agcMenuItem->Check(wxGetApp().getAGCMode());
    } else if (!soapyDev->listGains(SOAPY_SDR_RX, 0).size()) {
        wxGetApp().setAGCMode(true);
    }

Before trying the new GAIN_MODE_IF_AGC_AS_SETTING with CubicSDR, you may have to either recreate your CubicSDR config file ($HOME/.CubicSDR/config.xml), or edit it to make sure that agc_mode is set to 0 (instead of 1).

Franco

fventuri commented 3 years ago

For the cases where the client application can only use the generic setGain() and getGain() functions (i.e. the ones without a gain control name), as for instance trunk-recorder mentionedby @dlaw, I thought today of a possible approach to convey both the desired RFGR value and the desired IFGR value in the so called 'Legacy' gain control model, similar to the way we specify (military) time: for instance when we say 1259, we mean 12 hours and 59 minutes (after midnight). Using this convention, a generic gain value of 1259 would mean: set the RFGR (LNA state) to 12 and the IFGR to 59. Also with this approach an IFGR of say 0 could mean to enable the AGC; therefore a generic gain value of 1200 would mean RFGR=12 and IF AGC on Setting/getting an RFGR/LNA state of 0 might be a little confusing though, because it would just be '0059', i.e. 59 (for the case of an IFGR=59). Also since the gain value in setGain() and getGain() is defined as a double, another possible option could be to pass RFGR and IFGR as 'RFGR.IGFR', for instance 12.59 in the case above; with this option the case where the RFGR/LNAstate=0 would be more explicit, because it would look like 0.59.

Tonight I went ahead and implemented both options for the 'Legacy' gain control model; you can try them by editing the cmake config file CMakeLists.txt, and changing line 46 where it says 'GAIN_MODE_LEGACY_GENERIC_GAIN'; the values can be:

The code with these options is in the usual new-gain-controls branch in the repository; play around with it, and let me know what you think.

Franco

dlaw commented 3 years ago

Thanks again @fventuri for implementing all of these different options. And thanks to @SDRplay for the insight about how users tend to adjust the gains. I played around with all the different options and here are some thoughts:

  1. GAIN_MODE_IF_AGC_AS_SETTING is a big improvement in user experience. It makes perfect sense to see IF gain still present, but automatically changing when IF AGC is enabled. :+1:
  2. With all the different issues about setGain and getGain, my personal preference is for LNA state to be a menu item only and for IF gain to be the only gain returned by listGains. Nice and simple. Users should already be familiar with the concept of visiting the menu for front-end settings, because that is also where we find bias-T control and the various notch filters.
  3. But some users will wish for the convenience of setting the RF gain using a RF gain slider... in this case, I find "DB" mode to be the most preferable. I think there are just too many issues to ever permit a gain slider that is not in decibels.

It seems that @fventuri has the code well in hand, but once it is decided which of these options to merge, I volunteer to write some documentation about gains for the SoapySDRPlay wiki :writing_hand:

Cheers!

fventuri commented 3 years ago

Thanks for your feedback @dlaw

This weekend I thought that with so many choices on the gain controls it is almost impossible to make everyone happy.

Perhaps a better approach would be to select the more sensible and useful models among them, and create a a new 'meta' setting called say 'gain_control_model' (or something like that), whose values could be something like 'Legacy', 'DB', 'LNA', etc. This meta setting would not be listed by getSettingInfo() to avoid applications like CubicSDR to expose it in their UI, but it would be available via the readSetting() and writeSetting() methods.

This way a more advanced user could set up the gain control as they prefer - they would have to just add an entry for that setting in the CubicSDR config.xml file with their preferred choice. Since the code in the new-gain-controls branch is already organized with one source code file for each gain control model, it wouldn't be too much work to implement this approach (and a user with enough source code experience could probably add their own gain control model very easily).

That's just an idea that popped up in my head; if there's a general consensus that it is worth trying, I could work on it this coming weekend.

Franco

nmaster2042 commented 3 years ago

@fventuri

Sorry, for late reply.

I see a lot of development on gain modes, how can I select one ? Is it at compile time ?

fventuri commented 3 years ago

@nmaster2042 - no problem at all - I have been busy these days too, so I understand you 100%

Yes, at this time it is a compile time option that you can set in the cmake config file CMakeLists.txt at lines 43-47 (or you can pass them as '-D' args in the cmake command line, for instance: cmake -DGAIN_MODE=LNA ..)

The different gain control models available at this time are described in detail in these comments: https://github.com/pothosware/SoapySDRPlay3/issues/35#issue-874111531 ('Legacy', 'SDRplay', 'DB', and 'LNA'), and https://github.com/pothosware/SoapySDRPlay3/issues/35#issuecomment-830989193 ('Normalized').

You may also want to to try out a new implementation of the IF AGC controls that in @dlaw and my opinion provides a much better user experience with the IF AGC enabled in CubicSDR. To try it, you'll have to set GAIN_MODE_IF_AGC_AS_SETTING to True in line 45 of CMakeLists.txt and rebuild CubicSDR after changing a few lines in the source file AppFrame.cpp (see this comment: https://github.com/pothosware/SoapySDRPlay3/issues/35#issuecomment-835839731)

Finally I just ported to the new-gain-control branch some code changes that @SDRplay suggested to avoid skipping frequency/gain reduction updates with applications changing them very frequently like OpenWebRX; I pushed these changes to master yesterday, and I thought I would port them here too to make it easier to merge this branch once we have decided what to do about the new gain controls.

Franco

olliehaffenden commented 3 years ago

Thanks to @fventuri for pointing me to this issue and for all the hard work and thought; thanks also to @dlaw, @SDRplay and @vsonnier for your contributions. As I wrote in a now deleted post here, I had run into this issue after discovering experimentally that the "gains" as reported by SoapySDR were in fact gain reductions.

My use case might be worth adding to your deliberations: I'm working on adding SoapySDR support to the Dream Digital Radio Mondiale receiver project, and I want to be able to measure and report the received signal power in dBuV at the antenna input. Dream already measures the power in-band in the digital signal, but needs to apply a correction for the amount of gain applied in the front-end SDR device. For this to work consistently, the overall gain reported by the "generic" getGain() function needs to be (a) in dB and (b) to include all the variable gains in the frontend. (It doesn't need to be the absolute gain - there's a "calibration factor" in the config file to convert the final figure to dBuV).

As I hope you'll agree, the only option that satisfies this use case is the "DB" model, and I'd even started implementing something like it myself before Franco thankfully pointed me here. I'll try out the DB model next and let you know how I get on!

Ollie.

olliehaffenden commented 3 years ago

Update: I've tried out this branch with GAIN_MODE set to "DB", and it's working perfectly. I tried all 7 of the LNA settings on the RSP1A and my signal strength measurement gives the same value (within a dB) over the whole range. I am most grateful at having been saved a lot of work!

The only surprise was that the "RF Gain setting", i.e. using writeSetting("rfgain_sel", ...), was also in dB, but requiring an exact match. For some reason I expected it to set the LNAState directly as in the Legacy model. I can see the argument for having the setting presented as meaningful options, but there's the difficult problem of how to deal with changes of frequency, which you have discussed above. I had thought that it could just keep the LNAState, but I'd overlooked the jaggedness of the tables, which means that a particular LNAState might no longer be valid following a frequency change. On the other hand, it's also likely that a given dB gain value wouldn't be valid in a different band.

In the end I undefined GAIN_MODE_DB_POSITIVE so that I could simply use the negative of the values in the table (I'm with @dlaw on this point!). I think that will be fine for my application.

@fventuri's suggestion of a meta-setting is interesting and might eventually be a better option than requiring compile-time settings.

Thanks again!

OIllie.

nmaster2042 commented 2 years ago

Hi.

Very sorry @fventuri, things didn't happened like I expected (personal case). I tried different gain modes you added and conclude that for my use, the DB is best for my use.

I agree with you for a meta "gain_control_mode", it would be better than compile time config in CMakefile ;-) (I'm little lost with all things that can be modified in CMakefile).

If this meta parameter is added but not exposed, how could it be used ? Could it be passed in manual device selection in Cubic for ex ?

fventuri commented 2 years ago

@nmaster2042 - no problem at all! I myself have been very busy with work in the last month, so I understand completely.

No, the code in the new-gain-controls branch is still the way it was, where the user has to select the gain control model at compile time.

Based on your suggestion, I think I'll create a new branch with the option to select the gain control model at device creation (i.e. through a parameter in the SoapySDR device string), with the default set to 'legacy' mode, so it wouldn't affect the existing users of CubicSDR.

Franco

nmaster2042 commented 2 years ago

@fventuri : Sure it will be the best option.

fventuri commented 2 years ago

@nmaster2042 @luarvique

I just pushed to the branch new-gain-controls (https://github.com/pothosware/SoapySDRPlay3/tree/new-gain-controls) a version of SoapySDRPlay3 with the suggested change to use a setting (called gain_ctrl_mode) to allow the user to select a different gain control mode, without having to change CMakeLists.txt and without having to rebuild this module.

There are currently four options available:

All the gain control modes above except for the first one ('legacy') also offer a 'generic' setGain() (i.e. without a gain name) method for client applications like OpenWebRX; details of these methods and how the generic setGain() is implemented are in the comments in the GainControls.cpp source file (https://github.com/pothosware/SoapySDRPlay3/blob/new-gain-controls/GainControls.cpp).

Tonight I ran a few tests with each one of these gain control modes using the application CubicSDR, and it seems to me that they work as expected; feel free to run more tests, and tell me if you see any problem.

I also renamed the old 'new-gain-controls' branch (the one where you had to rebuild every time you wanted to change gain control mode) to new-gain-controls.old, in case you want to compare between the previous implementation and the current one (for the 'db' mode).

Let me know what you think, and I really apologize that it took me so long to get this new code out.

Franco

luarvique commented 2 years ago

Hello, Franco!

It is very nice to see that you have finally got to do something about setGain(). I am somewhat disappointed though that you have not implemented the mode I suggested, where setGain() controls IFGR, while RFGR is left alone, set with a separate (already existing) parameter.

I am going to try your changes, but since MSI.SDR has been shown to use completely different units to control RFGR, it is unlikely that any of the highly elaborate models you implemented are going to make any sense for my use case.

luarvique commented 2 years ago

So, since the effective gain now depends on the frequency, why aren't you updating gain settings in setFrequency()?

Also, what happens if a previously set gain is out of range for a newly selected frequency?

fventuri commented 2 years ago

@luarvique - thanks for the feedback!

First of all I didn't mean to disappoint you; I just implemented these four types of gain controls in the last week or so, but now that the C++ class infrastructure is in place (it took me a few tries to come up with the current approach), I think it would be pretty straightforward to add another gain control mode based on the IF gain (or gain reduction); I have been really busy with work recently, but I think this weekend I should be able to find the time to do it, so you can give it a try and compare it with the other methods.

A few more notes:

Franco

luarvique commented 2 years ago

I think it would be pretty straightforward to add another gain control mode based on the IF gain (or gain reduction); I have been really busy with work recently, but I think this weekend I should be able to find the time to do it, so you can give it a try and compare it with the other methods.

Thanks,, that would be hugely appreciated. I have also submitted a pull request with the appropriate changes, although they are an order of magnitude less elaborate than what your current implementation is.

* you're right that the RF gain reduction depends on the frequency, however it actually depends on the frequency 'band', and perhaps in many (or even most) cases the user does not change the frequency band very often

Be it frequency or band, you are still creating a situation where user changes to the frequency inadvertently break their previously chosen gain value. There is no way to explain this as a normal, logical behavior to a user. Hence, you probably want to store the last selected (or the default) gain value somewhere and call setGain() on frequency changes.

I suppose this situation does not apply when the user sets both RFGR and IFGR explicitly, since they probably know what they are doing. It also does not happen if only IFGR affects the overall gain, since IFGR meaning is constant and well defined.

* the other problem with some LNA states not being valid any longer if the user changes frequency band would also present itself with say a setting of `rfgain_sel` that was used to select the LNA state; in that case too a change of frequency to a band where the value of `rfgain_sel` is no longer valid could cause problems.

This is true, but the rfgain_sel value is not treated as actual generic gain. It has no implicit semantics, but is rather some weird setting that is allowed to behave any way it likes. A magic switch, so to say.

  * this module SoapySDRPlay3 uses the SDRplay API and service which are proprietary software licensed to be run only with legitimate SDRplay devices; I am not a lawyer, but I suspect that using SoapySDRPlay3 with your MSI.SDR, you might be in violation of that license

Well, I doubt that this particular license term is enforceable. As to the ethics of the MSI.SDR usage, the manufacturer does not claim this device to be an SDRPlay or its clone. Anyone can take the same two MSI chips and connect them according to the datasheet.

Before we continue down this slippery quasi-legal slope, I also own a real, genuine RSPduo which is, of course, a much better device than MSI.SDR. It just doesn't fit the use case where MSI.SDR is being used. 

  * If I were you for the MSI.SDR I would rather look at the SoapyMiri module (https://github.com/ericek111/SoapyMiri), since it uses the libmirisdr library

I will probably look into that, but there are only so many open source packages I can fix in the remaining free time, and I am kind of hitting this limit here.

PS: Speaking of fixes, there is a partial deadlock in the current SoapySDRPlay3 code. Please, see pull request #59.

nmaster2042 commented 2 years ago

@fventuri I built the new branch and made tests with CubicSDR. For me it's a good improvement to be able to chose gain control method without rebuilding driver. your work is much appreciated. Thank you for time spent in this project.

ericek111 commented 2 years ago

Thanks for mentioning me here. I agree that sometimes more control over the device's gains is required, and sometimes it's not. The "sweet spot" heavily depends on your antenna (to not get into overload) and on the frequency. And since there are 4 gain controls, it's not trivial to make it right (this thread is an evidence of that).

The libmirisdr driver by Miroslav Slugeň can work in both modes, with one slider approximating the real attenuation, or with 4 sliders (or less, if combined) for each stage separately: https://github.com/ericek111/libmirisdr-5/blob/master/src/gain.c#L20 I found one problem with that approach -- it automatically switches on the LNA and the mixer amplifier upon reaching 43 dB, which gives me worse SNR. I have implemented these separate controls initially, then decided against them for ease of use, then put them back in. You guys are actually doing it exactly the same way -- adding a setting to control the gain calculation method.

Looking at the code, it's kind of what I had in mind, except I believe these opinionated decisions should be handled by the (painfully unstable, insecure by design, closed-source and not portable) driver itself.

jketterl commented 2 years ago

details of these methods and how the generic setGain() is implemented are in the comments in the GainControls.cpp source file (https://github.com/pothosware/SoapySDRPlay3/blob/new-gain-controls/GainControls.cpp).

I was requesting documentation for the actual customers / users / operators of these devices. I think this needs more explanation than just inline documetation in a source file. Please address, if you want this to succeed, you need to be able to explain in plain and simple words what happens, otherwise this will only lead to more confusion.

  • you're right that the RF gain reduction depends on the frequency, however it actually depends on the frequency 'band', and perhaps in many (or even most) cases the user does not change the frequency band very often (and if they do, they could perhaps refresh the RF gain reduction by just changing it slightly, 1 dB up and down). Case in point with OpenWebRX I think the typical usage would be to say provide a profile for the whole 2m band (144MHz-148MHz), and in that scenario the RF gain reduction/LNA state would always be inside the same frequency band; in other words the same LNA state would yield the same (or very similar) RF gain reduction at one end of the band at 144Mhz as it would at the other end at 148MHz

I think you've picked out a very use case here. What about users that hook up their SDRs to dualband antennas for 2m and 70cm? What about using a diplexer or wideband antennas to get even wider coverage?

The user in the OpenWebRX context does not have the ability to adjust the gain, so please, for the love of consistency: If the gain is a function of frequency, adjust the gain when adjusting the frequency.

fventuri commented 2 years ago

Thanks for your useful and interesting comments @luarvique @nmaster2042 @ericek111 @jketterl Thanks for trying it out and for the nice words @nmaster2042

Tonight I took a quick stab at implementing the IFGR_ONLY gain control mode (which I just called IFGR), and it wasn't too much work; I ran a quick test with CubicSDR here, and it seemed to work OK, so I just pushed the code to the usual new-gain-controls branch (https://github.com/pothosware/SoapySDRPlay3/tree/new-gain-controls); please give it a try and let me know how it goes.

It's getting late here and I do not have much time and energy left to write down my thoughts; just a quick comment on the documentation request: all these gain control modes are still a work in progress (that's why they are not in the master branch), and at this stage I don't expect the 'casual' user who installs from a distribution package to use this code; this is just for those who want to experiment, enjoy trying new things (that can possibly break), and don't mind looking at the source code to figure things out. I also imagine that, after some discussion here, several aspects of these gain control modes will change (perhaps some of them might not prove useful after all). When all is said and done (if we'll ever get there some day LOL), I plan to add some simple usage notes in the Wiki to avoid confusion (including some warning about changing frequency bands).

Franco

luarvique commented 2 years ago

Hello, Franco!

I've submitted a pull request to OpenWebRX, adding support for the gain model selection and a few other options present in SoapySDRPlay3:

https://github.com/jketterl/openwebrx/pull/317

nmaster2042 commented 2 years ago

Hi Franco.

Tried on CubicSDR, it works fine here too.