Skip to content

Commit

Permalink
Async: !Send (#2980)
Browse files Browse the repository at this point in the history
* Actually add inherent set_interrupt_handler fns

* Mark Async drivers !Send
  • Loading branch information
bugadani authored Jan 20, 2025
1 parent 9ab751f commit 04ba779
Show file tree
Hide file tree
Showing 9 changed files with 131 additions and 27 deletions.
2 changes: 2 additions & 0 deletions esp-hal/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- `Rng` and `Trng` now implement `Peripheral<P = Self>` (#2992)

- `Async` drivers are no longer `Send` (#2980)

### Fixed

- `DmaDescriptor` is now `#[repr(C)]` (#2988)
Expand Down
23 changes: 22 additions & 1 deletion esp-hal/MIGRATING-0.23.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,25 @@
# Migration Guide from v0.23.x to v?.??.?
# Migration Guide from v0.23.x to v1.0.0-beta.0

## `Async` drivers can no longer be sent between cores and executors

To work around this limitation, send the blocking driver, and configure it into `Async` mode
in the target context.

```diff
#[embassy_executor::task]
-async fn interrupt_driven_task(mut spi: Spi<'static, Async>) {
+async fn interrupt_driven_task(spi: Spi<'static, Blocking>) {
+ let mut spi = spi.into_async();
...
}

let spi = Spi::new(...)
.unwrap()
// ...
- .into_async();

send_spawner.spawn(interrupt_driven_task(spi)).unwrap();
```

## RMT changes

Expand Down
21 changes: 20 additions & 1 deletion esp-hal/src/i2c/master/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,25 @@ impl<'d> I2c<'d, Blocking> {
Ok(i2c)
}

#[cfg_attr(
not(multi_core),
doc = "Registers an interrupt handler for the peripheral."
)]
#[cfg_attr(
multi_core,
doc = "Registers an interrupt handler for the peripheral on the current core."
)]
#[doc = ""]
/// Note that this will replace any previously registered interrupt
/// handlers.
///
/// You can restore the default/unhandled interrupt handler by using
/// [crate::DEFAULT_INTERRUPT_HANDLER]
#[instability::unstable]
fn set_interrupt_handler(&mut self, handler: InterruptHandler) {
self.i2c.info().set_interrupt_handler(handler);
}

/// Listen for the given interrupts
#[instability::unstable]
pub fn listen(&mut self, interrupts: impl Into<EnumSet<Event>>) {
Expand Down Expand Up @@ -715,7 +734,7 @@ impl<'d> I2c<'d, Blocking> {
impl private::Sealed for I2c<'_, Blocking> {}

impl InterruptConfigurable for I2c<'_, Blocking> {
fn set_interrupt_handler(&mut self, handler: crate::interrupt::InterruptHandler) {
fn set_interrupt_handler(&mut self, handler: InterruptHandler) {
self.i2c.info().set_interrupt_handler(handler);
}
}
Expand Down
20 changes: 18 additions & 2 deletions esp-hal/src/i2s/master.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,33 +264,49 @@ impl<Dm> I2s<'_, Dm>
where
Dm: DriverMode,
{
/// Sets the interrupt handler
#[cfg_attr(
not(multi_core),
doc = "Registers an interrupt handler for the peripheral."
)]
#[cfg_attr(
multi_core,
doc = "Registers an interrupt handler for the peripheral on the current core."
)]
#[doc = ""]
/// Note that this will replace any previously registered interrupt
/// handlers.
///
/// Interrupts are not enabled at the peripheral level here.
/// You can restore the default/unhandled interrupt handler by using
/// [crate::interrupt::DEFAULT_INTERRUPT_HANDLER]
#[instability::unstable]
pub fn set_interrupt_handler(&mut self, handler: InterruptHandler) {
// tx.i2s and rx.i2s is the same, we could use either one
self.i2s_tx.i2s.set_interrupt_handler(handler);
}

/// Listen for the given interrupts
#[instability::unstable]
pub fn listen(&mut self, interrupts: impl Into<EnumSet<I2sInterrupt>>) {
// tx.i2s and rx.i2s is the same, we could use either one
self.i2s_tx.i2s.enable_listen(interrupts.into(), true);
}

/// Unlisten the given interrupts
#[instability::unstable]
pub fn unlisten(&mut self, interrupts: impl Into<EnumSet<I2sInterrupt>>) {
// tx.i2s and rx.i2s is the same, we could use either one
self.i2s_tx.i2s.enable_listen(interrupts.into(), false);
}

/// Gets asserted interrupts
#[instability::unstable]
pub fn interrupts(&mut self) -> EnumSet<I2sInterrupt> {
// tx.i2s and rx.i2s is the same, we could use either one
self.i2s_tx.i2s.interrupts()
}

/// Resets asserted interrupts
#[instability::unstable]
pub fn clear_interrupts(&mut self, interrupts: impl Into<EnumSet<I2sInterrupt>>) {
// tx.i2s and rx.i2s is the same, we could use either one
self.i2s_tx.i2s.clear_interrupts(interrupts.into());
Expand Down
17 changes: 12 additions & 5 deletions esp-hal/src/interrupt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,18 @@ pub const DEFAULT_INTERRUPT_HANDLER: InterruptHandler = InterruptHandler::new(
/// Trait implemented by drivers which allow the user to set an
/// [InterruptHandler]
pub trait InterruptConfigurable: crate::private::Sealed {
/// Set the interrupt handler
///
/// Note that this will replace any previously registered interrupt handler.
/// Some peripherals offer a shared interrupt handler for multiple purposes.
/// It's the users duty to honor this.
#[cfg_attr(
not(multi_core),
doc = "Registers an interrupt handler for the peripheral."
)]
#[cfg_attr(
multi_core,
doc = "Registers an interrupt handler for the peripheral on the current core."
)]
#[doc = ""]
/// Note that this will replace any previously registered interrupt
/// handlers. Some peripherals offer a shared interrupt handler for
/// multiple purposes. It's the users duty to honor this.
///
/// You can restore the default/unhandled interrupt handler by using
/// [DEFAULT_INTERRUPT_HANDLER]
Expand Down
6 changes: 5 additions & 1 deletion esp-hal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@
// MUST be the first module
mod fmt;

use core::marker::PhantomData;

#[cfg(riscv)]
#[cfg_attr(docsrs, doc(cfg(feature = "unstable")))]
#[cfg_attr(not(feature = "unstable"), doc(hidden))]
Expand Down Expand Up @@ -321,7 +323,9 @@ pub struct Blocking;

/// Driver initialized in async mode.
#[derive(Debug)]
pub struct Async;
pub struct Async(PhantomData<*const ()>);

unsafe impl Sync for Async {}

impl crate::DriverMode for Blocking {}
impl crate::DriverMode for Async {}
Expand Down
27 changes: 23 additions & 4 deletions esp-hal/src/spi/master.rs
Original file line number Diff line number Diff line change
Expand Up @@ -583,12 +583,22 @@ impl<'d> Spi<'d, Blocking> {
{
SpiDma::new(self.spi, channel.map(|ch| ch.degrade()).into_ref())
}
}

impl InterruptConfigurable for Spi<'_, Blocking> {
/// Sets the interrupt handler
#[cfg_attr(
not(multi_core),
doc = "Registers an interrupt handler for the peripheral."
)]
#[cfg_attr(
multi_core,
doc = "Registers an interrupt handler for the peripheral on the current core."
)]
#[doc = ""]
/// Note that this will replace any previously registered interrupt
/// handlers.
///
/// Interrupts are not enabled at the peripheral level here.
/// You can restore the default/unhandled interrupt handler by using
/// [crate::interrupt::DEFAULT_INTERRUPT_HANDLER]
#[instability::unstable]
fn set_interrupt_handler(&mut self, handler: InterruptHandler) {
let interrupt = self.driver().info.interrupt;
for core in Cpu::other() {
Expand All @@ -599,6 +609,15 @@ impl InterruptConfigurable for Spi<'_, Blocking> {
}
}

impl InterruptConfigurable for Spi<'_, Blocking> {
/// Sets the interrupt handler
///
/// Interrupts are not enabled at the peripheral level here.
fn set_interrupt_handler(&mut self, handler: InterruptHandler) {
Spi::set_interrupt_handler(self, handler);
}
}

impl<'d> Spi<'d, Async> {
/// Converts the SPI instance into blocking mode.
pub fn into_blocking(self) -> Spi<'d, Blocking> {
Expand Down
22 changes: 21 additions & 1 deletion esp-hal/src/uart.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1276,13 +1276,33 @@ where
impl crate::private::Sealed for Uart<'_, Blocking> {}

impl InterruptConfigurable for Uart<'_, Blocking> {
fn set_interrupt_handler(&mut self, handler: crate::interrupt::InterruptHandler) {
fn set_interrupt_handler(&mut self, handler: InterruptHandler) {
// `self.tx.uart` and `self.rx.uart` are the same
self.tx.uart.info().set_interrupt_handler(handler);
}
}

impl Uart<'_, Blocking> {
#[cfg_attr(
not(multi_core),
doc = "Registers an interrupt handler for the peripheral."
)]
#[cfg_attr(
multi_core,
doc = "Registers an interrupt handler for the peripheral on the current core."
)]
#[doc = ""]
/// Note that this will replace any previously registered interrupt
/// handlers.
///
/// You can restore the default/unhandled interrupt handler by using
/// [crate::interrupt::DEFAULT_INTERRUPT_HANDLER]
#[instability::unstable]
pub fn set_interrupt_handler(&mut self, handler: InterruptHandler) {
// `self.tx.uart` and `self.rx.uart` are the same
self.tx.uart.info().set_interrupt_handler(handler);
}

/// Listen for the given interrupts
#[instability::unstable]
pub fn listen(&mut self, interrupts: impl Into<EnumSet<UartInterrupt>>) {
Expand Down
20 changes: 8 additions & 12 deletions hil-test/tests/embassy_interrupt_spi_dma.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use esp_hal::{
},
time::RateExtU32,
timer::AnyTimer,
Async,
Blocking,
};
use esp_hal_embassy::InterruptExecutor;
use hil_test as _;
Expand All @@ -37,14 +37,14 @@ static INTERRUPT_TASK_WORKING: AtomicBool = AtomicBool::new(false);

#[cfg(any(esp32, esp32s2, esp32s3))]
#[embassy_executor::task]
async fn interrupt_driven_task(spi: esp_hal::spi::master::SpiDma<'static, Async>) {
async fn interrupt_driven_task(spi: esp_hal::spi::master::SpiDma<'static, Blocking>) {
let mut ticker = Ticker::every(Duration::from_millis(1));

let (rx_buffer, rx_descriptors, tx_buffer, tx_descriptors) = dma_buffers!(128);
let dma_rx_buf = DmaRxBuf::new(rx_descriptors, rx_buffer).unwrap();
let dma_tx_buf = DmaTxBuf::new(tx_descriptors, tx_buffer).unwrap();

let mut spi = spi.with_buffers(dma_rx_buf, dma_tx_buf);
let mut spi = spi.with_buffers(dma_rx_buf, dma_tx_buf).into_async();

loop {
let mut buffer: [u8; 8] = [0; 8];
Expand All @@ -59,9 +59,11 @@ async fn interrupt_driven_task(spi: esp_hal::spi::master::SpiDma<'static, Async>

#[cfg(not(any(esp32, esp32s2, esp32s3)))]
#[embassy_executor::task]
async fn interrupt_driven_task(mut i2s_tx: esp_hal::i2s::master::I2sTx<'static, Async>) {
async fn interrupt_driven_task(i2s_tx: esp_hal::i2s::master::I2s<'static, Blocking>) {
let mut ticker = Ticker::every(Duration::from_millis(1));

let mut i2s_tx = i2s_tx.into_async().i2s_tx.build();

loop {
let mut buffer: [u8; 8] = [0; 8];

Expand Down Expand Up @@ -137,14 +139,13 @@ mod test {
.with_mode(Mode::_0),
)
.unwrap()
.with_dma(dma_channel2)
.into_async();
.with_dma(dma_channel2);

#[cfg(not(any(esp32, esp32s2, esp32s3)))]
let other_peripheral = {
let (_, rx_descriptors, _, tx_descriptors) = dma_buffers!(128);

let other_peripheral = esp_hal::i2s::master::I2s::new(
esp_hal::i2s::master::I2s::new(
peripherals.I2S0,
esp_hal::i2s::master::Standard::Philips,
esp_hal::i2s::master::DataFormat::Data8Channel8,
Expand All @@ -153,11 +154,6 @@ mod test {
rx_descriptors,
tx_descriptors,
)
.into_async()
.i2s_tx
.build();

other_peripheral
};

let sw_ints = SoftwareInterruptControl::new(peripherals.SW_INTERRUPT);
Expand Down

0 comments on commit 04ba779

Please sign in to comment.