diff --git a/esp-hal/src/twai/mod.rs b/esp-hal/src/twai/mod.rs index ab537e26dde..e690ef40d22 100644 --- a/esp-hal/src/twai/mod.rs +++ b/esp-hal/src/twai/mod.rs @@ -98,22 +98,22 @@ //! can_rx_pin, //! can_tx_pin, //! TWAI_BAUDRATE, -//! TwaiMode::SelfTest +//! TwaiMode::SelfTest, //! ); //! //! // Partially filter the incoming messages to reduce overhead of receiving //! // undesired messages -//! can_config.set_filter(const { SingleStandardFilter::new(b"xxxxxxxxxx0", -//! b"x", [b"xxxxxxxx", b"xxxxxxxx"]) }); +//! can_config.set_filter( +//! const { SingleStandardFilter::new(b"xxxxxxxxxx0", b"x", [b"xxxxxxxx", b"xxxxxxxx"]) }, +//! ); //! //! // Start the peripheral. This locks the configuration settings of the //! // peripheral and puts it into operation mode, allowing packets to be sent //! // and received. //! let mut can = can_config.start(); //! -//! # // TODO: `new_*` should return Result not Option -//! let frame = EspTwaiFrame::new_self_reception(StandardId::ZERO, -//! &[1, 2, 3]).unwrap(); // Wait for a frame to be received. +//! let frame = EspTwaiFrame::new_data(StandardId::ZERO, &[1, 2, 3], 0, true).unwrap(); +//! // Wait for a frame to be received. //! let frame = block!(can.receive())?; //! //! # loop {} @@ -409,11 +409,11 @@ impl From for Id { #[instability::unstable] impl embedded_can::Frame for EspTwaiFrame { fn new(id: impl Into, data: &[u8]) -> Option { - Self::new(id.into(), data) + Self::new_data(id.into(), data, 0, false).ok() } fn new_remote(id: impl Into, dlc: usize) -> Option { - Self::new_remote(id.into(), dlc) + Self::new_request(id.into(), dlc, false).ok() } fn is_extended(&self) -> bool { @@ -569,39 +569,23 @@ impl EspTwaiFrame { Self { bytes } } - /// Make a new [`EspTwaiFrame`] from parameters. - fn new_from_parameters( - id: impl Into, + /// Make a new [`EspTwaiFrame`] from parameters, without validation. + unsafe fn new_unchecked( remote_request: bool, - self_reception: bool, - dlc: usize, + id: impl Into, data: &[u8], - ) -> Result { - let data_len = data.len(); - - // Assert that: - // - Max data length is 8 - // - remote request frames have no data payload - if data_len > 8 || (remote_request & (data_len > 0)) { - return Err(EspTwaiError::InvalidDataLength(data_len as u8)); - } - // Assert that: - // - Max DLC is 15 - // - Data length smaller than 8 must have equal DLC - // - Data length equal to 8 hmust ave DLC >= 8 - if dlc > 15 || ((data_len < 8) & (dlc != data_len)) || ((data_len == 8) & (dlc < 8)) { - return Err(EspTwaiError::NonCompliantDlc(dlc as u8)); - } - + dlc: usize, + self_reception: bool, + ) -> Self { let mut bytes = [0u8; 13]; // Id - let (extended_format, data_start): (u8, usize) = match id.into() { + let (extended_format, data_start): (bool, usize) = match id.into() { Id::Standard(id) => { let raw = id.as_raw(); bytes[1] = (raw >> 3) as u8; bytes[2] = (raw << 5) as u8; - (0, 3) + (false, 3) } Id::Extended(id) => { let raw = id.as_raw(); @@ -609,37 +593,96 @@ impl EspTwaiFrame { bytes[2] = (raw >> 13) as u8; bytes[3] = (raw >> 5) as u8; bytes[4] = (raw << 3) as u8; - (1, 5) + (true, 5) } }; // Frame Info - let ff = extended_format << 7; + let ff = (extended_format as u8) << 7; let rtr = (remote_request as u8) << 6; let sr = (self_reception as u8) << 4; let dlc = (dlc as u8) & 0b1111; bytes[0] = ff | rtr | sr | dlc; // Data - let data_end = data_start + data_len; + let data_end = data_start + data.len(); bytes[data_start..data_end].copy_from_slice(data); - Ok(Self { bytes }) + Self { bytes } + } + + /// Create a new Data Frame. + /// + /// # Arguments + /// * `id` - Identifier. + /// * `data` - Data payload (up to 8 bytes). + /// * `dlc` - Custom Data Length Code (`0` defaults to `data.len()`). + /// * `self_reception` - If `true`, this frame will be received after a call to `transmit()`. + pub fn new_data( + id: impl Into, + data: &[u8], + dlc: usize, + self_reception: bool, + ) -> Result { + let data_len = data.len(); + // Default DLC + let dlc = match dlc { + 0 => data_len, + _ => dlc, + }; + + // Assert that max data length is 8 + if data_len > 8 { + return Err(EspTwaiError::InvalidDataLength(data_len as u8)); + } + // Assert that: + // - Max DLC is 15 + // - Data length smaller than 8 must have equal DLC + // - Data length equal to 8 must have DLC >= 8 + if dlc > 15 || ((data_len < 8) && (dlc != data_len)) || ((data_len == 8) && (dlc < 8)) { + return Err(EspTwaiError::NonCompliantDlc(dlc as u8)); + } + + // SAFETY: Safe because we validated the parameters above. + unsafe { Ok(Self::new_unchecked(false, id, data, dlc, self_reception)) } + } + + /// Create a new Request Frame. + /// + /// # Arguments + /// * `id` - Identifier. + /// * `dlc` - Data Length Code. + /// * `self_reception` - If `true`, this frame will be received after a call to `transmit()`. + pub fn new_request( + id: impl Into, + dlc: usize, + self_reception: bool, + ) -> Result { + // Assert that max DLC is 15 + if dlc > 15 { + return Err(EspTwaiError::NonCompliantDlc(dlc as u8)); + } + + // SAFETY: Safe because we validated the parameters above. + unsafe { Ok(Self::new_unchecked(true, id, &[], dlc, self_reception)) } } /// Create a new `EspTwaiFrame` with the specified ID and data payload. + #[deprecated(note = "Please use `new_data` instead")] pub fn new(id: impl Into, data: &[u8]) -> Option { - Self::new_from_parameters(id.into(), false, false, data.len(), data).ok() + Self::new_data(id, data, 0, false).ok() } /// Create a new `EspTwaiFrame` for a transmission request with the /// specified ID and data length (DLC). + #[deprecated(note = "Please use `new_request` instead")] pub fn new_remote(id: impl Into, dlc: usize) -> Option { - Self::new_from_parameters(id.into(), true, false, dlc, &[]).ok() + Self::new_request(id, dlc, false).ok() } /// Create a new `EspTwaiFrame` ready for self-reception with the specified /// ID and data payload. + #[deprecated(note = "Please use `new_data` instead")] pub fn new_self_reception(id: impl Into, data: &[u8]) -> Option { - Self::new_from_parameters(id.into(), false, true, data.len(), data).ok() + Self::new_data(id, data, 0, true).ok() } } diff --git a/hil-test/src/bin/misc_drivers.rs b/hil-test/src/bin/misc_drivers.rs index b61dff40b43..6ac8b2781ae 100644 --- a/hil-test/src/bin/misc_drivers.rs +++ b/hil-test/src/bin/misc_drivers.rs @@ -574,13 +574,25 @@ mod twai { } #[test] - fn test_send_receive(mut ctx: Context) { - let frame = EspTwaiFrame::new_self_reception(StandardId::ZERO, &[1, 2, 3]).unwrap(); + fn test_send_receive_data(mut ctx: Context) { + let frame = EspTwaiFrame::new_data(StandardId::ZERO, &[1, 2, 3], 0, true).unwrap(); block!(ctx.twai.transmit(&frame)).unwrap(); let frame = block!(ctx.twai.receive()).unwrap(); - assert_eq!(frame.data(), &[1, 2, 3]) + assert_eq!(frame.data(), &[1, 2, 3]); + assert_eq!(frame.data_length_code(), 3); + } + + #[test] + fn test_send_receive_request(mut ctx: Context) { + let frame = EspTwaiFrame::new_request(StandardId::ZERO, 4, true).unwrap(); + block!(ctx.twai.transmit(&frame)).unwrap(); + + let frame = block!(ctx.twai.receive()).unwrap(); + + assert_eq!(frame.data(), &[]); + assert_eq!(frame.data_length_code(), 4); } } @@ -644,8 +656,7 @@ mod twai { #[test] async fn test_async_transmit_and_receive(mut ctx: Context) { - let frame = - EspTwaiFrame::new_self_reception(StandardId::new(0).unwrap(), b"12345678").unwrap(); + let frame = EspTwaiFrame::new_data(StandardId::ZERO, b"12345678", 0, true).unwrap(); transmit_frames(&mut ctx, &frame, 31).await; receive_frames(&mut ctx, 31).await; } @@ -653,8 +664,7 @@ mod twai { #[test] // regression test for https://github.com/esp-rs/esp-hal/issues/4235 async fn test_buffer_overrun_on_empty_queue(mut ctx: Context) { - let frame = - EspTwaiFrame::new_self_reception(StandardId::new(0).unwrap(), b"12345678").unwrap(); + let frame = EspTwaiFrame::new_data(StandardId::ZERO, b"12345678", 0, true).unwrap(); interrupt::disable(Cpu::ProCpu, TWAI0); @@ -693,7 +703,8 @@ mod twai { let mut twai = config.into_async().start(); - let frame = EspTwaiFrame::new(StandardId::new(5).unwrap(), b"12345678").unwrap(); + let frame = + EspTwaiFrame::new_data(StandardId::new(5).unwrap(), b"12345678", 0, false).unwrap(); twai.transmit_async(&frame).await.unwrap(); }