SoapySDRPlay3
SoapySDRPlay3 copied to clipboard
Override setGain and getGainRange to avoid changing RFGR.
SDRPlay's "RFGR" is actually a mode selector for the front-end LNA. It does not have units of dB. (In fact, the resulting gain is frequency-dependent.) As a result, the default gain allocation algorithm does not apply and only IFGR should be used for automatic adjustment of the overall system gain.
(See https://github.com/pothosware/SoapySDRPlay2/issues/60 for previous discussion, https://www.sdrplay.com/docs/SDRplay_SDR_API_Specification.pdf section 5.3 for the definition of RFGR.)
FYI, there is some discussion going on regarding the best path forward for #26, cjcliffe/CubicSDR#825, and the new-gain-controls
branch.
However, I believe that this PR is still appropriate for an immediate merge into master
-- it is a standalone fix for a bug in SoapySDRPlay behavior, and it is independent of any future changes to the gain controls.
Would it be possible to get this PR merged now, even as we continue discussion about the best long term solution for gains in the SDRPlay driver?
This change does not break any existing use cases, and I think everyone agrees that setting the overall system gain should not treat the LNA state number as a value in dB.
@dlaw Sorry for answering you only now, but during the day I have to work my regular job, so I can reply to these comments only after dinner (or on weekends).
I think that the "generic" getGain()
and setGain()
should be defined consistently, i.e. if a user sets the "generic" (to be somehow defined) gain using setGain()
, when they later call getGain()
it should return a value somewhat close to what they set (or at least 'semantically' close).
Looking at the current code for setGain()
in the new-gain-controls
branch (https://github.com/pothosware/SoapySDRPlay3/blob/new-gain-controls/Settings.cpp#L637), I see that it is defined as a combination of the IF gain (or gain reduction) and the RF gain (or LNA state).
I looked at the commit history for that code and I see that that approach was introduced by @SDRplay almost a year ago with this commit (https://github.com/pothosware/SoapySDRPlay3/commit/1da639a608a7d7eef305640ed39f1ceb29774ce6#diff-d3c542b2c3cb1d01a894f311212c4cf20d0fe52be1ed128b5ff2c475507e834f).
Since it was a year ago I honestly don't remember much about the rationale for this approach (perhaps it is similar to what SDRuno does, not sure), but I would like to hear @SDRplay's opinion of what he thinks the behavior of the "generic" getGain()
and setGain()
should be, before going in one direction or the other.
Franco
I think this PR makes sense to merge immediately as a stopgap measure while we continue the discussion about new-gain-controls
over in #27. The current behavior of setGain is so broken as to be unusable. This PR is the minimal change to fix the incorrect behavior.
I have some application (in my case, trunk-recorder
) which is not capable of setting named gains. My only option is to say I want a "generic" gain of X dB. The current driver implementation of setGain will change my LNA state and IFGR so that the sum of the 0-9 LNA state and 22-59 LNA state is X. This is basically guaranteed to set the LNA state to 9, making the application unusable.
For now -- until we have some smarter way to handle the LNA state as an RF gain in decibels -- I think the correct behavior is for generic setGain to set the IFGR value and getGain to return the IFGR value, leaving LNA state alone.
@dlaw I think there is something I am missing with your setup over there.
You are saying that your application trunk-recorder
can only use a "generic" setGain()
(i.e. one without a named gain control like IF
or RF
). Since that "generic" setGain()
is not currently implemented in the master branch, I am assuming you are using the new-gain-controls
branch (please correct me if I am wrong), where the implementation of the "generic" setGain()
should not have the problem of say setting the LNA state always to 9, since it is computed from a set of tables (https://github.com/pothosware/SoapySDRPlay3/blob/new-gain-controls/Settings.cpp#L642-L662) from a value in the range [0-28] (if I counted them correctly), which means that if you are using an RSPduo and call setGain(...,14)
, you would get an IF=45 and LNAstate=5, which at a quick glance seem reasonable half-the-way kind of settings.
Franco
@fventuri I have this problem with master
branch. SoapySDR provides default setGain
and getGain
implementations which have the behavior I describe unless they are overridden.
Here is the undesired code which forces LNA state to 9 on master
:
https://github.com/pothosware/SoapySDR/blob/master/lib/Device.cpp#L294
Here is an old issue describing the problem: https://github.com/pothosware/SoapySDRPlay2/issues/60
Since it seems like we still have quite some discussion ahead of us on new-gain-controls
, I think it makes sense to prevent generic setGain
from messing up the LNA state in the interim.