Skip to content

add default SPI word types #320

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
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
## [Unreleased]

### Added
- Default SPI word types
- Added `Can` Controller Area Network traits.
- `Error` traits for SPI, I2C and Serial traits. The error types used in those must
implement these `Error` traits, which implies providing a conversion to a common
Expand Down
12 changes: 7 additions & 5 deletions src/spi/blocking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
//! traits. To save boilerplate when that's the case a `Default` marker trait may be provided.
//! Implementing that marker trait will opt in your type into a blanket implementation.

use super::U8;

/// Blocking transfer
pub trait Transfer<W> {
pub trait Transfer<W = U8> {
/// Error type
type Error: crate::spi::Error;

Expand All @@ -24,7 +26,7 @@ impl<T: Transfer<W>, W> Transfer<W> for &mut T {
}

/// Blocking write
pub trait Write<W> {
pub trait Write<W = U8> {
/// Error type
type Error: crate::spi::Error;

Expand All @@ -41,7 +43,7 @@ impl<T: Write<W>, W> Write<W> for &mut T {
}

/// Blocking write (iterator version)
pub trait WriteIter<W> {
pub trait WriteIter<W = U8> {
/// Error type
type Error: crate::spi::Error;

Expand All @@ -66,7 +68,7 @@ impl<T: WriteIter<W>, W> WriteIter<W> for &mut T {
///
/// This allows composition of SPI operations into a single bus transaction
#[derive(Debug, PartialEq)]
pub enum Operation<'a, W: 'static> {
pub enum Operation<'a, W: 'static = U8> {
/// Write data from the provided buffer, discarding read data
Write(&'a [W]),
/// Write data out while reading data into the provided buffer
Expand All @@ -75,7 +77,7 @@ pub enum Operation<'a, W: 'static> {

/// Transactional trait allows multiple actions to be executed
/// as part of a single SPI transaction
pub trait Transactional<W: 'static> {
pub trait Transactional<W: 'static = U8> {
/// Associated error type
type Error: crate::spi::Error;

Expand Down
9 changes: 9 additions & 0 deletions src/spi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,15 @@
pub mod blocking;
pub mod nb;

/// 8-bit SPI Word (default)
pub type U8 = u8;
/// 9-bit SPI Word
pub struct U9;
/// 16-bit SPI Word
pub struct U16;
/// 18-bit SPI Word
pub struct U18;
Comment on lines +16 to +21
Copy link

Choose a reason for hiding this comment

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

How are these zero-sized types supposed to be used? I think you'd need something more like

pub struct U9(pub u16);
pub type U16 = u16;
pub struct U18(pub u32);

Copy link
Member Author

@burrbull burrbull Nov 3, 2021

Choose a reason for hiding this comment

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

You are right it was stupid for me. But you suggestion will not work also.
It should be something like:

pub trait SpiWord {
   type Bytes;
}
pub struct U9;
impl SpiWord for U9 {
    type Bytes = u16;
}

Copy link

Choose a reason for hiding this comment

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

I see, this solution also works. However, now there isn't any implicit type annotation anymore so when calling, for example, Transfer::transfer() you'll have to be explicit about the datatype. This means you cannot call it like

spi.transfer(&buf);

but you'll have to call it as

Transfer::<U8>::transfer(&mut spi, &buf);

My suggestion was to have newtype wrappers around the odd bitwidths and then store an array of those. This would allow the compiler to infer the type automatically:

let buf: [U9; 16] = [U9(0), U9(23), ...];
spi.transfer(&buf);

Copy link
Member Author

Choose a reason for hiding this comment

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

Wrapping each element of array is a pain


/// Clock polarity
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum Polarity {
Expand Down
4 changes: 3 additions & 1 deletion src/spi/nb.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Serial Peripheral Interface

use super::U8;

/// Full duplex (master mode)
///
/// # Notes
Expand All @@ -16,7 +18,7 @@
///
/// - Some SPIs can work with 8-bit *and* 16-bit words. You can overload this trait with different
/// `Word` types to allow operation in both modes.
pub trait FullDuplex<Word> {
pub trait FullDuplex<Word = U8> {
/// An enumeration of SPI errors
type Error: crate::spi::Error;

Expand Down