-
Notifications
You must be signed in to change notification settings - Fork 72
I2C with interrupts #198
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
base: master
Are you sure you want to change the base?
I2C with interrupts #198
Changes from 4 commits
97cfd70
896fef3
f8f1690
ea41151
c8c2a5c
0f95321
b7a41f3
31b34ce
8798af7
658c372
ee3791e
ba02de0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,386 @@ | ||||||||||||||
//! Inter-Integrated Circuit (I2C) bus interruption | ||||||||||||||
|
||||||||||||||
use crate::i2c::{I2c, Instance, SclPin, SdaPin}; | ||||||||||||||
use crate::rcc::{Clocks, APB1}; | ||||||||||||||
use crate::time::Hertz; | ||||||||||||||
use core::fmt; | ||||||||||||||
|
||||||||||||||
/// I2c errors. | ||||||||||||||
#[derive(Debug, Copy, Clone)] | ||||||||||||||
pub enum I2cError { | ||||||||||||||
/// Device is busy, can't start something else | ||||||||||||||
DeviceBusy, | ||||||||||||||
/// Received a nack | ||||||||||||||
Nack, | ||||||||||||||
/// Error happened on the bus | ||||||||||||||
BusError, | ||||||||||||||
/// Arbitration loss, | ||||||||||||||
ArbitrationLoss, | ||||||||||||||
/// Overrun detected (salve mode) | ||||||||||||||
Overrun, | ||||||||||||||
/// Unable to compute the stop state because previous state was not expected | ||||||||||||||
StateError, | ||||||||||||||
/// Transfer complete status but nothing do do next | ||||||||||||||
TransferCompleteNoRead, | ||||||||||||||
/// Send buffer is empty | ||||||||||||||
TxBufferEmpty, | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
/// State of i2c communication. | ||||||||||||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||||||||||||||
pub enum State { | ||||||||||||||
/// I2c is Idle | ||||||||||||||
Idle, | ||||||||||||||
/// Send has started | ||||||||||||||
TxStart, | ||||||||||||||
/// Ready for send | ||||||||||||||
TxReady, | ||||||||||||||
/// Data written in send buffer | ||||||||||||||
TxSent, | ||||||||||||||
/// Send is complete, but device is not stopped | ||||||||||||||
TxComplete, | ||||||||||||||
/// Send is complete | ||||||||||||||
TxStop, | ||||||||||||||
/// Receive has started | ||||||||||||||
RxStart, | ||||||||||||||
/// Ready for receive | ||||||||||||||
RxReady, | ||||||||||||||
/// Received is complete | ||||||||||||||
RxStop, | ||||||||||||||
/// Nack for send | ||||||||||||||
TxNack, | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
impl core::fmt::Display for State { | ||||||||||||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||||||||||||||
write!(f, "{:?}", self) | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
/// I2c1Int provides interface to communicate with i2c devices using interruptions. | ||||||||||||||
pub struct I2cInt<I2C, PINS> { | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A Lines 201 to 204 in 9e29ec6
|
||||||||||||||
dev: I2c<I2C, PINS>, | ||||||||||||||
state: State, | ||||||||||||||
/// Last error that happened on i2c communications. | ||||||||||||||
pub last_error: Option<I2cError>, | ||||||||||||||
|
||||||||||||||
current_write_addr: Option<u8>, | ||||||||||||||
tx_ind: usize, | ||||||||||||||
tx_buf: Option<&'static [u8]>, | ||||||||||||||
rx_ind: usize, | ||||||||||||||
rx_buf: [u8; 256], // for the moment use a static buffer here. | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the long term, I'd like to see a user configurable slice (e.g. |
||||||||||||||
recv: Option<(u8, usize)>, | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
impl<I2C, SCL, SDA> I2cInt<I2C, (SCL, SDA)> | ||||||||||||||
where | ||||||||||||||
I2C: Instance, | ||||||||||||||
{ | ||||||||||||||
/// Configures the I2C peripheral to work in master mode | ||||||||||||||
pub fn new_int<F>( | ||||||||||||||
|
pub fn new_int<F>( | |
pub fn new<F>( |
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.
This should be more flexible in its configuration, I guess 🤔
Sh3Rm4n marked this conversation as resolved.
Show resolved
Hide resolved
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.
When specifying I2C addresses, always note, if the LSB is still part of the address or assumed to be the R/W bit, to void confusion. In this case, it is the latter.
Outdated
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.
interrupt
sound like, this function would return the type of the interrupt, which got triggered rather than reaction on the interrupt. I'd look for a more descriptive name. Maybe react_on_interrupt()
? Better suggestions are welcome.
Outdated
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 would prefer an if expression to change self.state
. It is more readable IMO
if isr.nackf().bit() { | |
self.dev.i2c.icr.write(|w| w.nackcf().bit(true)); | |
self.state = State::TxNack; | |
self.sate = if isr.nackf().bit() { | |
self.dev.i2c.icr.write(|w| w.nackcf().bit(true)); | |
State::TxNack |
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.
As you already have chosen a perfect name for a boolean function, I find this description redundant:
/// # Returns | |
/// true if busy, false if not. | |
pub fn is_busy(&mut self) -> bool { | |
pub fn is_busy(&mut self) -> bool { |
Outdated
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.
State
does include RX and TX states. Therefor this function name does not make sense?
pub fn get_tx_state(&self) -> State { | |
pub fn get_state(&self) -> State { |
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.
While I see, that this is useful, I would like to see that this function returns a type (or list of types e.g. enums) instead of an u32
.
What exactly does u32
mean for a potential user of this function? This does not seem clear right now.
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.
Why use a newly defined
I2cError
type, why don't reusecrate::i2c::I2cError
?EDIT: Well I kind of see the reason, but would like to see the error shared maybe?