Skip to content
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

Change transfer_multiple to not copy any buffers #8

Merged
merged 1 commit into from
Oct 27, 2016

Conversation

awelkie
Copy link
Contributor

@awelkie awelkie commented Oct 26, 2016

This changes the SpidevTransfer struct (which is now just an alias for spi_ioc_transfer) to hold references to the tx and rx buffers instead of holding the buffers themselves. This makes it possible to use transfers without copying or heap allocations.

@awelkie
Copy link
Contributor Author

awelkie commented Oct 26, 2016

Note that this does change the API, which you can see in my changes to the examples.

Copy link
Member

@posborne posborne left a comment

Choose a reason for hiding this comment

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

CI is failing due to PhantomData not having an implementation the Default trait prior to 1.6.0. I think I'll just go ahead and make the oldest supported version of rust be 1.7.0 as I have done with some other crates recently.


// optional overrides
pub speed_hz: u32,
pub delay_usecs: u16,
pub bits_per_word: u8,
pub cs_change: u8,
pub pad: u32,

tx_buf_ref: PhantomData<&'a [u8]>,
rx_buf_ref: PhantomData<&'b mut [u8]>,
Copy link
Member

Choose a reason for hiding this comment

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

Nice use of PhantomData here. Was concerned about the the transfer outlasting the lifetime of the buffers at first, but this handles that nicely.

@posborne
Copy link
Member

On the API change, I'll be sure to bump the version in semver appropriately.

@posborne posborne merged commit fbe6d06 into rust-embedded:master Oct 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants