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

Customising gain sliders, etc. #825

Open
SDRplay opened this issue May 28, 2020 · 20 comments
Open

Customising gain sliders, etc. #825

SDRplay opened this issue May 28, 2020 · 20 comments
Labels

Comments

@SDRplay
Copy link
Contributor

SDRplay commented May 28, 2020

@vsonnier for some reason I don't have your email address - can you drop me a note at [email protected]

@fventuri and I have been making some changes to the way that SoapySDRPlay works Re: gain control and I just want to understand what custom options there are in CubicSDR for things like sliders and the device selection.

Is there a webpage somewhere that describes how the system works?

Happy for you to close this as it's not technically an issue :-)

Thanks,
Andy.

@vsonnier
Copy link
Collaborator

vsonnier commented May 28, 2020

@SDRplay The sliders are both for setting and reading the current gain by setGain and getGain APIs. As explained for example here : pothosware/SoapySDRPlay3#61 it was to expose the RF select control as a "gain" which is more practical.

However, what happens if the user changes the RF select with the menu ? Because CubicSDR is generic, there is no way it can keep the RFGR slider in sync except by re-reding the current value by getGain.

That is why both getGain and setGain are used in the sliders management. Roughly getGain is called regurarly to display the current gain value, by a similar method the Sound/Notch slider is updated.

But there is a catch: This is also used by the IFGR slider of course... so enter AGC.
Consider the following use case:

  • AGC is off: User change the 'manual' gain to 25. (setGain(25))
  • User switch to AGC on, play with the device. AGC adapts the IFGR gain to 50.
  • User switch back to AGC off. There are now 2 possibilities:
    A) Re-read the latest gain by getGain blindly and so update the current slider value by the one AGC settled, i.e 50. (getGain() == 50)
    B) Force back the latest "manual" gain the user previously set, by re-set the gain with setGain(25) so the slider (and current) IFGR gain is restored to 25.

Both behaviours are valid, and I chose B) to code the setGain and getGain behaviour of the SDRPlay. If the current code changed that to A) it is just a matter of taste and we can keep it.

@vsonnier
Copy link
Collaborator

vsonnier commented May 29, 2020

In the CubicSDR architecture, anything that is listed as "gain" by SoapySDR listGains is translated into a gain slider. All gain sliders becomes hidden when AGC is ON, assuming AGC overrides the manual gain controls so that displaying them is irrelevant.

What I could do quite cheaply is to add a toggle option in 'Display' settings to hide the sliders or not when AGC is on. Because of the getGain refresh, we would see the gain moving in relatime when AGC is ON that way.

Or not: it depends on the device getGain ability to actually read the current applied AGC gain.

I quickly checked getGain in the available SoapySDR modules source code and indeed most of them are likely to only report the last setGain requested.

In this case, we would not see the sliders moving with AGC on, and the displayed value would be misleading.

However, we may try to add a visual cue on the sliders display like a pulsating "AGC on" overlay, so that the user knows the AGC is on...

I'll try to toy with those ideas when I have some time.

@SDRplay
Copy link
Contributor Author

SDRplay commented May 29, 2020

I've made changes to SoapySDRPlay to accept the following...

setGain using IFGR, IFGain, RFGR or RFGain and also now support 0-29 gain value (similar to RTL device)

So the way these work is that gain is gain and GR is gain reduction

We need the ability to display either IFGain and RFGain as sliders when IFAGC is disabled OR just RFGain slider when IFAGC is enabled

That's really it.

@SDRplay
Copy link
Contributor Author

SDRplay commented May 29, 2020

I haven't pushed the changes yet, I'll do that now

@SDRplay
Copy link
Contributor Author

SDRplay commented May 29, 2020

https://github.com//SDRplay/SoapySDRPlay

The result of this is that there are now 4 sliders! Having the ability to turn them on/off individually would solve the problem. If you think about it, a system that allows individual gains to be named should really not assume that enabling AGC affects them all.

I'm interested to know what people make of the support for setGain with a single value (in the range of 0 - 29) where 29 is max gain. Might make using other software that support the SoapySDR framework a bit easier to use with a RSP - that's the idea any way.

Best regards,

Andy

@vsonnier
Copy link
Collaborator

The result of this is that there are now 4 sliders!

No suprise here, you declared 4 gains :

results.push_back("IFGR");
results.push_back("IFGain");
results.push_back("RFGR");	
results.push_back("RFGain");

Why not keeping only IFGain and RFGain then ? No need to be too complex here, let's match the SDRUno default behaviour where gains are 'gains', not GR.

If you think about it, a system that allows individual gains to be named should really not assume that enabling AGC affects them all.

Agreed, alas the SoapySDR API only supports one AGC setting per-direction (RX or TX) not per-gain:
virtual void setGainMode(const int direction, const size_t channel, const bool automatic);

We need the ability to display either IFGain and RFGain as sliders when IFAGC is disabled OR just RFGain slider when IFAGC is enabled.

That would be the idea, however it would need some work adding in Display options to hide or not some sliders. Problem is, it is device-dependent.

A simpler solution would be what I suggested earlier : Generic option to hide/display all gains + some overlay info on sliders when AGC is on.

@SDRplay
Copy link
Contributor Author

SDRplay commented May 29, 2020

I left IFGR and RFGR for backwards compatability - there will be some applications that are already using them.

Having the ability to turn on/off the slider display for individual named gain ranges in CubicSDR will solve the problem I believe.

Andy

@fventuri
Copy link

A few thoughts here:

  • I just looked at the source code for a few SoapySDR drivers from pothosware (RTLSDR, AirspyHF, and UHD), and it looks like they all define gains as a variable where a lower value means less signal and a higher value means more signal, so I would stick with that convention. I also saw that their gain names are short mnemonics like 'TUNER', 'IF1', 'RF', 'LNA', or 'PGA' (for the UHD), so again I would just suggest to use simple strings like 'IF' and 'RF'. Looking at what they return in 'getGainRange(name)', I see that in a few cases the ranges they return have a negative value for the lower bound (for instance AirspyHF can return a range of [-48,0]); based on that I would suggest to make say getGainRange('IF') return the gain range [-59,-20]
  • I also saw that in a case (the AirspyHF when the boolean 'hasgains' is false), the SoapySDR driver returns a range of [0,0]; this gave me an idea on how to possibly deal with the AGC issue: SoapySDRplay will return for getGainRange('IF') a range of [0,0] (or perhaps just 'null') when the AGC is enabled, and CubicSDR could be changed to either hide or gray out any gain control whose range is [0,0] (or null), since after all it doesn't make sense anyway to show a slider for something that has no range.
  • (this one is more of a workflow thing) I think major changes and experiments like this one should not go into master in the 'SoapySDRplay' repository, but they should be developed in its separate feature branch (for instance 'feature/new_gain_controls'), and then eventually merged into master once we are all comfortable with them, and they have been tested; this way we avoid 'surprises' like some user downloading the code from master, compiling it, and finding out that CubicSDR has now four different gain controls for their RSP. If you agree, I can help rearrange master and the new branch tonight after work.

Franco

@vsonnier
Copy link
Collaborator

vsonnier commented May 29, 2020

(this one is more of a workflow thing) I think major changes and experiments like this one should not go into master in the 'SoapySDRplay' repository, but they should be developed in its separate feature branch (for instance 'feature/new_gain_controls'), and then eventually merged into master once we are all comfortable with them, and they have been tested; this way we avoid 'surprises' like some user downloading the code from master, compiling it, and finding out that CubicSDR has now four different gain controls for their RSP. If you agree, I can help rearrange master and the new branch tonight after work.

Cannot agree more on this. I don't know when I have time to add related improvements to CubicSDR so better not add experimental features that will persist in master an indefinite period.

@SDRplay
Copy link
Contributor Author

SDRplay commented May 29, 2020

I don't really care where changes go. The API is in beta, the repo I added the changes to effectively is still in beta, so just make sure we're not creating an beta alpha lol - just don't put them in a branch - because if our support system is anything to go by - NO ONE outside of developers will know how to git clone a branch - so if you don't want any one to test it then a branch seems a perfectly good place for changes to go! :-)

This is why I didn't want any changes to go into pothosware repo until the API was released and the version of SoapySDRPlay in SDRplay repo was stable. But seriously, put the changes wherever you feel they need to go.

@fventuri
Copy link

Thanks Andy and Vincent.

I rearranged the SoapySDRPlay repo so that now master has the 'stable' version and a new branch called 'new-gain-changes' (I can easily change its name to whatever you like) contains the IF and RF GR/gain changes Andy was trying out. Since I used the command 'git reset' to realign these two branches, I'd suggest that you re-sync your local copy from scratch (i.e. mv SoapySDRPlay SoapySDRPlay.old; git clone [email protected]:SDRplay/SoapySDRPlay.git), and check to make sure everything looks OK.

A few thoughts about your comments earlier:

  • honestly I don't think SDRplay support system should support anything but what's in master; if some customer decides to clone the branch called say 'experimental-new-controls', build it, and use it, they should know they are 100% on their own
  • cloning from the new branch is pretty easy actually:
git clone -b new-gain-controls https://github.com/SDRplay/SoapySDRPlay.git

that's al it takes

  • having a well organized repository will also help in the future - say you or your developers are working on featureX, while I am working at the same time on fixY, and Vincent is working on featureZ, then we'll be able to work independently without stepping on each other toes, and when we are done and good to go, we can merge our changes into master - after all, this is what Version Control Systems (VCS) like git are for (otherwise we could just exchange a bunch of zip files over floppies, like it was the 90s :-))

Franco

@SDRplay
Copy link
Contributor Author

SDRplay commented May 30, 2020

honestly I don't think SDRplay support system should support anything but what's in master
cloning from the new branch is pretty easy actually

haha - I love the optimism Franco. I actually hate version control systems because they are developed by coders (who in my opinion shouldn't be allowed to design any system LOL). Version control systems are no longer just used by developers, they are a means by which software is distributed and I guarantee you that they confuse the heck out of non-coders who just want to install software. How many people found the API3+RSPduo branch that you worked very hard on - very few and it means software doesn't get the level of testing it requires, it's just a fact I'm afraid. It just means that those of us that know where this branch is and what it is called, and how to clone it, need to test it that bit harder. Give me floppy disks any day :-)

Remember, we're not using master to develop SoapySDRPlay, we're using it to get as many people as possible to test it. Branches do not do that. Just my twopenneth.

btw I agree that IFGain and RFGain could be, and should be, shortened to IF and RF - in fact I've got another idea about RFGR and RF gain... but this is going off topic for this thread. I'll let you know what it is.

I appreciate the work that you guys do.

Best regards,

Andy

@vsonnier
Copy link
Collaborator

vsonnier commented May 30, 2020

I left IFGR and RFGR for backwards compatability - there will be some applications that are already using them.

Shower thought (actually restroom, let's pretend it is shower),
make some cheap "backward compatibility' that way:

  • listGains returns the new names only IF and RF listing the non-GR gains. This is what a well written application should use dynamically to 'discover' gains.
  • make setGain and getGain work for both old and new names, i.e set/getGain("IFGR"/"RFGR") will continue to work for applications that have hardcoded such API calls.

@SDRplay
Copy link
Contributor Author

SDRplay commented May 30, 2020

sounds like a plan 👍

@vsonnier
Copy link
Collaborator

Remember, we're not using master to develop SoapySDRPlay, we're using it to get as many people as possible to test it. Branches do not do that. Just my twopenneth.

For the clean v2.13 Soapy module, we still have issues open regularly because users don't know how to compile or use properly, so we don't need more explanation about a master apparently not working, 'please use another revision by Git'...etc.

I think @fventuri look clean and stable, and as soon as the present SDRPlay version is considered OK make it the default version in Pothos repo so that is become standard among the ecosystem.

Still, I can predict a new stream of issues about supposed 'non working module' because of the new service-based driver that will change users's habits.

Now if advanced users want a really newer and experimental module, they will be able to use a cutting edge SDRPlay repo one.

@SDRplay
Copy link
Contributor Author

SDRplay commented May 30, 2020

we shouldn't push this to pothosware until the API 3.x is released on non-Windows platforms. That will cause a world of issues.

this is why we're in the position we are in - we are trying to encourage people to use the SDRplay one so that we can gather data. It should not be hidden away and then just pushed to pothosware without a LOT of testing - that's the dilemma we're in with using master vs branches.

The open issues on the pothosware repo are to do with gain control, not building issues, maybe they are being raised on CubicSDR issues? Please forward them to the SoapySDRPlay issues and I'll be happy to deal with them.

I would consider pothosware version released, SDRplay master as beta and now the SDRplay branch is alpha

Unfortunately we need data on all three to make sure we're doing the right thing. I know I don't have all the answers here, so I'm happy to go with the flow as long as are moving forward 😃

@vsonnier
Copy link
Collaborator

@SDRplay I understand, you prefer waiting for all the bugs to be ironed out. In this case, the issue support will on you, and I'm glad for that :)

What I was suggesting was just a way to offer support to the new RSPs faster by releasing sooner a 'stable' version to the the general public.

It is your choice to prefer a 'when it's done' kind of release policy, but be prepared to receive a stream of re-routed issues from Cubic then :)

@SDRplay
Copy link
Contributor Author

SDRplay commented May 30, 2020

well not really. Currently the situation is API 2.13 people are pointed to pothosware repo - that is clear.

In the readme on the pothosware repo, API 3.x people are pointed to SDRplay repo and we also point people there on our website.

it's just that now API 3.x people are not going to test the new gain system, so that just puts more emphasis on us to do a better job 😃

as soon as API 3.x is released, we should replace the repo on pothosware and phase out 2.x altogether - that's always been the plan.

The last issue (I think) is on the Mac platform, so I don't expect it to be too long before release.

@fventuri
Copy link

I just pushed to the branch 'new-gain-controls' the changes you suggested earlier:

  • rename the gain names to simply 'IF' and 'RF'
  • have listGains() only return 'IF' and 'RF', while keeping 'IFGR' and 'RFGR' around in all the other methods for backward compatibility.

I also cleaned up the code for the various gain methods (getGain(), setGain(), etc).

I created this github issue for these changes: pothosware/SoapySDRPlay3#10

Once it looks good to the three of us, I'll merge that branch into master so everybody can try it out and let us know their feedback - I do see your point Andy, and I promise you that branch won't be around more than strictly necessary. 😄

Franco

@fventuri
Copy link

Also I thought about the future issues with users having problems if the sdrplay_api service is not running or for some reason it is unresponsive.

I thought it would help to have a more explanatory message if the call to sdrplay_api_Open() fails, suggesting the user to check the health of the sdrplay_api service.

Github issue: pothosware/SoapySDRPlay3#11

Franco

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants