-
Notifications
You must be signed in to change notification settings - Fork 15
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
Now unlocking global mutex during sleep, to allow callbacks to execute. #59
Conversation
SoapySDRPlay.hpp
Outdated
int gr_changed; | ||
int rf_changed; | ||
int fs_changed; | ||
volatile int gr_changed; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why volatile? A simple std::atomic<int>
should be sufficient. When accessing, std::memory_order_relaxed
should also be fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly because "volatile" feels simpler to me than all the funny alphabet soup introduced in modern C++
@luarvique, @fmoessbauer - many thanks for your work on this issue and your suggestions. Franco |
@fventuri , this pull request has been pending for quite a while now. Could you please review and merge it? If you do not want to merge it as-is, please, implement a similar fix that involves releasing the global lock before a wait. |
@luarvique - thanks for the reminder. I wrote a very simple C++ program (attached) to try to reproduce the issue you reported here; in this program I have a loop that keeps changing frequency and RF gain (via the setting Do you mind showing me in more detail the steps you took in OpenWebRX to make that problem occur, or even better to write a simple program in C/C++/Python that causes that problem, so I can run some tests here? Thanks, |
Steps as follows:
systemctl status openwebrx or journalctl -u openwebrx
[WARNING] Sample rate/decimation update timeout. Please, let me know if you would like me to walk you through the source of the problem again. |
PS: The problem is obviously not limited to band changes, but it is the easiest to see there. |
@luarvique - this afternoon I built the latest version of OpenWebRX (v1.3.0-dev), and tonight I created three OpenWebRX profiles for my SDRplay RSPdx: one for 10MHz (HF), another one for 2m (VHF), and finally a third one for the 70cm band (UHF). The spurious messages:
are due to the fact that OpenWebRX might change the decimation (not the input RSP sample rate) when changing bands; in this case (i.e. when only the decimation is changed) the effect is immediate, and the I plan to fix that bug in the Franco |
Well, for me the problem frequently occurs when changing any values where a while() wait is present in the SoapySDRPlay3 code. The issue mostly disappeared once I implemented proper lock operation in the SoapySDRPlay3. It did remain in the frequency/decimation case and now, after your explanation, I see why. |
@luarvique - I just pushed to the I ran a few tests tonight changing bands with this bug fix, and I haven't seen any error/warning message so far, but I'll keep trying to try to reproduce your initial issue. Franco |
Let me build and install the latest master branch tonight - going to see if I can recreate the message for non-band changes. |
@fventuri , well, updated to your latest "master" branch code, rebuilt, reinstalled .so, restarted OpenWebRX ... and lo and behold:
In the above log, the update timeouts are of the main interest as far as this pull request is concerned. Once again, @fventuri , would you like me to go over why they happen with you? |
@luarvique - quick update; I ran a few more tests here and these are my findings:
Franco |
Please, disregard this message: it originates from OpenWebRX, does not signify an error, and does not affect SoapySDRPlay3 operation.
The same issue occurs with all the flags for me, not just with fsChanged. Every time you wait for a callback to change the flag, without releasing the global lock, there is a timeout.
I suspect that the significant difference between our setups is the host hardware architecture and the operating system. I am running Armbian on a ROC-RK3328-CC SBC. Differences in the hardware and OS may account for different threading library operation. |
@luarvique - ARM-32 or ARM-64? I have a Raspberry Pi sitting in a drawer here somewhere, and I thought I could give it a try using the latest Raspberry OS to see if with that hardware I can reproduce those timeouts for changes in gain and frequency. Franco |
ARM64. I can probably just give you SSH access to my current hardware, if absolutely necessary. |
@luarvique - I wonder if those timeout warnings when changing the gain reduction values might be due to the fact that the function I just updated the
Give it a try and hopefully this will explain those Franco |
That was actually my first suspicion and I also made SoapySDRPlay3 print warnings on sdrplay_api_Update() failures. But no, the function succeeded, and the gain reduction values were ok (0-3 for the little blue MSI.SDR). |
A gentle reminder that using the SDRplay API with non-SDRplay hardware is a violation of the API license. We cannot warrant how the API operates with non-SDRplay hardware and things like gain control use more than just the Mirics chipset and so you can easily see invalid responses in that condition. It is unfair that @fventuri spends his valuable time chasing down issues that are not supported by the API. If the issue post contained the hardware in use, then at least @fventuri and others could decide whether it should be allowed or actioned on in this issue list. If there are genuine issues that people are finding with the API using genuine SDRplay hardware, we're always happy to work with people on them. |
@SDRplay, well, the usage case in question does not allow for the use of the genuine hardware, since it simply would not fit. But I do have the genuine hardware (RSPduo) and can bet that the same exact issue happens with it as well, since the issue does not appear to be with the SDRPlay drivers. Also, please, allow me to use this opportunity for a gentle reminder of my own. This pull request has been pending for 5 months now, with no reaction from @SDRplay developers: I have also reported the issue to the user support (before attempting to fix it) and was essentially told to screw off. This is, mind you, for the genuine SDRPlay hardware. Thank you for the use of my valuable time. |
@luarvique I was not commenting on the issue you are referring to, I was commenting on the use of the SDRplay API - the issue you reported to us in May was about Bias-T control and that was incorporated into changes to the repo a couple of months ago. It wasn't ignored and I don't recall you being told to "screw off". The pull request was regarding a different issue, tuner switching, which was again incorporated into a number of changes that were made a few months ago. Just so I understand this correctly... you've seen an issue with the SoapySDRPlay library and a non-SDRplay piece of hardware. You assume it would also work in the same way on SDRplay hardware? You could have helped Franco out by testing on genuine SDRplay hardware - maybe you have, I just haven't read up through the entire thread. Generally speaking, these things are always best reported using genuine hardware, so that everyone has a far better chance of reproducing whatever issue is being chased. |
This is very good to know, although a simple note on the pull request at the time of the changes would be very helpful.
You did not. Your customer support did.
Yes, because, as far as SDRPlay driver is concerned, everything is fine. There are no errors coming from the driver. I checked. Also increased the wait time for a callback to come up to one second. Same thing.
Essentially, I cannot stick the whole RSPduo where that MSI.SDR dongle is currently located. I can try plugging it in as the last resort, but going to recheck the software first. |
@luarvique - since at this point it seems that the most significant difference between what you see there and what I am able to replicate on my side are those timeout warnings whenever the gain reduction settings are updated via The code in this new branch is identical to the code currently in
Please give it a try when you have some time, and let me know what you see there. Franco |
All right. Just to keep the experiment clean, I am also going to update to the latest OpenWebRX version from the master branch. |
So, I installed SoapySDRPlay3 from the debug branch you created, updated OpenWebRX to 1.2.1, and did make some local changes to that OWRX install (nothing serious though). Now, things are even more interesting though. Every now and then, SoapySDRPlay3 fails to change the frequency. The log below corresponds to me switching from 40m to 10m, then to 20m, then to 80m: Switching to 40m is ok:
Switching to 10m fails, leaving OWRX at 40m (although it does not know that and shows it as "10m"):
Switching to 20m also fails, but SoapySDRPlay3 now goes to 10m (!), mistakenly shown by OWRX as "20m":
Finally, switching to 80m succeeds and I can finally see actual 80m in OWRX:
@fventuri, since you are the expert, you tell me what it is doing there. |
Just to clarify, no LNA changes are being sent in the above test. Whatever it is doing above, isn't related to the LNA state. |
I suspect that your premature return on sdrplay_api_Update() errors may be the reason for the above behavior:
Please, consider waiting and retrying the sdrplay_api_Update() call a few times if it returns an error. The SDRPlay API server might simply be busy with some other work. |
@luarvique - I ran a few tests tonight trying to replicate the Whenever I see error messages like those, I usually set the SDRplay API debug level to verbose by uncommenting line 1876 in Franco |
This should fix any random timeouts and failures to set various RSP settings (such LNA state).