Ifilehk commented 1 year ago

Hello,

I am running a simple python script to check my RSP1a but I am seeing a strange behavior. The script runs as expected, but when I remove my device from the USB port, the streaming doesn't stop. I can still see samples printed in the console ! Testing the same with an rtlsdr dongle, the streaming stops as I would expect. Any clue ? Thank you for your help.

!/usr/bin/env python3

-- coding: utf-8 --

import SoapySDR from SoapySDR import * #SOAPYSDR constants import numpy #use numpy for buffers

args = dict(driver="sdrplay") sdr = SoapySDR.Device(args)

sdr.setSampleRate(SOAPY_SDR_RX, 0, 2e6) sdr.setFrequency(SOAPY_SDR_RX, 0, 100e6)

rxStream = sdr.setupStream(SOAPY_SDR_RX, SOAPY_SDR_CF32) sdr.activateStream(rxStream)

buff = numpy.array([0]*1024, numpy.complex64)

receive some samples

for i in range(1000000000): sr = sdr.readStream(rxStream, [buff], len(buff)) print('Ret', sr.ret, 'Flags', sr.flags, 'Time', sr.timeNs) for j in range(1024): print(j, buff[j] )

shutdown the stream

sdr.deactivateStream(rxStream) #stop streaming sdr.closeStream(rxStream)

fventuri commented 1 year ago

@Ifilehk - thanks for reporting the problem.

I created a new branch called check-for-device-removed (https://github.com/pothosware/SoapySDRPlay3/tree/check-for-device-removed) that checks for the sdrplay_api_DeviceRemoved event, and returns the error SOAPY_SDR_STREAM_ERROR if the device is removed while streaming.

I then ran a quick test with the SoapySDRUtil command:

SoapySDRUtil --args="driver=sdrplay" --rate=2e6 --direction=RX

and I see the Device removed error message when I unplug the RSP here (and the streaming stops).

Please give it a try and let me know if it works for you too. Franco

Ifilehk commented 1 year ago

Hello Marco,

Thank you for your quick response!!!

The current solution is OK but writing "[ERROR] Device removed" and "source :warning: Soapy source error: STREAM_ERROR" at every call of readStream. What about putting:

if (device_removed)
{
    throw std::runtime_error("Device removed");
    return SOAPY_SDR_STREAM_ERROR;
}

at the start of the readStreadStream function. Anyway if the device has been removed, not necessary to process a readStreadStream at all. In this case readStreadStream will return to the caller the exception and the log in the event callback:

else if (eventId == sdrplay_api_DeviceRemoved)
{
    device_removed = true;
    SoapySDR_log(SOAPY_SDR_ERROR, "Device removed");
}

Thank you !

fventuri commented 1 year ago

@Ifilehk - I noticed your Python script was missing a check on the return value from the function readStream(). Since the API document for readStream() says it can either return the number of elements read or an error code (https://github.com/pothosware/SoapySDR/blob/master/include/SoapySDR/Device.hpp#L350), I modified your script as follows:

#!/usr/bin/env python3

import SoapySDR
import numpy

sdr = SoapySDR.Device({'driver': 'sdrplay'})

sdr.setSampleRate(SoapySDR.SOAPY_SDR_RX, 0, 2e6)
sdr.setFrequency(SoapySDR.SOAPY_SDR_RX, 0, 100e6)

rxStream = sdr.setupStream(SoapySDR.SOAPY_SDR_RX, SoapySDR.SOAPY_SDR_CF32)
sdr.activateStream(rxStream)

buff = numpy.array([0]*1024, numpy.complex64)

# receive some samples
while True:
  sr = sdr.readStream(rxStream, [buff], len(buff))
  if sr.ret == SoapySDR.SOAPY_SDR_STREAM_ERROR:
    break
  print('Ret', sr.ret, 'Flags', sr.flags, 'Time', sr.timeNs)
  #for j in range(1024):
  #  print(j, buff[j])

# shutdown the stream
sdr.deactivateStream(rxStream)
sdr.closeStream(rxStream)

With this version I ran the usual test removing the device while it was streaming, and the behavior looks OK to me.

Please notice that this quick change only catches the error SOAPY_SDR_STREAM_ERROR; a more robust version would catch any error with something like this:

  if sr.ret < 0:
    break

Franco

Ifilehk commented 1 year ago

I agree with you Franco, but imagine a use case where readStream would be in a worker and called continuously. The worker will not stop calling it and at every readStream call a log line will be printed in the console. That is my problem. Console get polluted with same log line and at the speed of the log lines printing will make the application unresponsive. So it is more about outputting the log line just once, but of course keeping readStream returning SOAPY_SDR_STREAM_ERROR. A use case I am speaking about is a gnuradio worker.

Ifilehk commented 1 year ago

Hello Franco. Another "wish" if you permit. Since the errors are sent to the console, and like in my use case, I do not have direct access to readStream and the corresponding device, it would be nice if the error log line could identify the device. "Device removed" -> "Device ... removed".

fventuri commented 1 year ago

@Ifilehk - I usually consider CubicSDR as the reference client SDR application for the SoapySDRPlay3 module for a couple of reasons:

this module started with the intent of providing support in CubicSDR for SDRplay RSP devices; if you look at the earliest issues in this repository, you'll see that they all refer to CubicSDR (the GNU Radio gr-soapy module became available a few years later, if I remember correctly)
I also think that the number of users of this module via CubicSDR is (perhaps significantly) larger than the GNU Radio users of 'gr-soapy', since in my experience CubicSDR has a UI that is easier to use than GNU Radio flowgraphs

Because of this tonight I took a look at the source code for CubicSDR (https://github.com/cjcliffe/CubicSDR/blob/master/src/sdr/SoapySDRThread.cpp#L265-L285), and I noticed that they handle the errors returned by the function readStream() differently; it looks like with an error of SOAPY_SDR_STREAM_ERROR CubicSDR prints a warning message and tries to continue streaming; however it looks like the error code SOAPY_SDR_NOT_SUPPORTED causes CubicSDR to stop streaming altogether.

Therefore, given how CubicSDR handles these error codes, I changed my code to return the error SOAPY_SDR_NOT_SUPPORTED instead.

As per the error messages sent to the console, I also looked at the source code for two similar SoapySDR modules (SoapyRTLSDR and SoapyHackRF), and I see that, when an error occurs in readStream() (and acquireReadBuffer()), they don't print any error message to the console (see here for instance: https://github.com/pothosware/SoapyHackRF/blob/master/HackRF_Streaming.cpp#L590-L592), except in the case of buffer overflow where they just print the letter 'O'. This made me think if instead I should just get rid of the 'Device removed' error message to be consistent with what the other SoapySDR modules do.

Franco

Ifilehk commented 1 year ago

Was not aware of the CubicSDR story.

Indeed the log "Device removed" in readStream() (and/or acquireReadBuffer()) doesn't make sens. In my opinion it would make sens to log "Device removed" in SoapySDRPlay::ev_callback when the event is triggered thus making it appear just once in the log.

fventuri commented 1 year ago

@Ifilehk - first of all my apologies for not following up in a couple of weeks, since I was busy working on another SDR project.

The other day I looked again at how the SoapySDRPlay3 driver deals with real time events while streaming, and I noticed that I was already handling a very similar situation for the SDRPlay RSPduo when running in Slave mode (see here: https://github.com/pothosware/SoapySDRPlay3/blob/check-for-device-removed/Streaming.cpp#L196-L202) - I should have remembered about this case when we started this conversation.

Anyhow given how similar the two cases are (in one case the device is removed; in the other case the Master instance is removed while the Slave instance is still streaming), I think that they should follow the same approach, so I changed the code to print an error message and throw an exception in the case of the device being removed.

You can find this latest change in the usual check-for-device-removed branch (https://github.com/pothosware/SoapySDRPlay3/tree/check-for-device-removed).

Franco

fventuri commented 1 year ago

@Ifilehk - I just pushed to the master branch (https://github.com/pothosware/SoapySDRPlay3/commit/a48b702308306d31b77bd4727676139fbf1704c9) the commit that prints an error message and throws an exception if the device is removed while streaming.

Please let me know if you find any problem; if not, I'll close this issue in a few days.

Franco

Ifilehk commented 1 year ago

Hello Franco

Sorry for the late feedback. Was offline.

Just tried your solution. The problem I have with this one, is that gnuradio do not catch your exception. The result is a crash.

Instead gnuradio checks the return of readStream and acts consequently. So this solution is bad for me.

This is the gnuradio code:

for (;;) {

    // No command handlers while reading
    int result;
    {
        std::lock_guard<std::mutex> l(d_device_mutex);
        result = d_device->readStream(
            d_stream, output_items.data(), noutput_items, flags, time_ns, timeout_us);
    }

    if (result >= 0) {
        nout = result;
        break;
    }

    switch (result) {

    // Retry on overflow. Data has been lost.
    case SOAPY_SDR_OVERFLOW:
        std::cerr << "sO" << std::flush;
        continue;

    // Yield back to scheduler on timeout.
    //case SOAPY_SDR_TIMEOUT, SOAPY_SDR_NOT_SUPPORTED:
    //    break;

    // Report and yield back to scheduler on other errors.
    default:
        std::cerr << SoapySDR::errToStr(result) << std::flush;
        //d_logger->warn("Soapy source error: {:s}", SoapySDR::errToStr(result));
        break;
    }

    break;
};

If you have any idea how to come around would be grateful.

fventuri commented 1 year ago

@Ifilehk - first of all I apologize for making you wait so long.

I did not consider the case of the GNU Radio module 'gr-soapy' when I pushed that change more than two weeks ago.

Tonight I recreated the check-for-device-removed branch (https://github.com/pothosware/SoapySDRPlay3/tree/check-for-device-removed), so that it returns the SOAPY_SDR_NOT_SUPPORTED error code when the RSP is removed instead of throwing an exception (like it did before). I also changed the code for the case of the RSPduo slave device when the master is removed to handle this error the same way.

This behavior seems to work as expected with CubicSDR (i.e. the application stops streaming); when you have time, please build the SoapySDRPlay3 module from the check-for-device-removed branch, and give it a try with GNU Radio to make sure it works without unexpected problems there too.

Thanks, Franco

Ifilehk commented 1 year ago

Hello Franco, Patience is a virtue that I cultivate, so everything fine :-) On the other hand in my opinion, we all here should not enough praise you for what you are doing for this community and specially for your quick and constructive responses. That said :-):

This is working fine under gnuradio. The returned "brakes" the loop of the worker as expected.
But we could may be improve the error message, because I am dealing with more than one SDR. Would nice to know witch SDR have been disconnected. Currently the log of gnuradio is as follow:

STREAM_ERROROsO[1m[31m[ERROR] Device removed[0m STREAM_ERROROsO[1m[31m[ERROR] Device removed[0m STREAM_ERROROsO[1m[31m[ERROR] Device removed[0m STREAM_ERROROsO[1m[31m[ERROR] Device removed[0m ...

Would be nice to have a line like: STREAM_ERROROsO[1m[31m[ERROR] Device (SDR LABEL) removed[0m or similar identifying the device.

I hope it is feasible without too much effort.

Kind Regards

Tarik

fventuri commented 1 year ago

@Ifilehk - thanks for your feedback Tarik - I just merged the check-for-device-removed branch into the master branch, so that that fix should now be available to all the users.

As per your suggestion of adding the serial number (or label) of the device in those error messages, I think you do have a valid point; however these days I was thinking that perhaps that information should be added to all the error and warning messages in the SoapySDRPlay3 driver to keep them consistent.

What I plan to do is to create a new branch this weekend following your advice so we can try it out, see what the messages would look like, and then make a decision about it.

Franco

fventuri commented 1 year ago

@Ifilehk - I apologize for the delay in working on this request; I was out of town for the last couple of weeks.

I just pushed the new branch show-serial-number-in-messages where I added a new cmake command line option SHOW_SERIAL_NUMBER_IN_MESSAGES, that shows the serial number of the device at the beginning of each log message.

You can try out this new feature by building the SoapySDRPlay3 module as follows:

git clone https://github.com/pothosware/SoapySDRPlay3.git
cd SoapySDRPlay3
git checkout show-serial-number-in-messages
mkdir build
cd build
cmake -DSHOW_SERIAL_NUMBER_IN_MESSAGES=ON ..
make
sudo make install
sudo ldconfig

With this new version the error message you receive when disconnecting your device should look like this:

[ERROR] [S/N=<serial number>] - Device has been removed. Stopping.
[ERROR] [S/N=<serial number>] - Device is unavailable

Franco

Ifilehk commented 1 year ago

Hello Franco.

Same apology from my side. Had to deal with the famous virus.

Just tested your last branch and it works perfectly for me. Thank you for your support and wish you a successful continuation !!!

Regards

Tarik

fventuri commented 1 year ago

@Ifilehk - no problem at all! I hope you are doing OK and that the virus wasn't too bad for you.

Thanks for checking out the show-serial-number-in-message branch; when I merge it to master in the next few days, I think I'll leave the -DSHOW_SERIAL_NUMBER_IN_MESSAGES=ON as a compile time option, in order not to change the log messages for those who are not interested in the serial number, because they either have just one RSP connected, or they know which device the messages originate from, since they selected it in the SoapySDR connection string.

Franco

fventuri commented 1 year ago

@Ifilehk - I just merged this branch into the master branch. If I don't find any other problems, I'll go ahead and close this issue in a week or so.

Franco

pothosware / SoapySDRPlay3

RSP1A still streaming even if disconnected from USB port #53

!/usr/bin/env python3

-- coding: utf-8 --

receive some samples

shutdown the stream