Skip to content

[WIP] Implemented transactional interfaces #33

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,7 @@ openpty = "0.1.0"
# we don't need the `Error` implementation
default-features = false
version = "0.2.2"

[patch.crates-io]
embedded-hal = { git = "https://github.com/ryankurte/embedded-hal", branch = "feature/spi-i2c-transactions" }

53 changes: 53 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,27 @@ impl hal::blocking::i2c::WriteRead for I2cdev {
}
}

pub use hal::blocking::i2c::{Operation as I2cOperation};

impl hal::blocking::i2c::Transactional for I2cdev {
type Error = i2cdev::linux::LinuxI2CError;

fn exec<'a, O>(&mut self, address: u8, mut operations: O) -> Result<(), Self::Error>
where O: AsMut<[I2cOperation<'a>]> {

// Map operations from generic to linux objects
let mut messages: Vec<_> = operations.as_mut().iter_mut().map(|a| {
match a {
I2cOperation::Write(w) => LinuxI2CMessage::write(w),
I2cOperation::Read(r) => LinuxI2CMessage::read(r),
}
}).collect();

self.set_address(address)?;
self.inner.transfer(&mut messages).map(drop)
}
}

impl ops::Deref for I2cdev {
type Target = i2cdev::linux::LinuxI2CDevice;

Expand Down Expand Up @@ -278,6 +299,38 @@ impl hal::blocking::spi::Write<u8> for Spidev {
}
}

pub use hal::blocking::spi::{Operation as SpiOperation};

impl hal::blocking::spi::Transactional<u8> for Spidev {
type Error = io::Error;

fn exec<'a>(&mut self, operations: &mut [SpiOperation<'a, u8>]) -> Result<(), Self::Error> {

// Map types from generic to linux objects
let mut messages: Vec<_> = operations.iter_mut().map(|a| {
match a {
SpiOperation::Write(w) => SpidevTransfer::write(w),
SpiOperation::WriteRead(r) => {
// TODO: is spidev okay with the same array pointer
// being used twice? If not, need some kind of vector
// pool that will outlive the transfer
let w = unsafe {
Copy link
Contributor Author

@ryankurte ryankurte Mar 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@posborne is this too cursed? i can't seem to find any documentation as to whether it's okay or not.

Alternatively we might need some kind of memory pool to copy things into / keep the storage around till the transactions are all done (because the transfer object can't hold something owned)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From looking at the implementation I think it would work, as the buffers are copied in and out. However, a documented guarantee would be better.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @eldruin notes, this should be fine with how the kernel works (performing a copy_from_user). That being said, I do agree that it does feel off, although I can't reason about any way it could go wrong in practice.

I would be OK with landing this here and considering adding an API to spidev which would be designed to explicitly provide safe semantics for this way of interacting with the syscall for future use. That is, something like (not sure about naming):

pub fn read_write_shared(txrx_buf: &'a mut [u8]) -> Self

let p = r.as_ptr();
std::slice::from_raw_parts(p, r.len())
};

SpidevTransfer::read_write(w, r)
},
}
}).collect();

// Execute transfer
self.0.transfer_multiple(&mut messages)?;

Ok(())
}
}

impl ops::Deref for Spidev {
type Target = spidev::Spidev;

Expand Down