kevinmehall / rust-soapysdr

Rust bindings for SoapySDR, the vendor-neutral software defined radio hardware abstraction layer
Apache License 2.0
75 stars 22 forks source link

Device::rx_stream issue with heap allocation Device instance #9

Closed russel closed 5 years ago

russel commented 5 years ago

The rx_stream method on Device takes a reference to the device. This works fine when the device instance variable can be assured to live longer than any created stream, as in the example code. However in a situation where the device is created at runtime as a heap object, there seems to be no code that the borrow checker will accept since there seems to be no way of dealing with the lifetime issue. Is there an example anywhere of using heap allocated Device and RxStream objects that compiles and works – I am fairly convinced my problem is lack of understanding of Rust and handling heap allocated object with this sort of back reference coupling.

I have tried keeping device and stream in a struct, but: https://users.rust-lang.org/t/structs-and-lifetimes-a-conundrum/23145

kevinmehall commented 5 years ago

To be clear, it's ownership, not stack vs heap that matters here. You can take the stream example and Box the Device and RxStream to put them on the heap and it works fine, but I don't think that helps with your problem:

--- a/src/bin/soapy-sdr-stream.rs
+++ b/src/bin/soapy-sdr-stream.rs
@@ -73,7 +73,7 @@ fn main() {
         }
     };

-    let dev = soapysdr::Device::new(dev_args).expect("Error opening device");
+    let dev = Box::new(soapysdr::Device::new(dev_args).expect("Error opening device"));

     let channel = matches.opt_str("c").map_or(0, |channel| {
         channel.parse::<usize>().expect("Invalid channel")
@@ -113,7 +113,7 @@ fn main() {

     match direction {
         Rx => {
-            let mut stream = dev.rx_stream::<Complex<f32>>(&[channel]).unwrap();
+            let mut stream = Box::new(dev.rx_stream::<Complex<f32>>(&[channel]).unwrap());
             let mut buf = vec![Complex::new(0.0, 0.0); stream.mtu().unwrap()];

             let mut outfile = BufWriter::new(File::create(fname).expect("error opening output file"));

The advice you received on that thread seems about right. The rental crate recommendation is one way to solve it, but without a good grasp on lifetime syntax, it would probably be more confusing because it re-uses the lifetime syntax in its macro to behave in a nonstandard way.

A more intuitive way may be to spawn a thread that opens the device and stream and keeps ownership of them, rather than trying to put them in a struct. You could give that thread the Sender side of a channel to send buffers, and put the Receiver side in your structure.

kevinmehall commented 5 years ago

Actually, we may be able to improve the ergonomics of this library for this use case. A Device is actually just a pointer to a structure on the C++ heap. SoapySDR internally reference-counts devices (if you open the same device twice, you get a Device containing the same pointer). If SoapySDR exposed a method to increase the reference count on a device, we could make the Rust Device wrapper implement Clone, and have the stream types own a Device rather than borrow a reference to it, and it wouldn't need that lifetime parameter. Owning the stream would keep the device alive. This wouldn't really give up control over when the device is closed because you don't truly have that control already -- another thread could already have opened the same device and be holding another reference to it.

kevinmehall commented 5 years ago

Fixed in https://github.com/kevinmehall/rust-soapysdr/pull/10