-
Notifications
You must be signed in to change notification settings - Fork 201
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
add default SPI word types #320
Conversation
r? @ryankurte (rust-highfive has picked a reviewer for you, use r? to override) |
/// 9-bit SPI Word | ||
pub struct U9; | ||
/// 16-bit SPI Word | ||
pub struct U16; | ||
/// 18-bit SPI Word | ||
pub struct U18; |
There was a problem hiding this comment.
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);
There was a problem hiding this comment.
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;
}
There was a problem hiding this comment.
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);
There was a problem hiding this comment.
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
|
Thought about this reading #295
cc @eldruin @Sh3Rm4n @ryankurte
Should we restrict types be some trait?
Can also split
U16
toU16LE
andU16BE