i2c service code improvement
diff --git a/hal/blocking/src/i2c_hardware.rs b/hal/blocking/src/i2c_hardware.rs index c08d66b..11eb6d2 100644 --- a/hal/blocking/src/i2c_hardware.rs +++ b/hal/blocking/src/i2c_hardware.rs
@@ -136,9 +136,9 @@ pub mod slave { use super::*; - /// I2C slave events that can occur during slave operations + /// I2C slave events raised by the hardware ISR. #[derive(Copy, Clone, Debug, PartialEq, Eq)] - pub enum I2cSEvent { + pub enum I2cIsrEvent { /// Master is requesting to read from slave SlaveRdReq, /// Master is requesting to write to slave @@ -165,7 +165,7 @@ /// Number of bytes in transmit buffer pub tx_buffer_count: usize, /// Last slave event that occurred - pub last_event: Option<I2cSEvent>, + pub last_event: Option<I2cIsrEvent>, /// Whether an error condition exists pub error: bool, } @@ -272,7 +272,7 @@ /// Returns the most recent slave event, useful for debugging /// and state tracking. May return None if no events have occurred /// since reset or if the hardware doesn't track this information. - fn last_slave_event(&self) -> Option<I2cSEvent>; + fn last_slave_event(&self) -> Option<I2cIsrEvent>; } /// Blocking slave event handling (sync pattern) @@ -289,7 +289,7 @@ /// with master transactions. fn wait_for_slave_event( &mut self, - expected_event: I2cSEvent, + expected_event: I2cIsrEvent, timeout_ms: u32, ) -> Result<bool, Self::Error>; @@ -298,15 +298,17 @@ /// Blocks until any slave event occurs or timeout expires. /// Returns the event that occurred, or None if timeout expired. /// Useful when any event needs to be processed synchronously. - fn wait_for_any_event(&mut self, timeout_ms: u32) - -> Result<Option<I2cSEvent>, Self::Error>; + fn wait_for_any_event( + &mut self, + timeout_ms: u32, + ) -> Result<Option<I2cIsrEvent>, Self::Error>; /// Handle a specific slave event with blocking semantics /// /// Processes a slave event and may block if the event handling /// requires waiting for hardware completion. This is different /// from the polling version which always returns immediately. - fn handle_slave_event_blocking(&mut self, event: I2cSEvent) -> Result<(), Self::Error>; + fn handle_slave_event_blocking(&mut self, event: I2cIsrEvent) -> Result<(), Self::Error>; } /// Complete slave implementation combining core functionality
diff --git a/hal/nb/src/i2c_hardware.rs b/hal/nb/src/i2c_hardware.rs index 89c47bb..4ff3290 100644 --- a/hal/nb/src/i2c_hardware.rs +++ b/hal/nb/src/i2c_hardware.rs
@@ -15,21 +15,21 @@ //! //! ```rust,no_run //! use openprot_hal_nb::i2c_hardware::I2cSlaveEventPolling; -//! use openprot_hal_blocking::i2c_hardware::slave::I2cSEvent; +//! use openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent; //! //! fn poll_slave_events<T: I2cSlaveEventPolling>(slave: &mut T) -> Result<(), T::Error> { //! // Non-blocking check for events in main loop //! while let Some(event) = slave.poll_slave_events()? { //! match event { -//! I2cSEvent::SlaveWrReq => { +//! I2cIsrEvent::SlaveWrReq => { //! println!("Master wants to write to us"); //! slave.handle_slave_event(event)?; //! }, -//! I2cSEvent::SlaveRdReq => { +//! I2cIsrEvent::SlaveRdReq => { //! println!("Master wants to read from us"); //! slave.handle_slave_event(event)?; //! }, -//! I2cSEvent::SlaveStop => { +//! I2cIsrEvent::SlaveStop => { //! println!("Transaction complete"); //! slave.handle_slave_event(event)?; //! }, @@ -46,17 +46,17 @@ //! //! ```rust,no_run //! use openprot_hal_nb::i2c_hardware::I2cSlaveEventPolling; -//! use openprot_hal_blocking::i2c_hardware::slave::I2cSEvent; +//! use openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent; //! //! // Called from interrupt service routine //! fn i2c_slave_isr<T: I2cSlaveEventPolling>(slave: &mut T) { //! // Check specific events without blocking -//! if slave.is_event_pending(I2cSEvent::SlaveWrReq).unwrap_or(false) { -//! let _ = slave.handle_slave_event(I2cSEvent::SlaveWrReq); +//! if slave.is_event_pending(I2cIsrEvent::SlaveWrReq).unwrap_or(false) { +//! let _ = slave.handle_slave_event(I2cIsrEvent::SlaveWrReq); //! } //! -//! if slave.is_event_pending(I2cSEvent::SlaveRdReq).unwrap_or(false) { -//! let _ = slave.handle_slave_event(I2cSEvent::SlaveRdReq); +//! if slave.is_event_pending(I2cIsrEvent::SlaveRdReq).unwrap_or(false) { +//! let _ = slave.handle_slave_event(I2cIsrEvent::SlaveRdReq); //! } //! } //! ``` @@ -85,7 +85,7 @@ use embedded_hal::i2c::{AddressMode, SevenBitAddress}; use openprot_hal_blocking::i2c_hardware::slave::{ - I2cSEvent, I2cSlaveBuffer, I2cSlaveCore, I2cSlaveInterrupts, + I2cIsrEvent, I2cSlaveBuffer, I2cSlaveCore, I2cSlaveInterrupts, }; /// Non-blocking slave event handling (async/polling pattern) @@ -100,17 +100,17 @@ /// /// ```rust,no_run /// use openprot_hal_nb::i2c_hardware::I2cSlaveEventPolling; -/// use openprot_hal_blocking::i2c_hardware::slave::I2cSEvent; +/// use openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent; /// /// fn handle_i2c_events<T: I2cSlaveEventPolling>(slave: &mut T) -> Result<(), T::Error> { /// // Check for events without blocking /// if let Some(event) = slave.poll_slave_events()? { /// match event { -/// I2cSEvent::SlaveWrReq => { +/// I2cIsrEvent::SlaveWrReq => { /// // Master wants to write - prepare to receive /// slave.handle_slave_event(event)?; /// }, -/// I2cSEvent::SlaveRdReq => { +/// I2cIsrEvent::SlaveRdReq => { /// // Master wants to read - prepare data /// slave.handle_slave_event(event)?; /// }, @@ -125,13 +125,13 @@ /// /// ```rust,no_run /// use openprot_hal_nb::i2c_hardware::I2cSlaveEventPolling; -/// use openprot_hal_blocking::i2c_hardware::slave::I2cSEvent; +/// use openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent; /// /// // Fast ISR that doesn't block /// fn i2c_isr<T: I2cSlaveEventPolling>(slave: &mut T) { /// // Quick event check -/// if slave.is_event_pending(I2cSEvent::SlaveWrReq).unwrap_or(false) { -/// let _ = slave.handle_slave_event(I2cSEvent::SlaveWrReq); +/// if slave.is_event_pending(I2cIsrEvent::SlaveWrReq).unwrap_or(false) { +/// let _ = slave.handle_slave_event(I2cIsrEvent::SlaveWrReq); /// } /// } /// ``` @@ -152,7 +152,7 @@ /// /// ```rust,no_run /// use openprot_hal_nb::i2c_hardware::I2cSlaveEventPolling; - /// use openprot_hal_blocking::i2c_hardware::slave::I2cSEvent; + /// use openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent; /// /// fn check_events<T: I2cSlaveEventPolling>(slave: &mut T) -> Result<(), T::Error> { /// if let Some(event) = slave.poll_slave_events()? { @@ -163,7 +163,7 @@ /// Ok(()) /// } /// ``` - fn poll_slave_events(&mut self) -> Result<Option<I2cSEvent>, Self::Error>; + fn poll_slave_events(&mut self) -> Result<Option<I2cIsrEvent>, Self::Error>; /// Handle a specific slave event (called from ISR or event loop) /// @@ -183,15 +183,15 @@ /// /// ```rust,no_run /// use openprot_hal_nb::i2c_hardware::I2cSlaveEventPolling; - /// use openprot_hal_blocking::i2c_hardware::slave::I2cSEvent; + /// use openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent; /// /// fn handle_event<T: I2cSlaveEventPolling>(slave: &mut T) -> Result<(), T::Error> { - /// slave.handle_slave_event(I2cSEvent::SlaveWrReq)?; + /// slave.handle_slave_event(I2cIsrEvent::SlaveWrReq)?; /// println!("Write request handled"); /// Ok(()) /// } /// ``` - fn handle_slave_event(&mut self, event: I2cSEvent) -> Result<(), Self::Error>; + fn handle_slave_event(&mut self, event: I2cIsrEvent) -> Result<(), Self::Error>; /// Non-blocking check if a specific event is pending /// @@ -212,16 +212,16 @@ /// /// ```rust,no_run /// use openprot_hal_nb::i2c_hardware::I2cSlaveEventPolling; - /// use openprot_hal_blocking::i2c_hardware::slave::I2cSEvent; + /// use openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent; /// /// fn check_specific_event<T: I2cSlaveEventPolling>(slave: &T) -> Result<(), T::Error> { - /// if slave.is_event_pending(I2cSEvent::SlaveWrReq)? { + /// if slave.is_event_pending(I2cIsrEvent::SlaveWrReq)? { /// println!("Write request is pending"); /// } /// Ok(()) /// } /// ``` - fn is_event_pending(&self, event: I2cSEvent) -> Result<bool, Self::Error>; + fn is_event_pending(&self, event: I2cIsrEvent) -> Result<bool, Self::Error>; } /// Complete non-blocking slave implementation @@ -265,7 +265,7 @@ /// /// ```rust,no_run /// use openprot_hal_nb::i2c_hardware::I2cSlaveNonBlocking; -/// use openprot_hal_blocking::i2c_hardware::slave::I2cSEvent; +/// use openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent; /// /// fn main_loop<T: I2cSlaveNonBlocking>(mut slave: T) -> Result<(), T::Error> { /// loop {
diff --git a/platform/impls/baremetal/mock/src/i2c_hardware.rs b/platform/impls/baremetal/mock/src/i2c_hardware.rs index 580e501..2057aeb 100644 --- a/platform/impls/baremetal/mock/src/i2c_hardware.rs +++ b/platform/impls/baremetal/mock/src/i2c_hardware.rs
@@ -131,16 +131,16 @@ //! //! ```text //! use openprot_platform_mock::i2c_hardware::MockI2cHardware; -//! use openprot_hal_blocking::i2c_hardware::slave::I2cSEvent; +//! use openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent; //! //! let mut mock = MockI2cHardware::new(); //! //! // Inject single event for testing (mock only stores most recent event) -//! mock.inject_slave_event(I2cSEvent::SlaveWrReq); +//! mock.inject_slave_event(I2cIsrEvent::SlaveWrReq); //! //! // Poll for events (non-blocking) //! match mock.poll_slave_events() { -//! Ok(Some(I2cSEvent::SlaveWrReq)) => { +//! Ok(Some(I2cIsrEvent::SlaveWrReq)) => { //! // Event received as expected //! }, //! Ok(Some(_)) => { @@ -340,7 +340,7 @@ /// Number of valid bytes in transmit buffer (8 bytes: usize on 64-bit) slave_tx_count: usize, /// Most recent slave event that occurred (1 byte: `Option<enum>`) - last_slave_event: Option<openprot_hal_blocking::i2c_hardware::slave::I2cSEvent>, + last_slave_event: Option<openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent>, } impl MockI2cHardware { @@ -810,7 +810,7 @@ } } - fn last_slave_event(&self) -> Option<openprot_hal_blocking::i2c_hardware::slave::I2cSEvent> { + fn last_slave_event(&self) -> Option<openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent> { self.last_slave_event } } @@ -870,7 +870,7 @@ /// No complex event queue management. pub fn inject_slave_event( &mut self, - event: openprot_hal_blocking::i2c_hardware::slave::I2cSEvent, + event: openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent, ) { self.last_slave_event = Some(event); } @@ -881,7 +881,7 @@ /// Simplified version of event polling. pub fn poll_slave_events( &mut self, - ) -> Result<Option<openprot_hal_blocking::i2c_hardware::slave::I2cSEvent>, MockI2cError> { + ) -> Result<Option<openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent>, MockI2cError> { self.check_success()?; let event = self.last_slave_event.take(); Ok(event) @@ -892,7 +892,7 @@ /// Simplified event checking without complex queue management. pub fn is_event_pending_nb( &self, - event: openprot_hal_blocking::i2c_hardware::slave::I2cSEvent, + event: openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent, ) -> Result<bool, MockI2cError> { if self.config.success { Ok(self.last_slave_event == Some(event)) @@ -906,18 +906,18 @@ /// Processes a slave event with mock behavior. pub fn handle_slave_event_nb( &mut self, - event: openprot_hal_blocking::i2c_hardware::slave::I2cSEvent, + event: openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent, ) -> Result<(), MockI2cError> { self.check_success()?; self.last_slave_event = Some(event); // Simulate event handling based on event type match event { - openprot_hal_blocking::i2c_hardware::slave::I2cSEvent::SlaveWrRecvd => { + openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent::SlaveWrRecvd => { // Simulate receiving data using safe injection self.inject_slave_data(&[0xAA, 0xBB, 0xCC]); } - openprot_hal_blocking::i2c_hardware::slave::I2cSEvent::SlaveRdReq => { + openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent::SlaveRdReq => { // Prepare response data using safe method use openprot_hal_blocking::i2c_hardware::slave::I2cSlaveBuffer; match self.write_slave_response(&[0x11, 0x22, 0x33]) { @@ -1238,7 +1238,7 @@ self.base_hardware.slave_status() } - fn last_slave_event(&self) -> Option<openprot_hal_blocking::i2c_hardware::slave::I2cSEvent> { + fn last_slave_event(&self) -> Option<openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent> { self.base_hardware.last_slave_event() } } @@ -1259,7 +1259,7 @@ /// Inject slave event (for testing) pub fn inject_slave_event( &mut self, - event: openprot_hal_blocking::i2c_hardware::slave::I2cSEvent, + event: openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent, ) { self.base_hardware.inject_slave_event(event); } @@ -1267,7 +1267,7 @@ /// Poll slave events (for testing) pub fn poll_slave_events( &mut self, - ) -> Result<Option<openprot_hal_blocking::i2c_hardware::slave::I2cSEvent>, MockI2cError> { + ) -> Result<Option<openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent>, MockI2cError> { self.base_hardware.poll_slave_events() } } @@ -1465,14 +1465,14 @@ fn test_slave_events() { let mut mock = MockI2cHardware::new(); - use openprot_hal_blocking::i2c_hardware::slave::{I2cSEvent, I2cSlaveInterrupts}; + use openprot_hal_blocking::i2c_hardware::slave::{I2cIsrEvent, I2cSlaveInterrupts}; // Test event injection and polling - mock.inject_slave_event(I2cSEvent::SlaveWrReq); + mock.inject_slave_event(I2cIsrEvent::SlaveWrReq); // Test event polling match mock.poll_slave_events() { - Ok(event) => assert_eq!(event, Some(I2cSEvent::SlaveWrReq)), + Ok(event) => assert_eq!(event, Some(I2cIsrEvent::SlaveWrReq)), Err(_) => { panic!("Failed to poll events"); } @@ -1486,19 +1486,19 @@ } // Test event handling - match mock.handle_slave_event_nb(I2cSEvent::SlaveWrRecvd) { + match mock.handle_slave_event_nb(I2cIsrEvent::SlaveWrRecvd) { Ok(()) => {} Err(_) => { panic!("Failed to handle event"); } } - assert_eq!(mock.last_slave_event(), Some(I2cSEvent::SlaveWrRecvd)); + assert_eq!(mock.last_slave_event(), Some(I2cIsrEvent::SlaveWrRecvd)); // Test slave status match mock.slave_status() { Ok(status) => { assert!(!status.enabled); // Not enabled by default - assert_eq!(status.last_event, Some(I2cSEvent::SlaveWrRecvd)); + assert_eq!(status.last_event, Some(I2cIsrEvent::SlaveWrRecvd)); } Err(_) => { panic!("Failed to get status"); @@ -1510,10 +1510,10 @@ fn test_slave_event_pending_check() { let mut mock = MockI2cHardware::new(); - use openprot_hal_blocking::i2c_hardware::slave::I2cSEvent; + use openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent; // Initially no events pending - match mock.is_event_pending_nb(I2cSEvent::SlaveWrReq) { + match mock.is_event_pending_nb(I2cIsrEvent::SlaveWrReq) { Ok(pending) => assert!(!pending), Err(_) => { panic!("Failed to check pending"); @@ -1521,14 +1521,14 @@ } // Inject an event - mock.inject_slave_event(I2cSEvent::SlaveWrReq); - match mock.is_event_pending_nb(I2cSEvent::SlaveWrReq) { + mock.inject_slave_event(I2cIsrEvent::SlaveWrReq); + match mock.is_event_pending_nb(I2cIsrEvent::SlaveWrReq) { Ok(pending) => assert!(pending), Err(_) => { panic!("Failed to check pending"); } } - match mock.is_event_pending_nb(I2cSEvent::SlaveRdReq) { + match mock.is_event_pending_nb(I2cIsrEvent::SlaveRdReq) { Ok(pending) => assert!(!pending), Err(_) => { panic!("Failed to check pending"); @@ -1542,7 +1542,7 @@ panic!("Failed to poll events"); } } - match mock.is_event_pending_nb(I2cSEvent::SlaveWrReq) { + match mock.is_event_pending_nb(I2cIsrEvent::SlaveWrReq) { Ok(pending) => assert!(!pending), Err(_) => { panic!("Failed to check pending"); @@ -1567,7 +1567,7 @@ assert!(mock.poll_slave_events().is_err()); assert!(mock .handle_slave_event_nb( - openprot_hal_blocking::i2c_hardware::slave::I2cSEvent::SlaveWrReq + openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent::SlaveWrReq ) .is_err()); }
diff --git a/services/i2c/README.md b/services/i2c/README.md index ccf7381..8f92650 100644 --- a/services/i2c/README.md +++ b/services/i2c/README.md
@@ -98,11 +98,11 @@ loop { // Wait for Signals::USER on channel object_wait_signal(&channel, USER_BIT)?; - + let event = client.slave_receive(&mut buf)?; // Use event.source_address for per-requester state // Use event.kind to distinguish read/write/stop - + // Stage response for master's next read client.slave_set_response(&buf[..event.data_len])?; }
diff --git a/services/i2c/api/src/lib.rs b/services/i2c/api/src/lib.rs index ccbaead..25698e5 100644 --- a/services/i2c/api/src/lib.rs +++ b/services/i2c/api/src/lib.rs
@@ -7,6 +7,10 @@ pub mod seam; pub mod transport; -pub use protocol::*; +#[doc(inline)] +pub use protocol::{ + I2cError, I2cOp, I2cOpDesc, I2cOpKind, I2cRequestHeader, I2cResponseHeader, SlaveEvent, + MAX_OPS, MAX_PAYLOAD_SIZE, +}; pub use seam::I2cSlaveEvent; pub use transport::{Transport, TransportError};
diff --git a/services/i2c/api/src/protocol.rs b/services/i2c/api/src/protocol.rs index 2e1075e..5ee6302 100644 --- a/services/i2c/api/src/protocol.rs +++ b/services/i2c/api/src/protocol.rs
@@ -28,6 +28,7 @@ /// Max number of `Operation`s in one transaction. pub const MAX_OPS: usize = 16; +#[non_exhaustive] #[repr(u8)] #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum I2cOp { @@ -84,9 +85,10 @@ } /// Kind of event returned by `SlaveWaitEvent`. +#[non_exhaustive] #[repr(u8)] #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum SlaveEventKind { +pub enum SlaveEvent { /// Master wrote data to our slave address. DataReceived = 0x00, /// Master issued a read from our slave address. @@ -95,7 +97,7 @@ Stop = 0x02, } -impl TryFrom<u8> for SlaveEventKind { +impl TryFrom<u8> for SlaveEvent { type Error = I2cError; fn try_from(value: u8) -> Result<Self, Self::Error> { @@ -109,6 +111,7 @@ } /// Kind of a single bus operation within a transaction. +#[non_exhaustive] #[repr(u8)] #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum I2cOpKind { @@ -132,10 +135,10 @@ /// /// The bus-failure variants map 1-to-1 onto `embedded_hal::i2c::ErrorKind` /// (see [`crate::seam::error_kind`]). +#[non_exhaustive] #[repr(u8)] #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum I2cError { - Success = 0x00, InvalidOperation = 0x01, BufferTooSmall = 0x02, TooManyOperations = 0x03, @@ -157,7 +160,6 @@ impl From<u8> for I2cError { fn from(value: u8) -> Self { match value { - 0x00 => Self::Success, 0x01 => Self::InvalidOperation, 0x02 => Self::BufferTooSmall, 0x03 => Self::TooManyOperations, @@ -174,21 +176,42 @@ } } +impl core::fmt::Display for I2cError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Self::InvalidOperation => f.write_str("invalid i2c operation"), + Self::BufferTooSmall => f.write_str("buffer too small"), + Self::TooManyOperations => f.write_str("too many operations"), + Self::AddressNack => f.write_str("nack on address phase"), + Self::DataNack => f.write_str("nack on data byte"), + Self::Nack => f.write_str("nack (phase unknown)"), + Self::ArbitrationLoss => f.write_str("arbitration loss"), + Self::Bus => f.write_str("bus error"), + Self::Overrun => f.write_str("overrun"), + Self::Timeout => f.write_str("timeout"), + Self::NoData => f.write_str("no slave data pending"), + Self::InternalError => f.write_str("internal i2c server error"), + } + } +} + +impl core::error::Error for I2cError {} + #[repr(C, packed)] #[derive(Debug, Clone, Copy, FromBytes, IntoBytes, Immutable, KnownLayout)] pub struct I2cRequestHeader { - pub op_code: u8, - pub flags: u8, + pub(crate) op_code: u8, + pub(crate) flags: u8, /// Target address. 7-bit address in the low 7 bits; 10-bit reserved. - pub address: u16, + pub(crate) address: u16, /// Number of `I2cOpDesc` records that follow this header. - pub op_count: u16, + pub(crate) op_count: u16, /// Total bytes after the header (op descriptors + inline write data). - pub payload_len: u16, + pub(crate) payload_len: u16, } impl I2cRequestHeader { - pub const SIZE: usize = 8; + pub const SIZE: usize = core::mem::size_of::<Self>(); pub fn new(op: I2cOp, address: u16, op_count: u16, payload_len: u16) -> Self { Self { @@ -223,13 +246,13 @@ #[repr(C, packed)] #[derive(Debug, Clone, Copy, FromBytes, IntoBytes, Immutable, KnownLayout)] pub struct I2cOpDesc { - pub kind: u8, - pub reserved: u8, - pub len: u16, + pub(crate) kind: u8, + pub(crate) reserved: u8, + pub(crate) len: u16, } impl I2cOpDesc { - pub const SIZE: usize = 4; + pub const SIZE: usize = core::mem::size_of::<Self>(); pub fn new(kind: I2cOpKind, len: u16) -> Self { Self { @@ -251,18 +274,18 @@ #[repr(C, packed)] #[derive(Debug, Clone, Copy, FromBytes, IntoBytes, Immutable, KnownLayout)] pub struct I2cResponseHeader { - pub status: u8, - pub reserved: u8, + pub(crate) status: u8, + pub(crate) reserved: u8, /// Total read-payload bytes following this header. - pub payload_len: u16, + pub(crate) payload_len: u16, } impl I2cResponseHeader { - pub const SIZE: usize = 4; + pub const SIZE: usize = core::mem::size_of::<Self>(); pub fn success(payload_len: u16) -> Self { Self { - status: I2cError::Success as u8, + status: 0, reserved: 0, payload_len: payload_len.to_le(), } @@ -277,7 +300,7 @@ } pub fn is_success(&self) -> bool { - self.status == I2cError::Success as u8 + self.status == 0 } pub fn error_code(&self) -> I2cError { @@ -335,9 +358,11 @@ #[test] fn error_and_op_byte_mapping_is_stable() { - for raw in 0u8..=0x0B { + for raw in 0x01u8..=0x0B { assert_eq!(I2cError::from(raw) as u8, raw); } + // 0x00 is the success sentinel on the wire — it is not an I2cError variant. + assert_eq!(I2cError::from(0x00), I2cError::InternalError); assert_eq!(I2cError::from(0x0B), I2cError::NoData); assert_eq!(I2cError::from(0xFF), I2cError::InternalError); assert_eq!(I2cError::from(0x42), I2cError::InternalError); @@ -364,17 +389,14 @@ #[test] fn slave_event_kinds_roundtrip() { for (raw, kind) in [ - (0x00u8, SlaveEventKind::DataReceived), - (0x01, SlaveEventKind::ReadRequest), - (0x02, SlaveEventKind::Stop), + (0x00u8, SlaveEvent::DataReceived), + (0x01, SlaveEvent::ReadRequest), + (0x02, SlaveEvent::Stop), ] { - assert_eq!(SlaveEventKind::try_from(raw), Ok(kind)); + assert_eq!(SlaveEvent::try_from(raw), Ok(kind)); assert_eq!(kind as u8, raw); } - assert_eq!( - SlaveEventKind::try_from(0xFF), - Err(I2cError::InvalidOperation) - ); + assert_eq!(SlaveEvent::try_from(0xFF), Err(I2cError::InvalidOperation)); } #[test]
diff --git a/services/i2c/api/src/seam.rs b/services/i2c/api/src/seam.rs index 75f2af4..0b30eb7 100644 --- a/services/i2c/api/src/seam.rs +++ b/services/i2c/api/src/seam.rs
@@ -20,7 +20,7 @@ // Target-side traits are re-exported from `openprot_hal_blocking`. // The runtime stays generic, and each backend forwards to its platform driver. pub use openprot_hal_blocking::i2c_hardware::slave::{ - I2cSEvent, I2cSlaveBuffer, I2cSlaveCore, I2cSlaveInterrupts, SlaveStatus, + I2cIsrEvent, I2cSlaveBuffer, I2cSlaveCore, I2cSlaveInterrupts, SlaveStatus, }; pub use openprot_hal_blocking::i2c_hardware::{I2cBusRecovery, I2cHardwareCore}; @@ -33,10 +33,10 @@ pub trait I2cSlaveEvent: I2cSlaveBuffer { /// Return the next slave event and rx length, if any. /// Default impl uses `poll_slave_data()` and reports DataReceived kind. - fn try_next_slave_event(&mut self) -> Result<Option<(I2cSEvent, usize)>, Self::Error> { + fn try_next_slave_event(&mut self) -> Result<Option<(I2cIsrEvent, usize)>, Self::Error> { Ok(self .poll_slave_data()? - .map(|n| (I2cSEvent::SlaveWrRecvd, n))) + .map(|n| (I2cIsrEvent::SlaveWrRecvd, n))) } }
diff --git a/services/i2c/api/src/transport.rs b/services/i2c/api/src/transport.rs index 01238e3..f218a8a 100644 --- a/services/i2c/api/src/transport.rs +++ b/services/i2c/api/src/transport.rs
@@ -21,12 +21,23 @@ /// Why a transport round-trip failed. Deliberately tiny and transport-neutral; /// i2c-level status travels inside the response payload, not here. +#[non_exhaustive] #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum TransportError { /// The underlying channel/syscall/loopback call failed. Failed, } +impl core::fmt::Display for TransportError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Self::Failed => f.write_str("i2c transport round-trip failed"), + } + } +} + +impl core::error::Error for TransportError {} + /// Bytes-in → bytes-out, exactly one round-trip. /// /// `transact` writes the response into `resp` and returns its length. The
diff --git a/services/i2c/client/src/lib.rs b/services/i2c/client/src/lib.rs index 6386121..a11db95 100644 --- a/services/i2c/client/src/lib.rs +++ b/services/i2c/client/src/lib.rs
@@ -20,10 +20,12 @@ use i2c_api::seam::{error_kind, ErrorKind, ErrorType, I2c, Operation, SevenBitAddress}; use i2c_api::{ - I2cError, I2cOp, I2cOpDesc, I2cOpKind, I2cRequestHeader, I2cResponseHeader, SlaveEventKind, + I2cError, I2cOp, I2cOpDesc, I2cOpKind, I2cRequestHeader, I2cResponseHeader, SlaveEvent, Transport, TransportError, MAX_OPS, MAX_PAYLOAD_SIZE, }; +// One IPC message fits in a single 512-byte channel buffer on the server side. +// Raising this requires a matching change to the server's receive buffer. const MAX_BUF_SIZE: usize = 512; #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -41,12 +43,36 @@ #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct SlaveReceiveEvent { /// Kind of event that triggered this receive (DataReceived, ReadRequest, Stop). - pub kind: SlaveEventKind, + pub kind: SlaveEvent, /// Source I2C address (7-bit) of the master that wrote to us. - /// May be set to 0xFF if unavailable (hardware doesn't track it). - pub source_address: u8, + /// `None` if the hardware did not capture it. + pub source_address: Option<SevenBitAddress>, /// Number of data bytes in the buffer. pub data_len: usize, + /// True if the latched buffer exceeded `buf` and was truncated. + pub truncated: bool, +} + +impl core::fmt::Display for ClientError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Self::Transport(e) => write!(f, "i2c transport error: {e}"), + Self::ServerError(e) => write!(f, "i2c server error: {e}"), + Self::InvalidResponse => f.write_str("malformed i2c response"), + Self::BufferTooSmall => f.write_str("transaction exceeds one round-trip buffer"), + Self::TooManyOperations => f.write_str("too many operations in one transaction"), + } + } +} + +impl core::error::Error for ClientError { + fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { + match self { + Self::Transport(e) => Some(e), + Self::ServerError(e) => Some(e), + _ => None, + } + } } impl From<TransportError> for ClientError { @@ -74,6 +100,7 @@ } impl<T: Transport> I2cClient<T> { + /// Create a client bound to `transport`. pub const fn new(transport: T) -> Self { Self { transport } } @@ -126,17 +153,32 @@ } /// Set this bus's slave (target) address. + /// + /// # Errors + /// - [`ClientError::Transport`] — the IPC round-trip failed. + /// - [`ClientError::ServerError`] — the server rejected the address. + /// - [`ClientError::InvalidResponse`] — the response was malformed. pub fn configure_slave(&mut self, address: SevenBitAddress) -> Result<(), ClientError> { self.slave_cmd(I2cOp::ConfigureSlave, address as u16, 0, None) .map(|_| ()) } /// Enter slave mode (start ACKing the configured address). + /// + /// # Errors + /// - [`ClientError::Transport`] — the IPC round-trip failed. + /// - [`ClientError::ServerError`] — the server could not enable slave mode. + /// - [`ClientError::InvalidResponse`] — the response was malformed. pub fn enable_slave(&mut self) -> Result<(), ClientError> { self.slave_cmd(I2cOp::EnableSlave, 0, 0, None).map(|_| ()) } /// Leave slave mode. + /// + /// # Errors + /// - [`ClientError::Transport`] — the IPC round-trip failed. + /// - [`ClientError::ServerError`] — the server could not disable slave mode. + /// - [`ClientError::InvalidResponse`] — the response was malformed. pub fn disable_slave(&mut self) -> Result<(), ClientError> { self.slave_cmd(I2cOp::DisableSlave, 0, 0, None).map(|_| ()) } @@ -144,12 +186,22 @@ /// Arm interrupt-driven slave-RX notification. After this the server /// raises `Signals::USER` on this bus's channel when data is latched; /// the consumer then calls [`slave_receive`](Self::slave_receive). + /// + /// # Errors + /// - [`ClientError::Transport`] — the IPC round-trip failed. + /// - [`ClientError::ServerError`] — the server could not arm the notification. + /// - [`ClientError::InvalidResponse`] — the response was malformed. pub fn enable_notification(&mut self) -> Result<(), ClientError> { self.slave_cmd(I2cOp::EnableSlaveNotification, 0, 0, None) .map(|_| ()) } /// Disarm slave-RX notification (also drops any latched buffer). + /// + /// # Errors + /// - [`ClientError::Transport`] — the IPC round-trip failed. + /// - [`ClientError::ServerError`] — the server could not disarm the notification. + /// - [`ClientError::InvalidResponse`] — the response was malformed. pub fn disable_notification(&mut self) -> Result<(), ClientError> { self.slave_cmd(I2cOp::DisableSlaveNotification, 0, 0, None) .map(|_| ()) @@ -160,6 +212,11 @@ /// Call this after a `Signals::USER` wake on the channel. /// /// Response payload format: [kind (1), source_addr (1), data (0..)] + /// + /// # Errors + /// - [`ClientError::Transport`] — the IPC round-trip failed. + /// - [`ClientError::ServerError`]`(`[`I2cError::NoData`]`)` — nothing is latched yet. + /// - [`ClientError::InvalidResponse`] — the response was malformed. pub fn slave_receive(&mut self, buf: &mut [u8]) -> Result<SlaveReceiveEvent, ClientError> { let max = (buf.len().saturating_sub(2)).min(MAX_PAYLOAD_SIZE) as u16; let mut resp = [0u8; I2cResponseHeader::SIZE + MAX_PAYLOAD_SIZE]; @@ -197,7 +254,7 @@ let source_addr = resp[payload_offset + 1]; let data_len = payload_len - 2; - let kind = SlaveEventKind::try_from(kind_byte).map_err(|_| ClientError::InvalidResponse)?; + let kind = SlaveEvent::try_from(kind_byte).map_err(|_| ClientError::InvalidResponse)?; // Copy data into the caller's buffer. let copy = data_len.min(buf.len()); @@ -207,8 +264,13 @@ Ok(SlaveReceiveEvent { kind, - source_address: source_addr, - data_len, + source_address: if source_addr == 0xFF { + None + } else { + Some(source_addr) + }, + data_len: copy, + truncated: data_len > copy, }) } @@ -217,6 +279,12 @@ /// /// NOTE: not required for MCTP-over-I2C. Provided for testing slave-TX /// and register-echo patterns only. + /// + /// # Errors + /// - [`ClientError::BufferTooSmall`] — `data` exceeds the one round-trip buffer. + /// - [`ClientError::Transport`] — the IPC round-trip failed. + /// - [`ClientError::ServerError`] — the server rejected the TX buffer. + /// - [`ClientError::InvalidResponse`] — the response was malformed. pub fn slave_set_response(&mut self, data: &[u8]) -> Result<(), ClientError> { let hdr = I2cRequestHeader::new(I2cOp::SlaveSetResponse, 0, 0, data.len() as u16); let req_len = I2cRequestHeader::SIZE + data.len(); @@ -341,6 +409,14 @@ } impl<T: Transport> I2c<SevenBitAddress> for I2cClient<T> { + /// Execute one atomic I2C transaction (address + ordered read/write ops). + /// + /// # Errors + /// - [`ClientError::TooManyOperations`] — more than `MAX_OPS` ops supplied. + /// - [`ClientError::BufferTooSmall`] — total read or write payload exceeds one round-trip buffer. + /// - [`ClientError::Transport`] — the IPC round-trip failed. + /// - [`ClientError::ServerError`] — the server reported a bus-level error (NACK, timeout, …). + /// - [`ClientError::InvalidResponse`] — the response was malformed or payload length mismatch. fn transaction( &mut self, address: SevenBitAddress,
diff --git a/services/i2c/server-runtime/src/lib.rs b/services/i2c/server-runtime/src/lib.rs index bda63a1..a8ced56 100644 --- a/services/i2c/server-runtime/src/lib.rs +++ b/services/i2c/server-runtime/src/lib.rs
@@ -25,11 +25,9 @@ #![no_std] use i2c_api::seam::{ - I2c, I2cBusRecovery, I2cSEvent, I2cSlaveBuffer, I2cSlaveEvent, SevenBitAddress, + I2c, I2cBusRecovery, I2cIsrEvent, I2cSlaveBuffer, I2cSlaveEvent, SevenBitAddress, }; -use i2c_api::{ - I2cError, I2cOp, I2cRequestHeader, I2cResponseHeader, SlaveEventKind, MAX_PAYLOAD_SIZE, -}; +use i2c_api::{I2cError, I2cOp, I2cRequestHeader, I2cResponseHeader, SlaveEvent, MAX_PAYLOAD_SIZE}; use i2c_server::slave::dispatch_slave; use i2c_server::{dispatch, MAX_BUF_SIZE}; use userspace::syscall::{self, Signals}; @@ -52,7 +50,7 @@ rx_source: u8, /// Event kind that triggered the latch (DataReceived, ReadRequest, Stop). /// Only meaningful when rx_len > 0 or notification was armed. - rx_event_kind: SlaveEventKind, + rx_event_kind: SlaveEvent, } impl<B> Bus<B> { @@ -65,7 +63,7 @@ rx: [0u8; MAX_PAYLOAD_SIZE], rx_len: 0, rx_source: 0, - rx_event_kind: SlaveEventKind::DataReceived, + rx_event_kind: SlaveEvent::DataReceived, } } } @@ -132,13 +130,13 @@ Ok(Some((kind, _))) => { // Store the actual hardware event kind bus.rx_event_kind = match kind { - I2cSEvent::SlaveWrRecvd => SlaveEventKind::DataReceived, - I2cSEvent::SlaveRdReq => SlaveEventKind::ReadRequest, - I2cSEvent::SlaveStop => SlaveEventKind::Stop, - _ => SlaveEventKind::DataReceived, + I2cIsrEvent::SlaveWrRecvd => SlaveEvent::DataReceived, + I2cIsrEvent::SlaveRdReq => SlaveEvent::ReadRequest, + I2cIsrEvent::SlaveStop => SlaveEvent::Stop, + _ => SlaveEvent::DataReceived, }; // For DataReceived, read the buffer; other events have no data - if kind == I2cSEvent::SlaveWrRecvd { + if kind == I2cIsrEvent::SlaveWrRecvd { match bus.driver.read_slave_buffer(&mut bus.rx) { Ok(n) => { if n > 0 { @@ -176,8 +174,8 @@ } // Wake client on data events (DataReceived with bytes) or transaction // boundaries (Stop). ReadRequest without data is deferred (post-demo). - let should_wake = bus.notif_enabled - && (bus.rx_len > 0 || bus.rx_event_kind == SlaveEventKind::Stop); + let should_wake = + bus.notif_enabled && (bus.rx_len > 0 || bus.rx_event_kind == SlaveEvent::Stop); if should_wake { // ORs USER onto the bus channel without disturbing READABLE. if let Err(_) = syscall::object_set_peer_user_signal(bus.channel, true) { @@ -234,7 +232,7 @@ bus.notif_enabled = false; bus.rx_len = 0; bus.rx_source = 0; - bus.rx_event_kind = SlaveEventKind::DataReceived; + bus.rx_event_kind = SlaveEvent::DataReceived; encode_ok(&mut response_buf, 0) } Some((I2cOp::SlaveReceive, max_len)) => { @@ -272,7 +270,7 @@ } } } - None => encode_error(&mut response_buf, I2cError::InvalidOperation), + Some((_, _)) | None => encode_error(&mut response_buf, I2cError::InvalidOperation), }; if let Err(_) = syscall::channel_respond(channel, &response_buf[..resp_len]) { pw_log::error!("channel_respond failed");
diff --git a/services/i2c/server/src/lib.rs b/services/i2c/server/src/lib.rs index 71a9ac4..96bc46c 100644 --- a/services/i2c/server/src/lib.rs +++ b/services/i2c/server/src/lib.rs
@@ -110,7 +110,7 @@ match desc.op_kind() { Ok(I2cOpKind::Write) => write_total += desc.length(), Ok(I2cOpKind::Read) => read_total += desc.length(), - Err(_) => return encode_error(response, I2cError::InvalidOperation), + Ok(_) | Err(_) => return encode_error(response, I2cError::InvalidOperation), } } if write_total > MAX_PAYLOAD_SIZE @@ -146,7 +146,7 @@ *op = Operation::Read(head); read_rem = tail; } - Err(_) => unreachable!("op kinds validated in the sizing pass"), + Ok(_) | Err(_) => unreachable!("op kinds validated in the sizing pass"), } }
diff --git a/target/ast10x0/peripherals/i2c/hal_slave_impl.rs b/target/ast10x0/peripherals/i2c/hal_slave_impl.rs index 8ebe1d1..e8009bd 100644 --- a/target/ast10x0/peripherals/i2c/hal_slave_impl.rs +++ b/target/ast10x0/peripherals/i2c/hal_slave_impl.rs
@@ -16,24 +16,24 @@ //! read). Methods outside that path are honest best-effort and marked. use embedded_hal::i2c::SevenBitAddress; -use openprot_hal_blocking::i2c_hardware::slave::{I2cSEvent, I2cSlaveBuffer, I2cSlaveCore}; +use openprot_hal_blocking::i2c_hardware::slave::{I2cIsrEvent, I2cSlaveBuffer, I2cSlaveCore}; use openprot_hal_blocking::i2c_hardware::I2cBusRecovery; use super::controller::Ast1060I2c; use super::error::I2cError; use super::slave::{SlaveConfig, SlaveEvent}; -/// Driver `SlaveEvent` → HAL `I2cSEvent`. The notification path only acts on +/// Driver `SlaveEvent` → HAL `I2cIsrEvent`. The notification path only acts on /// the data-received case; the rest map to their nearest HAL kind. -fn to_hal_event(ev: SlaveEvent) -> I2cSEvent { +fn to_hal_event(ev: SlaveEvent) -> I2cIsrEvent { match ev { - SlaveEvent::ReadRequest => I2cSEvent::SlaveRdReq, - SlaveEvent::WriteRequest => I2cSEvent::SlaveWrReq, + SlaveEvent::ReadRequest => I2cIsrEvent::SlaveRdReq, + SlaveEvent::WriteRequest => I2cIsrEvent::SlaveWrReq, SlaveEvent::DataReceived { .. } | SlaveEvent::DataReceivedAndSent { .. } => { - I2cSEvent::SlaveWrRecvd + I2cIsrEvent::SlaveWrRecvd } - SlaveEvent::DataSent { .. } => I2cSEvent::SlaveRdProc, - SlaveEvent::Stop => I2cSEvent::SlaveStop, + SlaveEvent::DataSent { .. } => I2cIsrEvent::SlaveRdProc, + SlaveEvent::Stop => I2cIsrEvent::SlaveStop, } } @@ -118,7 +118,7 @@ /// This exposes the full hardware event (ReadRequest, Stop, etc.) alongside /// the receive count, so the server-runtime can store the actual event kind /// rather than always hardcoding DataReceived. - pub fn try_next_slave_event(&mut self) -> Result<Option<(I2cSEvent, usize)>, I2cError> { + pub fn try_next_slave_event(&mut self) -> Result<Option<(I2cIsrEvent, usize)>, I2cError> { let Some(ev) = self.handle_slave_interrupt() else { return Ok(None); };