-
Notifications
You must be signed in to change notification settings - Fork 259
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
Safer calls to setGainMode #701
Conversation
Will call hasGainMode first - this will prevent crashes when the underlying device situationally doesn't support setGainMode calls.
There was a lot of chaos from the v2 api release and bladrf2 support. Hopefully most of that is cleaned up now :-) Anyway, lets fix this. Open an issue with the details, or make a PR: https://github.com/pothosware/SoapyBladeRF |
I am fixing a few things with SoapyBladeRF and I think it is working now. I'm waiting for DC cal to run so that I can test it with the AGC - but it does work falling back to manual gain. The changes amounted to 1. Implementing a fairly robust hasGainMode that actually tries to enable automatic mode and 2. Re-adding support for the list functions (bandwidth and sample rate) - even though they were supplanted by the range-oriented functions, it really makes sense for the Soapy plugin itself to provide "recommended" values in the way that Soapy does. |
So I have tested this out - with a DC cal table present, it starts up with AGC and everything runs great. Without a DC cal table, it at least doesn't crash. Things are a little less than ideal, though, as it's not reflected to the user that it's in manual gain mode. The backend calls to setGainMode are skipped, but it would be best for the UI to somehow know that AGC isn't available, and run locked into manual gain mode. |
@drahosj thanks for the contribution; it will surely help avoid some unwanted crashes. And I agree we should probably handle this at the UI level at some point as suggested. |
You're welcome, and agreed. The crux of the problem is that CubicSDR
crashes (hard and fast) on oddball (or really anything other than strictly
expected) input.
It just so happened that making sure SoapyBladeRF always returns sane input
that CubicSDR expects is a quick and easy to fix, while modifying the
CubicSDR codebase is...not.
Longterm, CubicSDR will really need to be fortified against this sort of
thing. The world of open source SDR is anything but stabilized and
consistent, and frankly "do what CubicSDR expects" is a bit of a risky
stance to take. I don't know what that sort of error handling will look
like, and what kind of sane defaults CubicSDR will need to assume.
Every time this sort of bug comes up, I'm torn on how to fix it - but the
backend fix is and has always been cleaner and faster.
…On Thu, Mar 14, 2019, 8:06 PM Charles J. Cliffe ***@***.***> wrote:
@drahosj <https://github.com/drahosj> thanks for the contribution; it
will surely help avoid some unwanted crashes. And I agree we should
probably handle this at the UI level at some point as suggested.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#701 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABRgPyFRGc7bQr7i57yh6CybBhLXSFkMks5vWvIWgaJpZM4Z9mZf>
.
|
Throwing one more thing out there - when I've been fixing this, I've
started down the line of implementing extremely strict dynamic asserts
within CubicSDR. It ends up being a bit of a rabbit hole, and I usually
find the root cause pretty quickly - so that code has never seen the light
of day. But it'd go a long way towards facilitating future fixes with the
benefits of a) being much less work and b) not having to take a strong
stance on dictating design decisions.
…On Thu, Mar 14, 2019, 8:33 PM Jake Drahos ***@***.***> wrote:
You're welcome, and agreed. The crux of the problem is that CubicSDR
crashes (hard and fast) on oddball (or really anything other than strictly
expected) input.
It just so happened that making sure SoapyBladeRF always returns sane
input that CubicSDR expects is a quick and easy to fix, while modifying the
CubicSDR codebase is...not.
Longterm, CubicSDR will really need to be fortified against this sort of
thing. The world of open source SDR is anything but stabilized and
consistent, and frankly "do what CubicSDR expects" is a bit of a risky
stance to take. I don't know what that sort of error handling will look
like, and what kind of sane defaults CubicSDR will need to assume.
Every time this sort of bug comes up, I'm torn on how to fix it - but the
backend fix is and has always been cleaner and faster.
On Thu, Mar 14, 2019, 8:06 PM Charles J. Cliffe ***@***.***>
wrote:
> @drahosj <https://github.com/drahosj> thanks for the contribution; it
> will surely help avoid some unwanted crashes. And I agree we should
> probably handle this at the UI level at some point as suggested.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#701 (comment)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABRgPyFRGc7bQr7i57yh6CybBhLXSFkMks5vWvIWgaJpZM4Z9mZf>
> .
>
|
Will call hasGainMode first - this will prevent crashes when the underlying
device situationally doesn't support setGainMode calls.
This issue rears its head with the BladeRF - because AGC is only provided when a LUT is generated, it's really quite variable whether or not setGainMode will work, or will error out. As such, it would be best to avoid calling it without a call to hasGainMode to make sure it's supported.
Note that this requires a trustworthy result from hasGainMode; which is currently not the case for SoapyBladeRF. Things still crash because hasGainMode always returns true (for the RX channel), even when it actually doesn't. But that's a separate issue.