From d090388090bde16de87e9ed1bf47e1f8dcf3f061 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 20 Sep 2024 17:19:39 +0200 Subject: [PATCH 01/16] Some more gpio cleanup --- esp-hal/src/gpio/mod.rs | 110 ++++++++++++++++++---------------------- 1 file changed, 48 insertions(+), 62 deletions(-) diff --git a/esp-hal/src/gpio/mod.rs b/esp-hal/src/gpio/mod.rs index df03c5d67da..02540005b23 100644 --- a/esp-hal/src/gpio/mod.rs +++ b/esp-hal/src/gpio/mod.rs @@ -761,16 +761,11 @@ fn disable_usb_pads(gpionum: u8) { unsafe { &*crate::peripherals::USB_DEVICE::PTR } .conf0() .modify(|_, w| { - w.usb_pad_enable() - .clear_bit() - .dm_pullup() - .clear_bit() - .dm_pulldown() - .clear_bit() - .dp_pullup() - .clear_bit() - .dp_pulldown() - .clear_bit() + w.usb_pad_enable().clear_bit(); + w.dm_pullup().clear_bit(); + w.dm_pulldown().clear_bit(); + w.dp_pullup().clear_bit(); + w.dp_pulldown().clear_bit() }); } } @@ -799,7 +794,10 @@ where #[cfg(esp32)] crate::soc::gpio::errata36(GPIONUM, Some(pull_up), Some(pull_down)); - get_io_mux_reg(GPIONUM).modify(|_, w| w.fun_wpd().bit(pull_down).fun_wpu().bit(pull_up)); + get_io_mux_reg(GPIONUM).modify(|_, w| { + w.fun_wpd().bit(pull_down); + w.fun_wpu().bit(pull_up) + }); } } @@ -814,12 +812,9 @@ where disable_usb_pads(GPIONUM); get_io_mux_reg(GPIONUM).modify(|_, w| unsafe { - w.mcu_sel() - .bits(GPIO_FUNCTION as u8) - .fun_ie() - .set_bit() - .slp_sel() - .clear_bit() + w.mcu_sel().bits(GPIO_FUNCTION as u8); + w.fun_ie().set_bit(); + w.slp_sel().clear_bit() }); } @@ -936,14 +931,10 @@ where disable_usb_pads(GPIONUM); get_io_mux_reg(GPIONUM).modify(|_, w| unsafe { - w.mcu_sel() - .bits(alternate as u8) - .fun_ie() - .bit(open_drain) - .fun_drv() - .bits(DriveStrength::I20mA as u8) - .slp_sel() - .clear_bit() + w.mcu_sel().bits(alternate as u8); + w.fun_ie().bit(open_drain); + w.fun_drv().bits(DriveStrength::I20mA as u8); + w.slp_sel().clear_bit() }); } @@ -1231,10 +1222,10 @@ macro_rules! rtc_pins { // disable input paste::paste!{ - rtcio.$pin_reg.modify(|_,w| unsafe {w - .[<$prefix fun_ie>]().bit(input_enable) - .[<$prefix mux_sel>]().bit(mux) - .[<$prefix fun_sel>]().bits(func as u8) + rtcio.$pin_reg.modify(|_,w| unsafe { + w.[<$prefix fun_ie>]().bit(input_enable); + w.[<$prefix mux_sel>]().bit(mux); + w.[<$prefix fun_sel>]().bits(func as u8) }); } } @@ -1485,10 +1476,10 @@ macro_rules! analog { use $crate::peripherals::{GPIO}; get_io_mux_reg($pin_num).modify(|_,w| unsafe { - w.mcu_sel().bits(1) - .fun_ie().clear_bit() - .fun_wpu().clear_bit() - .fun_wpd().clear_bit() + w.mcu_sel().bits(1); + w.fun_ie().clear_bit(); + w.fun_wpu().clear_bit(); + w.fun_wpd().clear_bit() }); unsafe{ &*GPIO::PTR }.enable_w1tc().write(|w| unsafe { w.bits(1 << $pin_num) }); @@ -1505,29 +1496,27 @@ macro_rules! touch { (@pin_specific $touch_num:expr, true) => { paste::paste! { unsafe { &*RTC_IO::ptr() }.[< touch_pad $touch_num >]().write(|w| unsafe { - w - .xpd().set_bit() + w.xpd().set_bit(); // clear input_enable - .fun_ie().clear_bit() + w.fun_ie().clear_bit(); // Connect pin to analog / RTC module instead of standard GPIO - .mux_sel().set_bit() + w.mux_sel().set_bit(); // Disable pull-up and pull-down resistors on the pin - .rue().clear_bit() - .rde().clear_bit() - .tie_opt().clear_bit() + w.rue().clear_bit(); + w.rde().clear_bit(); + w.tie_opt().clear_bit(); // Select function "RTC function 1" (GPIO) for analog use - .fun_sel().bits(0b00) + w.fun_sel().bits(0b00) }); } }; (@pin_specific $touch_num:expr, false) => { paste::paste! { - unsafe { &*RTC_IO::ptr() }.[< touch_pad $touch_num >]().write(|w| - w - .xpd().set_bit() - .tie_opt().clear_bit() - ); + unsafe { &*RTC_IO::ptr() }.[< touch_pad $touch_num >]().write(|w| { + w.xpd().set_bit(); + w.tie_opt().clear_bit() + }); } }; @@ -2444,12 +2433,9 @@ fn is_listening(pin_num: u8) -> bool { fn set_int_enable(gpio_num: u8, int_ena: u8, int_type: u8, wake_up_from_light_sleep: bool) { let gpio = unsafe { &*crate::peripherals::GPIO::PTR }; gpio.pin(gpio_num as usize).modify(|_, w| unsafe { - w.int_ena() - .bits(int_ena) - .int_type() - .bits(int_type) - .wakeup_enable() - .bit(wake_up_from_light_sleep) + w.int_ena().bits(int_ena); + w.int_type().bits(int_type); + w.wakeup_enable().bit(wake_up_from_light_sleep) }); } @@ -2496,36 +2482,36 @@ mod asynch { where P: InputPin, { + async fn wait_for(&mut self, event: Event) { + self.listen(event); + PinFuture::new(self.pin.number()).await + } + /// Wait until the pin is high. If it is already high, return /// immediately. pub async fn wait_for_high(&mut self) { - self.listen(Event::HighLevel); - PinFuture::new(self.pin.number()).await + self.wait_for(Event::HighLevel).await } /// Wait until the pin is low. If it is already low, return immediately. pub async fn wait_for_low(&mut self) { - self.listen(Event::LowLevel); - PinFuture::new(self.pin.number()).await + self.wait_for(Event::LowLevel).await } /// Wait for the pin to undergo a transition from low to high. pub async fn wait_for_rising_edge(&mut self) { - self.listen(Event::RisingEdge); - PinFuture::new(self.pin.number()).await + self.wait_for(Event::RisingEdge).await } /// Wait for the pin to undergo a transition from high to low. pub async fn wait_for_falling_edge(&mut self) { - self.listen(Event::FallingEdge); - PinFuture::new(self.pin.number()).await + self.wait_for(Event::FallingEdge).await } /// Wait for the pin to undergo any transition, i.e low to high OR high /// to low. pub async fn wait_for_any_edge(&mut self) { - self.listen(Event::AnyEdge); - PinFuture::new(self.pin.number()).await + self.wait_for(Event::AnyEdge).await } } From 2825d715f8648efd3a6b37fd5e83205c2894db61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 20 Sep 2024 17:10:04 +0200 Subject: [PATCH 02/16] Allow TWAI loopback using the same pin, enable ESP32 --- esp-hal/src/twai/mod.rs | 9 ++++++--- hil-test/tests/twai.rs | 16 +++++++++------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/esp-hal/src/twai/mod.rs b/esp-hal/src/twai/mod.rs index f0e06ecda1e..a6a1b1eba97 100644 --- a/esp-hal/src/twai/mod.rs +++ b/esp-hal/src/twai/mod.rs @@ -770,9 +770,6 @@ where .mode() .write(|w| w.reset_mode().set_bit()); - rx_pin.init_input(Pull::None, crate::private::Internal); - rx_pin.connect_input_to_peripheral(T::INPUT_SIGNAL, crate::private::Internal); - if no_transceiver { tx_pin.set_to_open_drain_output(crate::private::Internal); } else { @@ -780,6 +777,12 @@ where } tx_pin.connect_peripheral_to_output(T::OUTPUT_SIGNAL, crate::private::Internal); + // Setting up RX pin later allows us to use a single pin in tests. + // `set_to_push_pull_output` disables input, here we re-enable it if rx_pin + // uses the same GPIO. + rx_pin.init_input(Pull::None, crate::private::Internal); + rx_pin.connect_input_to_peripheral(T::INPUT_SIGNAL, crate::private::Internal); + // Set the operating mode based on provided option match mode { TwaiMode::Normal => { diff --git a/hil-test/tests/twai.rs b/hil-test/tests/twai.rs index 508c74d1143..a1d2c852691 100644 --- a/hil-test/tests/twai.rs +++ b/hil-test/tests/twai.rs @@ -1,6 +1,6 @@ //! TWAI test -//% CHIPS: esp32c3 esp32c6 esp32h2 esp32s2 esp32s3 +//% CHIPS: esp32 esp32c3 esp32c6 esp32h2 esp32s2 esp32s3 #![no_std] #![no_main] @@ -33,19 +33,21 @@ mod tests { let io = Io::new(peripherals.GPIO, peripherals.IO_MUX); - let (tx_pin, rx_pin) = hil_test::common_test_pins!(io); + let (loopback_pin, _) = hil_test::common_test_pins!(io); let mut config = twai::TwaiConfiguration::new( peripherals.TWAI0, - rx_pin, - tx_pin, + loopback_pin.peripheral_input(), + loopback_pin, twai::BaudRate::B1000K, TwaiMode::SelfTest, ); - const FILTER: SingleStandardFilter = - SingleStandardFilter::new(b"00000000000", b"x", [b"xxxxxxxx", b"xxxxxxxx"]); - config.set_filter(FILTER); + config.set_filter(SingleStandardFilter::new( + b"00000000000", + b"x", + [b"xxxxxxxx", b"xxxxxxxx"], + )); let twai = config.start(); From 9a8ea9ab9707d0db9c6788a442824e568b96f72b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 20 Sep 2024 17:41:02 +0200 Subject: [PATCH 03/16] Impl Format for ErrorKind --- esp-hal/src/twai/mod.rs | 52 ++++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/esp-hal/src/twai/mod.rs b/esp-hal/src/twai/mod.rs index a6a1b1eba97..8d2eb9b21c3 100644 --- a/esp-hal/src/twai/mod.rs +++ b/esp-hal/src/twai/mod.rs @@ -175,27 +175,35 @@ pub enum ErrorKind { Other, } -impl core::fmt::Display for ErrorKind { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - match self { - Self::Overrun => write!(f, "The peripheral receive buffer was overrun"), - Self::Bit => write!( - f, - "Bit value that is monitored differs from the bit value sent" - ), - Self::Stuff => write!(f, "Sixth consecutive equal bits detected"), - Self::Crc => write!(f, "Calculated CRC sequence does not equal the received one"), - Self::Form => write!( - f, - "A fixed-form bit field contains one or more illegal bits" - ), - Self::Acknowledge => write!(f, "Transmitted frame was not acknowledged"), - Self::Other => write!( - f, - "A different error occurred. The original error may contain more information" - ), +macro_rules! impl_display { + ($($kind:ident => $msg:expr),* $(,)?) => { + impl core::fmt::Display for ErrorKind { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + $(Self::$kind => write!(f, $msg)),* + } + } } - } + + #[cfg(feature = "defmt")] + impl defmt::Format for ErrorKind { + fn format(&self, f: defmt::Formatter<'_>) { + match self { + $(Self::$kind => defmt::write!(f, $msg)),* + } + } + } + }; +} + +impl_display! { + Overrun => "The peripheral receive buffer was overrun", + Bit => "Bit value that is monitored differs from the bit value sent", + Stuff => "Sixth consecutive equal bits detected", + Crc => "Calculated CRC sequence does not equal the received one", + Form => "A fixed-form bit field contains one or more illegal bits", + Acknowledge => "Transmitted frame was not acknowledged", + Other => "A different error occurred. The original error may contain more information", } impl From for embedded_hal_02::can::ErrorKind { @@ -1589,7 +1597,7 @@ impl Instance for crate::peripherals::TWAI0 { #[cfg(any(esp32h2, esp32c6))] impl OperationInstance for crate::peripherals::TWAI0 {} -#[cfg(esp32c6)] +#[cfg(twai1)] impl Instance for crate::peripherals::TWAI1 { const SYSTEM_PERIPHERAL: system::Peripheral = system::Peripheral::Twai1; const NUMBER: usize = 1; @@ -1648,7 +1656,7 @@ mod asynch { use super::*; use crate::peripherals::TWAI0; - #[cfg(esp32c6)] + #[cfg(twai1)] use crate::peripherals::TWAI1; pub struct TwaiAsyncState { From 3a67fe53e271b46bbf6000c05cf3596bf03af70f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 20 Sep 2024 17:51:03 +0200 Subject: [PATCH 04/16] Generic frame constructors --- esp-hal/CHANGELOG.md | 1 + esp-hal/src/twai/mod.rs | 31 ++++++++++++------------------- hil-test/tests/twai.rs | 2 +- 3 files changed, 14 insertions(+), 20 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 2ed4b75424f..d663bd4c40e 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -51,6 +51,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - MSRV bump to 1.79 (#2156) - Allow handling interrupts while trying to lock critical section on multi-core chips. (#2197) - Removed the PS-RAM related features, replaced by `quad-psram`/`octal-psram`, `init_psram` takes a configuration parameter, it's now possible to auto-detect PS-RAM size (#2178) +- `EspTwaiFrame` constructors now accept any type that converts into `esp_hal::twai::Id` (#?) ### Fixed diff --git a/esp-hal/src/twai/mod.rs b/esp-hal/src/twai/mod.rs index 8d2eb9b21c3..1a5f34b8c44 100644 --- a/esp-hal/src/twai/mod.rs +++ b/esp-hal/src/twai/mod.rs @@ -120,9 +120,8 @@ //! // and received. //! let mut can = can_config.start(); //! -//! let frame = EspTwaiFrame::new_self_reception(StandardId::ZERO.into(), -//! &[1, 2, 3]).unwrap(); -//! // Wait for a frame to be received. +//! let frame = EspTwaiFrame::new_self_reception(StandardId::ZERO, +//! &[1, 2, 3]).unwrap(); // Wait for a frame to be received. //! let frame = block!(can.receive()).unwrap(); //! //! # loop {} @@ -464,18 +463,17 @@ pub struct EspTwaiFrame { impl EspTwaiFrame { /// Creates a new `EspTwaiFrame` with the specified ID and data payload. - pub fn new(id: Id, data: &[u8]) -> Option { + pub fn new(id: impl Into, data: &[u8]) -> Option { // TWAI frames cannot contain more than 8 bytes of data. if data.len() > 8 { return None; } let mut d: [u8; 8] = [0; 8]; - let (left, _unused) = d.split_at_mut(data.len()); - left.clone_from_slice(data); + (&mut d[..data.len()]).copy_from_slice(data); Some(EspTwaiFrame { - id, + id: id.into(), data: d, dlc: data.len(), is_remote: false, @@ -485,14 +483,14 @@ impl EspTwaiFrame { /// Creates a new `EspTwaiFrame` for a transmission request with the /// specified ID and data length (DLC). - pub fn new_remote(id: Id, dlc: usize) -> Option { + pub fn new_remote(id: impl Into, dlc: usize) -> Option { // TWAI frames cannot have more than 8 bytes. if dlc > 8 { return None; } Some(EspTwaiFrame { - id, + id: id.into(), data: [0; 8], dlc, is_remote: true, @@ -502,17 +500,16 @@ impl EspTwaiFrame { /// Creates a new `EspTwaiFrame` ready for self-reception with the specified /// ID and data payload. - pub fn new_self_reception(id: Id, data: &[u8]) -> Option { + pub fn new_self_reception(id: impl Into, data: &[u8]) -> Option { if data.len() > 8 { return None; } let mut d: [u8; 8] = [0; 8]; - let (left, _unused) = d.split_at_mut(data.len()); - left.clone_from_slice(data); + (&mut d[..data.len()]).copy_from_slice(data); Some(EspTwaiFrame { - id, + id: id.into(), data: d, dlc: data.len(), is_remote: false, @@ -547,12 +544,10 @@ impl EspTwaiFrame { impl embedded_hal_02::can::Frame for EspTwaiFrame { fn new(id: impl Into, data: &[u8]) -> Option { - let id: embedded_hal_02::can::Id = id.into(); Self::new(id.into(), data) } fn new_remote(id: impl Into, dlc: usize) -> Option { - let id: embedded_hal_02::can::Id = id.into(); Self::new_remote(id.into(), dlc) } @@ -587,12 +582,10 @@ impl embedded_hal_02::can::Frame for EspTwaiFrame { impl embedded_can::Frame for EspTwaiFrame { fn new(id: impl Into, data: &[u8]) -> Option { - let id: embedded_can::Id = id.into(); Self::new(id.into(), data) } fn new_remote(id: impl Into, dlc: usize) -> Option { - let id: embedded_can::Id = id.into(); Self::new_remote(id.into(), dlc) } @@ -1467,7 +1460,7 @@ pub trait OperationInstance: Instance { EspTwaiFrame::new_from_data_registers(id, register_block.data_3().as_ptr(), dlc) } } else { - EspTwaiFrame::new_remote(id.into(), dlc).unwrap() + EspTwaiFrame::new_remote(id, dlc).unwrap() } } else { // Frame uses extended 29 bit id. @@ -1491,7 +1484,7 @@ pub trait OperationInstance: Instance { EspTwaiFrame::new_from_data_registers(id, register_block.data_5().as_ptr(), dlc) } } else { - EspTwaiFrame::new_remote(id.into(), dlc).unwrap() + EspTwaiFrame::new_remote(id, dlc).unwrap() } }; diff --git a/hil-test/tests/twai.rs b/hil-test/tests/twai.rs index a1d2c852691..6bb6f9c1cd2 100644 --- a/hil-test/tests/twai.rs +++ b/hil-test/tests/twai.rs @@ -57,7 +57,7 @@ mod tests { #[test] #[timeout(3)] fn test_send_receive(mut ctx: Context) { - let frame = EspTwaiFrame::new_self_reception(StandardId::ZERO.into(), &[1, 2, 3]).unwrap(); + let frame = EspTwaiFrame::new_self_reception(StandardId::ZERO, &[1, 2, 3]).unwrap(); block!(ctx.twai.transmit(&frame)).unwrap(); let frame = block!(ctx.twai.receive()).unwrap(); From d530a121eef5d0158e4853cc8541a7e49d6ad658 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 20 Sep 2024 18:07:33 +0200 Subject: [PATCH 05/16] More TWAI cleanups --- esp-hal/src/twai/mod.rs | 250 +++++++++++++--------------------------- 1 file changed, 79 insertions(+), 171 deletions(-) diff --git a/esp-hal/src/twai/mod.rs b/esp-hal/src/twai/mod.rs index 1a5f34b8c44..723559bb4a7 100644 --- a/esp-hal/src/twai/mod.rs +++ b/esp-hal/src/twai/mod.rs @@ -246,6 +246,7 @@ impl embedded_can::Error for ErrorKind { } /// Specifies in which mode the TWAI controller will operate. +#[derive(Debug, Copy, Clone, Eq, PartialEq)] pub enum TwaiMode { /// Normal operating mode Normal, @@ -552,10 +553,7 @@ impl embedded_hal_02::can::Frame for EspTwaiFrame { } fn is_extended(&self) -> bool { - match self.id { - Id::Standard(_) => false, - Id::Extended(_) => true, - } + matches!(self.id, Id::Extended(_)) } fn is_remote_frame(&self) -> bool { @@ -590,10 +588,7 @@ impl embedded_can::Frame for EspTwaiFrame { } fn is_extended(&self) -> bool { - match self.id { - Id::Standard(_) => false, - Id::Extended(_) => true, - } + matches!(self.id, Id::Extended(_)) } fn is_remote_frame(&self) -> bool { @@ -785,23 +780,11 @@ where rx_pin.connect_input_to_peripheral(T::INPUT_SIGNAL, crate::private::Internal); // Set the operating mode based on provided option - match mode { - TwaiMode::Normal => { - // Do nothing special, the default state is Normal mode. - } - TwaiMode::SelfTest => { - // Set the self-test mode (no acknowledgement required) - T::register_block() - .mode() - .modify(|_, w| w.self_test_mode().set_bit()); - } - TwaiMode::ListenOnly => { - // Set listen-only mode - T::register_block() - .mode() - .modify(|_, w| w.listen_only_mode().set_bit()); - } - } + T::register_block().mode().modify(|_, w| { + // self-test mode turns off acknowledgement requirement + w.self_test_mode().bit(mode == TwaiMode::SelfTest); + w.listen_only_mode().bit(mode == TwaiMode::ListenOnly) + }); // Set TEC to 0 T::register_block() @@ -1024,7 +1007,7 @@ pub struct Twai<'d, T, DM: crate::Mode> { impl<'d, T, DM> Twai<'d, T, DM> where - T: OperationInstance, + T: Instance, DM: crate::Mode, { /// Stop the peripheral, putting it into reset mode and enabling @@ -1112,7 +1095,7 @@ pub struct TwaiTx<'d, T, DM: crate::Mode> { impl<'d, T, DM> TwaiTx<'d, T, DM> where - T: OperationInstance, + T: Instance, DM: crate::Mode, { /// Transmit a frame. @@ -1154,7 +1137,7 @@ pub struct TwaiRx<'d, T, DM: crate::Mode> { impl<'d, T, DM> TwaiRx<'d, T, DM> where - T: OperationInstance, + T: Instance, DM: crate::Mode, { /// Receive a frame @@ -1251,7 +1234,7 @@ unsafe fn copy_to_data_register(dest: *mut u32, src: &[u8]) { impl<'d, T, DM> embedded_hal_02::can::Can for Twai<'d, T, DM> where - T: OperationInstance, + T: Instance, DM: crate::Mode, { type Frame = EspTwaiFrame; @@ -1275,7 +1258,7 @@ where impl<'d, T, DM> embedded_can::nb::Can for Twai<'d, T, DM> where - T: OperationInstance, + T: Instance, DM: crate::Mode, { type Frame = EspTwaiFrame; @@ -1325,11 +1308,7 @@ pub trait Instance: crate::private::Sealed { /// Enables interrupts for the TWAI peripheral. fn enable_interrupts(); -} -/// An extension of the `Instance` trait that provides additional operations -/// for managing and interacting with the TWAI peripheral. -pub trait OperationInstance: Instance { /// Returns a reference to the asynchronous state for this TWAI instance. fn async_state() -> &'static asynch::TwaiAsyncState { &asynch::TWAI_STATE[Self::NUMBER] @@ -1418,12 +1397,15 @@ pub trait OperationInstance: Instance { // Trigger the appropriate transmission request based on self_reception flag if frame.self_reception { - #[cfg(any(esp32, esp32c3, esp32s2, esp32s3))] - register_block.cmd().write(|w| w.self_rx_req().set_bit()); - #[cfg(not(any(esp32, esp32c3, esp32s2, esp32s3)))] - register_block - .cmd() - .write(|w| w.self_rx_request().set_bit()); + cfg_if::cfg_if! { + if #[cfg(any(esp32, esp32c3, esp32s2, esp32s3))] { + register_block.cmd().write(|w| w.self_rx_req().set_bit()); + } else { + register_block + .cmd() + .write(|w| w.self_rx_request().set_bit()); + } + } } else { // Set the transmit request command, this will lock the transmit buffer until // the transmission is complete or aborted. @@ -1496,60 +1478,19 @@ pub trait OperationInstance: Instance { } } -#[cfg(any(esp32, esp32c3, esp32s2, esp32s3))] impl Instance for crate::peripherals::TWAI0 { const SYSTEM_PERIPHERAL: system::Peripheral = system::Peripheral::Twai0; const NUMBER: usize = 0; - const INPUT_SIGNAL: InputSignal = InputSignal::TWAI_RX; - const OUTPUT_SIGNAL: OutputSignal = OutputSignal::TWAI_TX; - - const INTERRUPT: crate::peripherals::Interrupt = crate::peripherals::Interrupt::TWAI0; - - fn async_handler() -> InterruptHandler { - asynch::twai0 - } - - #[inline(always)] - fn register_block() -> &'static RegisterBlock { - unsafe { &*crate::peripherals::TWAI0::PTR } - } - - fn reset_peripheral() { - PeripheralClockControl::reset(crate::system::Peripheral::Twai0); - } - - fn enable_peripheral() { - PeripheralClockControl::enable(crate::system::Peripheral::Twai0); - } - - fn enable_interrupts() { - let register_block = Self::register_block(); - register_block.int_ena().modify(|_, w| { - w.rx_int_ena() - .set_bit() - .tx_int_ena() - .set_bit() - .bus_err_int_ena() - .set_bit() - .arb_lost_int_ena() - .set_bit() - .err_passive_int_ena() - .set_bit() - }); + cfg_if::cfg_if! { + if #[cfg(any(esp32, esp32c3, esp32s2, esp32s3))] { + const INPUT_SIGNAL: InputSignal = InputSignal::TWAI_RX; + const OUTPUT_SIGNAL: OutputSignal = OutputSignal::TWAI_TX; + } else { + const INPUT_SIGNAL: InputSignal = InputSignal::TWAI0_RX; + const OUTPUT_SIGNAL: OutputSignal = OutputSignal::TWAI0_TX; + } } -} - -#[cfg(any(esp32, esp32c3, esp32s2, esp32s3))] -impl OperationInstance for crate::peripherals::TWAI0 {} - -#[cfg(any(esp32h2, esp32c6))] -impl Instance for crate::peripherals::TWAI0 { - const SYSTEM_PERIPHERAL: system::Peripheral = system::Peripheral::Twai0; - const NUMBER: usize = 0; - - const INPUT_SIGNAL: InputSignal = InputSignal::TWAI0_RX; - const OUTPUT_SIGNAL: OutputSignal = OutputSignal::TWAI0_TX; const INTERRUPT: crate::peripherals::Interrupt = crate::peripherals::Interrupt::TWAI0; @@ -1572,24 +1513,29 @@ impl Instance for crate::peripherals::TWAI0 { fn enable_interrupts() { let register_block = Self::register_block(); - register_block.interrupt_enable().modify(|_, w| { - w.ext_receive_int_ena() - .set_bit() - .ext_transmit_int_ena() - .set_bit() - .bus_err_int_ena() - .set_bit() - .arbitration_lost_int_ena() - .set_bit() - .err_passive_int_ena() - .set_bit() - }); + + cfg_if::cfg_if! { + if #[cfg(any(esp32, esp32c3, esp32s2, esp32s3))] { + register_block.int_ena().modify(|_, w| { + w.rx_int_ena().set_bit(); + w.tx_int_ena().set_bit(); + w.bus_err_int_ena().set_bit(); + w.arb_lost_int_ena().set_bit(); + w.err_passive_int_ena().set_bit() + }); + } else { + register_block.interrupt_enable().modify(|_, w| { + w.ext_receive_int_ena().set_bit(); + w.ext_transmit_int_ena().set_bit(); + w.bus_err_int_ena().set_bit(); + w.arbitration_lost_int_ena().set_bit(); + w.err_passive_int_ena().set_bit() + }); + } + } } } -#[cfg(any(esp32h2, esp32c6))] -impl OperationInstance for crate::peripherals::TWAI0 {} - #[cfg(twai1)] impl Instance for crate::peripherals::TWAI1 { const SYSTEM_PERIPHERAL: system::Peripheral = system::Peripheral::Twai1; @@ -1620,23 +1566,15 @@ impl Instance for crate::peripherals::TWAI1 { fn enable_interrupts() { let register_block = Self::register_block(); register_block.interrupt_enable().modify(|_, w| { - w.ext_receive_int_ena() - .set_bit() - .ext_transmit_int_ena() - .set_bit() - .bus_err_int_ena() - .set_bit() - .arbitration_lost_int_ena() - .set_bit() - .err_passive_int_ena() - .set_bit() + w.ext_receive_int_ena().set_bit(); + w.ext_transmit_int_ena().set_bit(); + w.bus_err_int_ena().set_bit(); + w.arbitration_lost_int_ena().set_bit(); + w.err_passive_int_ena().set_bit() }); } } -#[cfg(esp32c6)] -impl OperationInstance for crate::peripherals::TWAI1 {} - mod asynch { use core::{future::poll_fn, task::Poll}; @@ -1680,7 +1618,7 @@ mod asynch { impl Twai<'_, T, crate::Async> where - T: OperationInstance, + T: Instance, { /// Transmits an `EspTwaiFrame` asynchronously over the TWAI bus. pub async fn transmit_async(&mut self, frame: &EspTwaiFrame) -> Result<(), EspTwaiError> { @@ -1694,7 +1632,7 @@ mod asynch { impl<'d, T> TwaiTx<'d, T, crate::Async> where - T: OperationInstance, + T: Instance, { /// Transmits an `EspTwaiFrame` asynchronously over the TWAI bus. pub async fn transmit_async(&mut self, frame: &EspTwaiFrame) -> Result<(), EspTwaiError> { @@ -1724,7 +1662,7 @@ mod asynch { impl<'d, T> TwaiRx<'d, T, crate::Async> where - T: OperationInstance, + T: Instance, { /// Receives an `EspTwaiFrame` asynchronously over the TWAI bus. pub async fn receive_async(&mut self) -> Result { @@ -1733,7 +1671,7 @@ mod asynch { T::async_state().err_waker.register(cx.waker()); if let Poll::Ready(result) = T::async_state().rx_queue.poll_receive(cx) { - return Poll::Ready(result); + Poll::Ready(result) } else { let register_block = T::register_block(); let status = register_block.status().read(); @@ -1747,74 +1685,45 @@ mod asynch { if status.miss_st().bit_is_set() { return Poll::Ready(Err(EspTwaiError::EmbeddedHAL(ErrorKind::Overrun))); } - } - Poll::Pending + Poll::Pending + } }) .await } } - #[cfg(any(esp32c3, esp32, esp32s2, esp32s3))] #[handler] pub(super) fn twai0() { let register_block = TWAI0::register_block(); - let intr_enable = register_block.int_ena().read(); - let intr_status = register_block.int_raw().read(); - - let async_state = TWAI0::async_state(); + cfg_if::cfg_if! { + if #[cfg(any(esp32, esp32c3, esp32s2, esp32s3))] { + let intr_enable = register_block.int_ena().read(); + let intr_status = register_block.int_raw().read(); - if intr_status.tx_int_st().bit_is_set() { - async_state.tx_waker.wake(); - } + let int_ena_reg = register_block.int_ena(); - if intr_status.rx_int_st().bit_is_set() { - let status = register_block.status().read(); - - let rx_queue = &async_state.rx_queue; + let tx_int_status = intr_status.tx_int_st(); + let rx_int_status = intr_status.rx_int_st(); + } else { + let intr_enable = register_block.interrupt_enable().read(); + let intr_status = register_block.interrupt().read(); - if status.bus_off_st().bit_is_set() { - let _ = rx_queue.try_send(Err(EspTwaiError::BusOff)); - } + let int_ena_reg = register_block.interrupt_enable(); - if status.miss_st().bit_is_set() { - let _ = rx_queue.try_send(Err(EspTwaiError::EmbeddedHAL(ErrorKind::Overrun))); + let tx_int_status = intr_status.transmit_int_st(); + let rx_int_status = intr_status.receive_int_st(); } - - let frame = TWAI0::read_frame(); - - let _ = rx_queue.try_send(Ok(frame)); - - register_block.cmd().write(|w| w.release_buf().set_bit()); } - if intr_status.bits() & 0b11111100 > 0 { - async_state.err_waker.wake(); - } - - unsafe { - register_block - .int_ena() - .modify(|_, w| w.bits(intr_enable.bits() & (!intr_status.bits() | 1))); - } - } - - #[cfg(any(esp32h2, esp32c6))] - #[handler] - pub(super) fn twai0() { - let register_block = TWAI0::register_block(); - - let intr_enable = register_block.interrupt_enable().read(); - let intr_status = register_block.interrupt().read(); - let async_state = TWAI0::async_state(); - if intr_status.transmit_int_st().bit_is_set() { + if tx_int_status.bit_is_set() { async_state.tx_waker.wake(); } - if intr_status.receive_int_st().bit_is_set() { + if rx_int_status.bit_is_set() { let status = register_block.status().read(); let rx_queue = &async_state.rx_queue; @@ -1838,14 +1747,13 @@ mod asynch { async_state.err_waker.wake(); } + // Clear interrupt request bits unsafe { - register_block - .interrupt_enable() - .modify(|_, w| w.bits(intr_enable.bits() & (!intr_status.bits() | 1))); + int_ena_reg.modify(|_, w| w.bits(intr_enable.bits() & (!intr_status.bits() | 1))); } } - #[cfg(esp32c6)] + #[cfg(twai1)] #[handler] pub(super) fn twai1() { let register_block = TWAI1::register_block(); From 6618af96921e5977f8eaee973b285759839e0fef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 20 Sep 2024 19:04:02 +0200 Subject: [PATCH 06/16] Fix signals --- esp-hal/src/soc/esp32/gpio.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/esp-hal/src/soc/esp32/gpio.rs b/esp-hal/src/soc/esp32/gpio.rs index 47dea4116dc..44395379cef 100644 --- a/esp-hal/src/soc/esp32/gpio.rs +++ b/esp-hal/src/soc/esp32/gpio.rs @@ -467,7 +467,8 @@ pub enum OutputSignal { PWM2_4H = 120, PWM2_4L = 121, TWAI_TX = 123, - CAN_BUS_OFF_ON = 124, + TWAI_BUS_OFF_ON = 124, + TWAI_CLKOUT = 125, SPID4 = 128, SPID5 = 129, SPID6 = 130, From 893ddec691aa8388259012af0d5c8dcaff8d1cc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 20 Sep 2024 19:29:17 +0200 Subject: [PATCH 07/16] Set self-reception bit --- esp-hal/src/twai/mod.rs | 43 ++++++++++++++++------------------------- 1 file changed, 17 insertions(+), 26 deletions(-) diff --git a/esp-hal/src/twai/mod.rs b/esp-hal/src/twai/mod.rs index 723559bb4a7..a9bb4816174 100644 --- a/esp-hal/src/twai/mod.rs +++ b/esp-hal/src/twai/mod.rs @@ -1327,10 +1327,11 @@ pub trait Instance: crate::private::Sealed { fn write_frame(frame: &EspTwaiFrame) { // Assemble the frame information into the data_0 byte. let frame_format: u8 = matches!(frame.id, Id::Extended(_)) as u8; + let self_reception: u8 = frame.self_reception as u8; let rtr_bit: u8 = frame.is_remote as u8; let dlc_bits: u8 = frame.dlc as u8 & 0b1111; - let data_0: u8 = frame_format << 7 | rtr_bit << 6 | dlc_bits; + let data_0: u8 = frame_format << 7 | rtr_bit << 6 | self_reception << 4 | dlc_bits; let register_block = Self::register_block(); @@ -1338,8 +1339,9 @@ pub trait Instance: crate::private::Sealed { .data_0() .write(|w| unsafe { w.tx_byte_0().bits(data_0) }); - // Assemble the identifier information of the packet. - match frame.id { + // Assemble the identifier information of the packet and return where the data + // buffer starts. + let data_ptr = match frame.id { Id::Standard(id) => { let id = id.as_raw(); @@ -1350,6 +1352,8 @@ pub trait Instance: crate::private::Sealed { register_block .data_2() .write(|w| unsafe { w.tx_byte_2().bits((id << 5) as u8) }); + + register_block.data_3().as_ptr() } Id::Extended(id) => { let id = id.as_raw(); @@ -1366,33 +1370,20 @@ pub trait Instance: crate::private::Sealed { register_block .data_4() .write(|w| unsafe { w.tx_byte_4().bits((id << 3) as u8) }); + + register_block.data_5().as_ptr() } - } + }; // Store the data portion of the packet into the transmit buffer. - if !frame.is_remote { - match frame.id { - Id::Standard(_) => unsafe { - copy_to_data_register( - register_block.data_3().as_ptr(), - match frame.is_remote { - true => &[], - false => &frame.data[0..frame.dlc], - }, - ) - }, - Id::Extended(_) => unsafe { - copy_to_data_register( - register_block.data_5().as_ptr(), - match frame.is_remote { - true => &[], - false => &frame.data[0..frame.dlc], - }, - ) + unsafe { + copy_to_data_register( + data_ptr, + match frame.is_remote { + true => &[], // RTR frame, so no data is included. + false => &frame.data[0..frame.dlc], }, - } - } else { - // Is RTR frame, so no data is included. + ) } // Trigger the appropriate transmission request based on self_reception flag From 539c13fb97212936e94607bb6bff244899f55f70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 20 Sep 2024 19:40:35 +0200 Subject: [PATCH 08/16] Teach users to use const blocks --- esp-hal/src/twai/mod.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/esp-hal/src/twai/mod.rs b/esp-hal/src/twai/mod.rs index a9bb4816174..8d719c661c1 100644 --- a/esp-hal/src/twai/mod.rs +++ b/esp-hal/src/twai/mod.rs @@ -56,9 +56,8 @@ //! //! // Partially filter the incoming messages to reduce overhead of receiving //! // undesired messages -//! const FILTER: twai::filter::SingleStandardFilter = -//! SingleStandardFilter::new(b"xxxxxxxxxx0", b"x", [b"xxxxxxxx", -//! b"xxxxxxxx"]); can_config.set_filter(FILTER); +//! 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 @@ -110,10 +109,8 @@ //! //! // Partially filter the incoming messages to reduce overhead of receiving //! // undesired messages -//! const FILTER: twai::filter::SingleStandardFilter = -//! SingleStandardFilter::new(b"xxxxxxxxxx0", b"x", -//! [b"xxxxxxxx", b"xxxxxxxx"]); -//! can_config.set_filter(FILTER); +//! 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 @@ -862,6 +859,9 @@ where /// versa. Your application should check the id again once a frame has /// been received to make sure it is the expected value. /// + /// You may use a `const {}` block to ensure that the filter is parsed + /// during program compilation. + /// /// [ESP32C3 Reference Manual](https://www.espressif.com/sites/default/files/documentation/esp32-c3_technical_reference_manual_en.pdf#subsubsection.29.4.6) pub fn set_filter(&mut self, filter: impl Filter) { // Set or clear the rx filter mode bit depending on the filter type. From 5d5f6a651a07c8a3691cd898d3d1c4ca1c72c4a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 20 Sep 2024 19:56:57 +0200 Subject: [PATCH 09/16] Fix resetting TWAI --- esp-hal/src/twai/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/esp-hal/src/twai/mod.rs b/esp-hal/src/twai/mod.rs index 8d719c661c1..9f489341c95 100644 --- a/esp-hal/src/twai/mod.rs +++ b/esp-hal/src/twai/mod.rs @@ -755,8 +755,8 @@ where crate::into_ref!(tx_pin, rx_pin); // Enable the peripheral clock for the TWAI peripheral. - T::reset_peripheral(); T::enable_peripheral(); + T::reset_peripheral(); // Set RESET bit to 1 T::register_block() @@ -1495,7 +1495,7 @@ impl Instance for crate::peripherals::TWAI0 { } fn reset_peripheral() { - PeripheralClockControl::enable(crate::system::Peripheral::Twai0); + PeripheralClockControl::reset(crate::system::Peripheral::Twai0); } fn enable_peripheral() { From a152848e0a84f869fb5659f8351051ef9d204acb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 20 Sep 2024 20:11:28 +0200 Subject: [PATCH 10/16] Set opmode when starting --- esp-hal/src/twai/mod.rs | 40 ++++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/esp-hal/src/twai/mod.rs b/esp-hal/src/twai/mod.rs index 9f489341c95..a0685cbcc02 100644 --- a/esp-hal/src/twai/mod.rs +++ b/esp-hal/src/twai/mod.rs @@ -736,6 +736,7 @@ impl BaudRate { pub struct TwaiConfiguration<'d, T, DM: crate::Mode> { peripheral: PhantomData<&'d PeripheralRef<'d, T>>, phantom: PhantomData, + mode: TwaiMode, } impl<'d, T, DM> TwaiConfiguration<'d, T, DM> @@ -763,6 +764,12 @@ where .mode() .write(|w| w.reset_mode().set_bit()); + // Enable extended register layout + #[cfg(esp32)] + T::register_block() + .clock_divider() + .modify(|r, w| unsafe { w.bits(r.bits() | 0x80) }); + if no_transceiver { tx_pin.set_to_open_drain_output(crate::private::Internal); } else { @@ -776,12 +783,8 @@ where rx_pin.init_input(Pull::None, crate::private::Internal); rx_pin.connect_input_to_peripheral(T::INPUT_SIGNAL, crate::private::Internal); - // Set the operating mode based on provided option - T::register_block().mode().modify(|_, w| { - // self-test mode turns off acknowledgement requirement - w.self_test_mode().bit(mode == TwaiMode::SelfTest); - w.listen_only_mode().bit(mode == TwaiMode::ListenOnly) - }); + // Freeze REC by changing to LOM mode + Self::set_mode(TwaiMode::ListenOnly); // Set TEC to 0 T::register_block() @@ -801,6 +804,7 @@ where let mut cfg = TwaiConfiguration { peripheral: PhantomData, phantom: PhantomData, + mode, }; cfg.set_baud_rate(baud_rate); @@ -892,9 +896,32 @@ where .write(|w| unsafe { w.err_warning_limit().bits(limit) }); } + fn mode() -> TwaiMode { + let mode = T::register_block().mode().read(); + + if mode.self_test_mode().bit_is_set() { + TwaiMode::SelfTest + } else if mode.listen_only_mode().bit_is_set() { + TwaiMode::ListenOnly + } else { + TwaiMode::Normal + } + } + + /// Set the operating mode based on provided option + fn set_mode(mode: TwaiMode) { + T::register_block().mode().modify(|_, w| { + // self-test mode turns off acknowledgement requirement + w.self_test_mode().bit(mode == TwaiMode::SelfTest); + w.listen_only_mode().bit(mode == TwaiMode::ListenOnly) + }); + } + /// Put the peripheral into Operation Mode, allowing the transmission and /// reception of packets using the new object. pub fn start(self) -> Twai<'d, T, DM> { + Self::set_mode(self.mode); + // Put the peripheral into operation mode by clearing the reset mode bit. T::register_block() .mode() @@ -1022,6 +1049,7 @@ where TwaiConfiguration { peripheral: PhantomData, phantom: PhantomData, + mode: TwaiConfiguration::::mode(), } } From 2736c3e9654842464df83145ef5c7fe78eaf5651 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 20 Sep 2024 20:17:52 +0200 Subject: [PATCH 11/16] Apply errata workaround --- esp-hal/src/twai/mod.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/esp-hal/src/twai/mod.rs b/esp-hal/src/twai/mod.rs index a0685cbcc02..12e09dd863a 100644 --- a/esp-hal/src/twai/mod.rs +++ b/esp-hal/src/twai/mod.rs @@ -922,6 +922,29 @@ where pub fn start(self) -> Twai<'d, T, DM> { Self::set_mode(self.mode); + // Clear the TEC and REC + T::register_block() + .tx_err_cnt() + .write(|w| unsafe { w.tx_err_cnt().bits(0) }); + + let rec = + if cfg!(any(esp32, esp32s2, esp32s3, esp32c3)) && self.mode == TwaiMode::ListenOnly { + // Errata workaround: Prevent transmission of dominant error frame while in + // listen only mode by setting REC to 128 before exiting reset mode. + // This forces the controller to be error passive (thus only transmits recessive + // bits). The TEC/REC remain frozen in listen only mode thus + // ensuring we remain error passive. + 128 + } else { + 0 + }; + T::register_block() + .rx_err_cnt() + .write(|w| unsafe { w.rx_err_cnt().bits(rec) }); + + // Clear any interrupts + _ = T::register_block().int_raw().read(); + // Put the peripheral into operation mode by clearing the reset mode bit. T::register_block() .mode() From 1fdff7be61dea4bf9c1dc1f9ccf84359e5550a20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 20 Sep 2024 20:30:07 +0200 Subject: [PATCH 12/16] Fix ESP32 baudrate --- esp-hal/src/twai/mod.rs | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/esp-hal/src/twai/mod.rs b/esp-hal/src/twai/mod.rs index 12e09dd863a..e809fbf79ef 100644 --- a/esp-hal/src/twai/mod.rs +++ b/esp-hal/src/twai/mod.rs @@ -764,11 +764,13 @@ where .mode() .write(|w| w.reset_mode().set_bit()); - // Enable extended register layout #[cfg(esp32)] - T::register_block() - .clock_divider() - .modify(|r, w| unsafe { w.bits(r.bits() | 0x80) }); + { + // Enable extended register layout + T::register_block() + .clock_divider() + .modify(|r, w| unsafe { w.bits(r.bits() | 0x80) }); + } if no_transceiver { tx_pin.set_to_open_drain_output(crate::private::Internal); @@ -836,7 +838,24 @@ where // have 1 subtracted from them before being stored into the register. let timing = baud_rate.timing(); - let prescale = (timing.baud_rate_prescaler / 2) - 1; + let mut prescaler = timing.baud_rate_prescaler; + + if cfg!(esp32) { + if timing.baud_rate_prescaler > 128 { + // Enable /2 baudrate divider + T::register_block() + .int_ena() + .modify(|r, w| unsafe { w.bits(r.bits() | 0x08) }); + prescaler = timing.baud_rate_prescaler / 2; + } else { + // Disable /2 baudrate divider + T::register_block() + .int_ena() + .modify(|r, w| unsafe { w.bits(r.bits() & !0xF7) }); + } + } + + let prescale = (prescaler / 2) - 1; let sjw = timing.sync_jump_width - 1; let tseg_1 = timing.tseg_1 - 1; let tseg_2 = timing.tseg_2 - 1; From 2bafa7601b66e232084618087dda7916a8aed7e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 20 Sep 2024 19:33:19 +0200 Subject: [PATCH 13/16] Clean up read_frame a bit --- esp-hal/src/twai/mod.rs | 36 +++++++++++------------------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/esp-hal/src/twai/mod.rs b/esp-hal/src/twai/mod.rs index e809fbf79ef..7df6cfd55ed 100644 --- a/esp-hal/src/twai/mod.rs +++ b/esp-hal/src/twai/mod.rs @@ -1486,33 +1486,20 @@ pub trait Instance: crate::private::Sealed { let dlc = (data_0 & 0b1111) as usize; // Read the payload from the packet and construct a frame. - let frame = if is_standard_format { + let (id, data_ptr) = if is_standard_format { // Frame uses standard 11 bit id. let data_1 = register_block.data_1().read().tx_byte_1().bits(); - let data_2 = register_block.data_2().read().tx_byte_2().bits(); let raw_id: u16 = ((data_1 as u16) << 3) | ((data_2 as u16) >> 5); - let id = StandardId::new(raw_id).unwrap(); - - if is_data_frame { - // Create a new frame from the contents of the appropriate TWAI_DATA_x_REG - // registers. - unsafe { - EspTwaiFrame::new_from_data_registers(id, register_block.data_3().as_ptr(), dlc) - } - } else { - EspTwaiFrame::new_remote(id, dlc).unwrap() - } + let id = Id::from(StandardId::new(raw_id).unwrap()); + (id, register_block.data_3().as_ptr()) } else { // Frame uses extended 29 bit id. let data_1 = register_block.data_1().read().tx_byte_1().bits(); - let data_2 = register_block.data_2().read().tx_byte_2().bits(); - let data_3 = register_block.data_3().read().tx_byte_3().bits(); - let data_4 = register_block.data_4().read().tx_byte_4().bits(); let raw_id: u32 = (data_1 as u32) << 21 @@ -1520,20 +1507,19 @@ pub trait Instance: crate::private::Sealed { | (data_3 as u32) << 5 | (data_4 as u32) >> 3; - let id = ExtendedId::new(raw_id).unwrap(); + let id = Id::from(ExtendedId::new(raw_id).unwrap()); + (id, register_block.data_5().as_ptr()) + }; - if is_data_frame { - unsafe { - EspTwaiFrame::new_from_data_registers(id, register_block.data_5().as_ptr(), dlc) - } - } else { - EspTwaiFrame::new_remote(id, dlc).unwrap() - } + let frame = if is_data_frame { + unsafe { EspTwaiFrame::new_from_data_registers(id, data_ptr, dlc) } + } else { + EspTwaiFrame::new_remote(id, dlc).unwrap() }; // Release the packet we read from the FIFO, allowing the peripheral to prepare // the next packet. - register_block.cmd().write(|w| w.release_buf().set_bit()); + Self::release_receive_fifo(); frame } From 36a54d9bd2bff85b4b603f5ec5737aa7d15ed03c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 20 Sep 2024 20:39:11 +0200 Subject: [PATCH 14/16] Changelog --- esp-hal/CHANGELOG.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index d663bd4c40e..fc3cfb450cf 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -44,14 +44,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Removed gpio pin generics from I8080 driver type. (#2171) - I8080 driver now decides bus width at transfer time rather than construction time. (#2171) - Replaced `AnyPin` with `InputSignal` and `OutputSignal` and renamed `ErasedPin` to `AnyPin` (#2128) -- Replaced the `ErasedTimer` enum with the `AnyTimer` struct. (#?) +- Replaced the `ErasedTimer` enum with the `AnyTimer` struct. (#2144) - Changed the parameters of `Spi::with_pins` to no longer be optional (#2133) - Renamed `DummyPin` to `NoPin` and removed all internal logic from it. (#2133) - The `NO_PIN` constant has been removed. (#2133) - MSRV bump to 1.79 (#2156) - Allow handling interrupts while trying to lock critical section on multi-core chips. (#2197) - Removed the PS-RAM related features, replaced by `quad-psram`/`octal-psram`, `init_psram` takes a configuration parameter, it's now possible to auto-detect PS-RAM size (#2178) -- `EspTwaiFrame` constructors now accept any type that converts into `esp_hal::twai::Id` (#?) +- `EspTwaiFrame` constructors now accept any type that converts into `esp_hal::twai::Id` (#2207) ### Fixed @@ -68,6 +68,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - SPI: Fixed an issue where `wait` has returned before the DMA has finished writing the memory (#2179) - SPI: Fixed an issue where repeated calls to `dma_transfer` may end up looping indefinitely (#2179) - SPI: Fixed an issue that prevented correctly reading the first byte in a transaction (#2179) +- TWAI on ESP32 (#2207) ### Removed From 07b54d168b64305d89a11502badabd1a2788e60c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 20 Sep 2024 20:51:57 +0200 Subject: [PATCH 15/16] Clean up clippy --- esp-hal/src/twai/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/esp-hal/src/twai/mod.rs b/esp-hal/src/twai/mod.rs index 7df6cfd55ed..ea84c1b3b99 100644 --- a/esp-hal/src/twai/mod.rs +++ b/esp-hal/src/twai/mod.rs @@ -468,7 +468,7 @@ impl EspTwaiFrame { } let mut d: [u8; 8] = [0; 8]; - (&mut d[..data.len()]).copy_from_slice(data); + d[..data.len()].copy_from_slice(data); Some(EspTwaiFrame { id: id.into(), @@ -504,7 +504,7 @@ impl EspTwaiFrame { } let mut d: [u8; 8] = [0; 8]; - (&mut d[..data.len()]).copy_from_slice(data); + d[..data.len()].copy_from_slice(data); Some(EspTwaiFrame { id: id.into(), From 7b4839ec416509f8bfca15c7479afe1975c2bdf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 20 Sep 2024 21:04:05 +0200 Subject: [PATCH 16/16] Fix compile errors --- esp-hal/src/twai/mod.rs | 14 +++++++++++--- examples/src/bin/twai.rs | 8 ++++---- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/esp-hal/src/twai/mod.rs b/esp-hal/src/twai/mod.rs index ea84c1b3b99..6d9bd2ffa85 100644 --- a/esp-hal/src/twai/mod.rs +++ b/esp-hal/src/twai/mod.rs @@ -838,9 +838,11 @@ where // have 1 subtracted from them before being stored into the register. let timing = baud_rate.timing(); + #[cfg_attr(not(esp32), allow(unused_mut))] let mut prescaler = timing.baud_rate_prescaler; - if cfg!(esp32) { + #[cfg(esp32)] + { if timing.baud_rate_prescaler > 128 { // Enable /2 baudrate divider T::register_block() @@ -961,8 +963,14 @@ where .rx_err_cnt() .write(|w| unsafe { w.rx_err_cnt().bits(rec) }); - // Clear any interrupts - _ = T::register_block().int_raw().read(); + // Clear any interrupts by reading the status register + cfg_if::cfg_if! { + if #[cfg(any(esp32, esp32c3, esp32s2, esp32s3))] { + let _ = T::register_block().int_raw().read(); + } else { + let _ = T::register_block().interrupt().read(); + } + } // Put the peripheral into operation mode by clearing the reset mode bit. T::register_block() diff --git a/examples/src/bin/twai.rs b/examples/src/bin/twai.rs index 14d690f4e22..89e1e2cedde 100644 --- a/examples/src/bin/twai.rs +++ b/examples/src/bin/twai.rs @@ -65,9 +65,9 @@ fn main() -> ! { // be explicitly checked in the application instead of fully relying on // these partial acceptance filters to exactly match. // A filter that matches StandardId::ZERO. - const FILTER: SingleStandardFilter = - SingleStandardFilter::new(b"00000000000", b"x", [b"xxxxxxxx", b"xxxxxxxx"]); - twai_config.set_filter(FILTER); + twai_config.set_filter( + const { SingleStandardFilter::new(b"00000000000", 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 @@ -77,7 +77,7 @@ fn main() -> ! { if IS_FIRST_SENDER { // Send a frame to the other ESP // Use `new_self_reception` if you want to use self-testing. - let frame = EspTwaiFrame::new(StandardId::ZERO.into(), &[1, 2, 3]).unwrap(); + let frame = EspTwaiFrame::new(StandardId::ZERO, &[1, 2, 3]).unwrap(); block!(can.transmit(&frame)).unwrap(); println!("Sent a frame"); }