fpagliughi / rust-industrial-io

Rust interface to the Linux Industrial I/O subsystem
MIT License
45 stars 21 forks source link

Added method to get mutable slice of channel buffer #27

Open eivindbergem opened 1 year ago

eivindbergem commented 1 year ago

The library is missing a way to write data to buffers (or am I missing something?).

I added a method that returns a mutable slice to the buffer data. It's wrapped in an Option because iio_buffer_start() may return NULL.

fpagliughi commented 1 year ago

You're probably right. I don't think I have a board with a D/A for output on it! Just A/D for input. I've been meaning to get one, because I figured that I forgot something. I'll have a look at this shortly and get it in.

fpagliughi commented 1 year ago

Finally looking at this, I'm not sure it's the best way to implement it, for two reasons:

  1. The channel's data is not contiguous in the slice if the buffer is for multiple channels, right? So slice[0] would be for the requested channel, but slice[1] might be a different channel.
  2. It's different than how we did it for reading immutable data from the buffer. For that we created an iterator.

So, I'm thinking the better way to solve the problem would be to create a mutable iterator that takes care of the stepping.

Alternately, modeling both mutable/immutable to a pointer or slice-like struct that allows indexing, but handles the first address/step issues behind the scenes.

Or... do both. Iterator, and random access struct.

fpagliughi commented 1 year ago

I added a mutable iterator to the buffer which handles stepping through the channel offsets (like the previous implementation of the immutable iterator).

See: 0995d4eb5457112259877781ca6417017aef9912

It's untested since I don't have a D/A output board, but I'll see if I can use the Dummy driver to do some testing.

The commit also updates the implementation of the (immutable) iterator to tie its lifetime to the buffer - which was a stupid omission up till now - and makes it return a reference to the item rather than a copy of the item. This is a breaking change, but it's probably better for them to be consistent - and more consistent with slice iterators.

eivindbergem commented 1 year ago

Finally looking at this, I'm not sure it's the best way to implement it, for two reasons:

1. The channel's data is not contiguous in the slice if the buffer is for multiple channels, right? So `slice[0]` would be for the requested channel, but `slice[1]` might be a different channel.

2. It's different than how we did it for reading immutable data from the buffer. For that we created an iterator.

So, I'm thinking the better way to solve the problem would be to create a mutable iterator that takes care of the stepping.

Alternately, modeling both mutable/immutable to a pointer or slice-like struct that allows indexing, but handles the first address/step issues behind the scenes.

Or... do both. Iterator, and random access struct.

Yes, you're right. I had only used it with a single channel, so I didn't really consider how it would work with multiple channels.

fpagliughi commented 1 year ago

I ordered a D/A board, which should arrive today. So I will try to test out the mutable iterator this weekend. If it works, I'll get the new release out right away. Let me know if you have a chance to test it yourself.

But.... you gave me an idea...

If you did just have a single channel A/D or D/A, and the step size matched the size of the data type (no padding), then a slice probably would be slightly more efficient than the manual iterator.

So your function might be like:

if begin.is_null()  || self.num_channel() != 1 || self.step_bytes() != mem::size_of::<T>() {
    None
} else {
    ...
}

Thos other buffer functions would need to be added, but that would be trivial.

voibit commented 5 months ago

Needed access to the whole buffer, added slice methods similarly, but without templates in voibit@38e9a141bf6e617dff396b2af4f2f9d13159b387