Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Async: !Send #2980

Merged
merged 2 commits into from
Jan 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -601,6 +601,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 @@ -707,7 +726,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
Loading