JuliaTelecom / UHDBindings.jl

Julia C bindings for UHD to monitor USRP devices.
MIT License
5 stars 3 forks source link

Error checking in recv #10

Closed dd0 closed 1 year ago

dd0 commented 1 year ago

The high-level recv! function does not check the error code in received packet metadata. Because of this, overflows will silently drop samples: the C library prints an O as a warning, but the Julia program does not find out about this.

Some applications that continuously receive samples from the SDR might not care about this, but it causes significant issues if we want to request a fixed number of samples. For example:

n = 1000000
UHDBindings.restartStreamer(radio.rx, UHDBindings.LibUHD.UHD_STREAM_MODE_NUM_SAMPS_AND_DONE, num_samps = n)
s = Vector{ComplexF32}(undef, n)
UHDBindings.recv!(s, radio)

If a sample is dropped due to overflow, the buffer will never be filled to the end and recv! ends up in an infinite loop.

I think it would be good to raise an exception in this case (as well as for other error types). I'd be happy to submit a PR, but I wanted to ask for your opinion first -- raising exceptions by default might break existing continuous-streaming applications. Maybe behind a flag in the UHDRx object or by default only if the streamer is not in continuous mode?

RGerzaguet commented 1 year ago

Hello, Thank you for starting the discussion. :) This is actually a design question that I have thought a lot about. Many of my applications are continuous, and I manage the overflow/underflow on top of the recv! function. That's why I haven't included any exception handling in the receive function. Nevertheless, you are right, and sometimes managing exceptions can be acceptable in the recv! call. However, I'm not sure if we should make this behavior the default (as it could potentially break a lot of things). Still, it's something that can be implemented. It can be done in various ways, such as:

  1. Branching in recv function based on streaming mode as you proposed
  2. Also as you proposed, by adding a flag when initializing the UHD device and including it in the UDHRx structure (e.g., raise_exception=nothing). The advantage is that this flag can also be used in the restartStreamer function to ensure that changing the streamer can change the exception behavior. By default, it should not track exceptions (raise_exception=false), but the user can choose to enable exception tracking (raise_exception=true). If the streamer mode uses a fixed number of samples (UHDBindings.LibUHD.UHD_STREAM_MODE_NUM_SAMPS_AND_DONE), we can set the exception mode to true unless the user explicitly sets raise_exception=false.

What are your thoughts on this? Do you have any better ideas for the flag than raise_exception? I'm currently overwhelmed with tasks, so the PR would be greatly appreciated. :)

Note that in the example you have provided, I don’t really get how we can monitor the error as the recv (populate_buffer!) call is blocking ? By asynchronously inspect metadata ?

dd0 commented 1 year ago

I agree that raising exceptions by default would be the wrong design for many applications, so a flag like you describe in (2) sounds good to me. As far as I can tell, there is no way to read the streaming mode after it has been configured, so this is something we would have to store separately in the UHDRx object. raise_exception sounds like a good name to me.

The uhd_rx_streamer_recv call in populate_buffer! is blocking, but it will return if it encounters an error. The function returns the number of received samples, or 0 to indicate an error (doc), so I would assume that it exits immediately on error.

Error checking would then be done in populate_buffer!: if the exception flag is set in the radio object, and the returned metadata indicates an error code different from ERROR_CODE_NONE or ERROR_CODE_TIMEOUT (which could occur e.g. when reading from a radio scheduled to start receiving in the future), we raise an appropriate exception.

RGerzaguet commented 1 year ago

Solved by #11 Many thanks @dd0 for the support ! :)

dd0 commented 1 year ago

Thank you for the help with working out the details and taking a look at the PR so quickly!