Start some cleanup, ripout lending_iterator, and use system-install…

…ed PSRDADA

Cargo.toml

@@ -16,7 +16,6 @@ categories = ["encoding", "parser-implementations"]
 psrdada-sys = { path = "./psrdada-sys", version = "0.4.0" }
 page_size = "0.6"
 tracing = "0.1"
-lending-iterator = "0.1"
 nom = "7"
 
 [dev-dependencies]

README.md

@@ -7,36 +7,19 @@
 [![Codecov](https://img.shields.io/codecov/c/github/kiranshila/psrdada-rs?style=flat-square)](https://app.codecov.io/gh/kiranshila/psrdada-rs)
 
 This is a rust library around the [psrdada](http://psrdada.sourceforge.net/) library commonly used in radio astronomy.
-Unfortunately, the C library is for the most part undocumented, so the behavior presented by this rust library is what
-the authors have been able to ascertain by reading the original example code.
-As such, this might not be a 1-to-1 implementation of the original use case.
+Unfortunately, the C library is for the most part undocumented, so the behavior presented by this rust library is what the authors have been able to ascertain by reading the original example code.
+As such, this might not be a 1-to-1 implementation of the original use case and implements only a subset
+of the features available in the C library.
 
 ## Usecase
 
 Use this library if you want a safe abstraction around working with psrdada.
 As in, use this library if you need to interface with applications that are expecting psrdada buffers.
 Do not use if you don't have to, as it (psrdada itself) isn't as performant or featureful as other IPC libraries.
 
-### Alternatives
-
-The rust library [shmem-ipc](https://github.com/diwic/shmem-ipc) has excellent performance over shmem, useful for large
-data transfers (like windows of spectral data). It creates shared ringbuffers, much like psrdada.
-Interfacing with D-Bus is fine for signaling and headers.
-
-If you _need_ CUDA support, [NVSHMEM](https://developer.nvidia.com/nvshmem)
-is a thing that exists, and you should use it. Also, linux has [mkfifo](https://linux.die.net/man/3/mkfifo) which works fine with CUDA
-as discussed [here](https://forums.developer.nvidia.com/t/gpu-inter-process-communications-ipc-question/35936/12).
-
-Lastly, there is [ipc-channel](https://github.com/servo/ipc-channel), which uses the Rust channel API over OS-native IPC abstractions.
-It's a really nice library.
-
-In short, if you are constructing a pipeline from scratch, don't use psrdada.
-There are more mature, documented, more performant alternatives.
-
 ## Installation
 
-We are building and linking the psrdada library as part of the build of this crate, which requires you have a working C compiler.
-See the [cc](https://docs.rs/cc/latest/cc/) crate for more details.
+You need to build and install PSRDADA manually, following the installation guide found [here](https://psrdada.sourceforge.net/download.shtml). Alternatively, you can use the [nix](https://nixos.org/) flake [here](https://github.com/kiranshila/psrdada.nix/blob/main/flake.nix) to declaratively create environments (shells/docker containers/operating systems) with PSRDADA baked in (deterministically).
 
 ## Example
 
@@ -107,22 +90,9 @@ read_block.read_exact(&mut buf).unwrap();
 without that `write_block.commit()` line, this code would not compile as there still exist a write in progress.
 Additionally, you can only ever `split` once, so you'll only ever have a single reader and writer for each type.
 
-## What we learned about psrdada
-
-- Don't use `ipcio_t` or `dada_hdu`.
-
-They are wrappers around `ipcbuf_t` and have all sorts of undefined behavior.
-Specifically, `ipcio_t` reimplemented stdlib `read` and `write` behavior, but in unsafe ways.
-Our abstraction presented here reimplements the behavior, but with Rust's compile-time guarantees.
-`dada_hdu` combines two `ipcbuf_t`s, the header and data buffers.
-However, doing so breaks CUDA support (for some reason) and messes up the signaling of successful reads.
-
-- "End of data" is more or less a meaningless flag.
+### Thanks
 
-End of data doesn't prevent us from reading more data or writing more data. It is just a signal we can observe.
-The iterator interface we provide will produce `None` if we run out of data, trying to be consistent with what that
-might mean. Additionally, there is a very specific order in which eod is set and read. It _must_ be set after `mark_filled`
-and before `unlock_write`. It _must_ be read after `mark_cleared` and before `unlock_read`. Any other ordering doesn't work.
+Much of the implementation is inspired by other "modern" wrappings of PSRDADA, especially [PSRDADA_CPP](https://gitlab.mpcdf.mpg.de/mpifr-bdg/psrdada_cpp).
 
 ### License
 

flake.nix

@@ -29,7 +29,9 @@
         pkgs = import nixpkgs { inherit system overlays; };
         nativeBuildInputs = with pkgs; [ rustPlatform.bindgenHook pkg-config ];
         buildInputs = with pkgs; [
-          rust-bin.stable.latest.default
+          (rust-bin.stable.latest.default.override {
+              extensions = ["rust-src" "rust-analyzer"];
+            })
           psrdada.packages.${system}.default
         ];
       in with pkgs; {

src/builder.rs

@@ -73,7 +73,8 @@ impl DadaClientBuilder {
 
     #[tracing::instrument]
     /// Builder for DadaClient
-    /// Buffer size will default to 4x of 128*Page Size
+    ///
+    /// Buffer size will default to 4x of 128*Page Size.
     /// Header size will default to 8x of Page Size
     pub fn build(self) -> PsrdadaResult<DadaClient> {
         // Unpack the things we need, defaulting as necessary

src/client.rs

@@ -64,10 +64,16 @@ impl DadaClient {
         Ok(s)
     }
 
-    #[tracing::instrument]
+    #[deprecated(note = "This wasn't an obvious name, use `DataClient::connect` instead")]
     /// Construct a new DadaClient and connect to existing ring buffers
     pub fn new(key: i32) -> PsrdadaResult<Self> {
-        let (data_buf, header_buf) = Self::connect(key)?;
+        Self::connect(key)
+    }
+
+    #[tracing::instrument]
+    /// Construct a new DadaClient by connecting to existing ring buffers
+    pub fn connect(key: i32) -> PsrdadaResult<Self> {
+        let (data_buf, header_buf) = Self::connect_both(key)?;
         let s = Self {
             data_buf,
             header_buf,
@@ -78,7 +84,7 @@ impl DadaClient {
 
     #[tracing::instrument]
     /// Internal method to actually build and connect
-    fn connect(key: i32) -> PsrdadaResult<(*const ipcbuf_t, *const ipcbuf_t)> {
+    fn connect_both(key: i32) -> PsrdadaResult<(*const ipcbuf_t, *const ipcbuf_t)> {
         debug!(key, "Connecting to dada buffer");
         unsafe {
             let data_buf = Box::into_raw(Box::default());
@@ -203,7 +209,7 @@ mod tests {
     fn test_connect() {
         let key = next_key();
         let _client = DadaClientBuilder::new(key).build().unwrap();
-        let _connected = DadaClient::new(key).unwrap();
+        let _connected = DadaClient::connect(key).unwrap();
     }
 
     #[test]

src/dada_iter.rs

@@ -0,0 +1,7 @@
+//! A trait that we will use that leverages [generic associated types](https://blog.rust-lang.org/2022/10/28/gats-stabilization.html)
+//! to create a dada iterator that garuntees that references to a given buffer only exist when it is safe to do so.
+
+pub trait DadaIterator {
+    type Item<'a>;
+    fn next<'a>(&mut self) -> Option<Self::Item<'a>>;
+}

src/headers.rs

@@ -199,7 +199,7 @@ mod tests {
         let hdr = b"FOO\tBAR # A comment\nBAZ   \tquuz123#morecomment__\n\nbEanS __RICE__";
         let (remaining, pairs) = header(hdr).unwrap();
 
-        let p1 = pairs.get(0).unwrap();
+        let p1 = pairs.first().unwrap();
         assert_eq!(b"FOO", p1.0);
         assert_eq!(b"BAR", p1.1);
 
@@ -219,7 +219,7 @@ mod tests {
         let hdr = b"foo bar\nbaz buzz#foob\n\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0";
         let (_, pairs) = header(hdr).unwrap();
 
-        let p1 = pairs.get(0).unwrap();
+        let p1 = pairs.first().unwrap();
         assert_eq!(b"foo", p1.0);
         assert_eq!(b"bar", p1.1);