esp-rs / esp-idf-hal

embedded-hal implementation for Rust on ESP32 and ESP-IDF
https://docs.esp-rs.org/esp-idf-hal/
Apache License 2.0
443 stars 169 forks source link

Panic on Peripherals access from concurrent async tasks #469

Closed Serhii-the-Dev closed 1 month ago

Serhii-the-Dev commented 1 month ago

When there are two parallel async tasks and each of those tasks tries to Peripherals::take() the code crashes on the very moment a second task invokes that method. It seems the issue is tied to atomics somehow however I'm not familiar nor with MCUs in general nor with ESP-IDF architecture to elaborate on that. The MCU used: ESP32-PICO-D4 on a LILYGO dev board. I have a demo program that setups a WiFi hotspot and runs UART listener in parallel, the idea is to have a board connected to another device via serial and control it from a laptop/phone for a limited timespan (kind of a first-time-setup scenario), however the panic could be reproduced on any other example which includes the parallel async tasks scenario. Here is my sample code (also adding demo.zip with a full project):

use core::convert::TryInto;

use edge_executor::LocalExecutor;
use embedded_svc::wifi::{self, AccessPointConfiguration, AuthMethod};
use esp_idf_svc::{
    eventloop::EspSystemEventLoop, hal::uart::AsyncUartRxDriver, nvs::EspDefaultNvsPartition,
    timer::EspTaskTimerService, wifi::EspWifi,
};
use esp_idf_svc::{
    hal::{
        gpio,
        prelude::Peripherals,
        task,
        timer::{TimerConfig, TimerDriver},
        uart::{self},
        units::Hertz,
    },
    wifi::AsyncWifi,
};
use futures;
use log::*;

const SSID: &str = env!("WIFI_SSID");
const PASSWORD: &str = env!("WIFI_PASS");

fn main() -> anyhow::Result<()> {
    esp_idf_svc::sys::link_patches();
    esp_idf_svc::log::EspLogger::initialize_default();

    let local_ex: LocalExecutor = Default::default();

    let wifi_task = local_ex.spawn(async move {
        let _ = setup_wifi().await;
    });

    let uart_task = local_ex.spawn(async move {
        let _ = read_uart(Hertz(400_000)).await;
    });

    task::block_on(local_ex.run(async { futures::join!(wifi_task, uart_task) }));

    Ok(())
}

async fn setup_wifi() {
    println!("Accessing peripherals");
    let peripherals = Peripherals::take().expect("Can not access peripherals");
    let sys_loop = EspSystemEventLoop::take().expect("Can not access system loop");
    let nvs = EspDefaultNvsPartition::take().expect("Can not access NVS partition");
    let timer_service = EspTaskTimerService::new().expect("Can not create task timer service");

    let mut wifi = AsyncWifi::wrap(
        EspWifi::new(peripherals.modem, sys_loop.clone(), Some(nvs))
            .expect("Can not create ESP WiFi"),
        sys_loop,
        timer_service,
    )
    .expect("Can not create WiFi");

    let wifi_configuration = wifi::Configuration::AccessPoint(AccessPointConfiguration {
        ssid: SSID.try_into().unwrap(),
        ssid_hidden: false,
        auth_method: AuthMethod::WPA2Personal,
        password: PASSWORD.try_into().unwrap(),
        ..Default::default()
    });
    wifi.set_configuration(&wifi_configuration)
        .expect("Can not set WiFi configuration");
    wifi.start().await.expect("Can not start WiFi");
    wifi.wait_netif_up()
        .await
        .expect("Can not start net interface");

    info!(
        "Created Wi-Fi with WIFI_SSID `{}` and WIFI_PASS `{}`",
        SSID, PASSWORD
    );

    println!("Initializing timer");
    let mut timer = TimerDriver::new(peripherals.timer01, &TimerConfig::new())
        .expect("Can not create timer driver");

    println!("Awaiting 10 seconds");
    timer
        .delay(timer.tick_hz() * 10)
        .await
        .expect("Can not delay");

    wifi.stop().await.expect("Can not stop WiFi");
}

async fn read_uart<'t>(baudrate: Hertz) {
    println!("Starting UART listener");
    let peripherals = Peripherals::take().unwrap();

    let rx = peripherals.pins.gpio13;
    let config = uart::config::Config::new().baudrate(baudrate);

    println!("Creating UART driver");
    let serial = AsyncUartRxDriver::new(
        peripherals.uart1,
        rx,
        Option::<gpio::Gpio0>::None,
        Option::<gpio::Gpio1>::None,
        &config,
    )
    .expect("Can not create UART driver");

    let mut buf = [0_u8; 1];

    loop {
        let _ = serial.read(&mut buf).await;
        println!("Read 0x{:02x}", buf[0]);
    }
}

The trace I have:

Starting UART listener
Guru Meditation Error: Core  0 panic'ed (Unhandled debug exception). 
Debug exception reason: BREAK instr 
Core  0 register dump:
PC      : 0x400d39c3  PS      : 0x00060336  A0      : 0x800d7e9a  A1      : 0x3ffbde30  
0x400d39c3 - async_task::raw::RawTask<F,T,S,M>::run
    at ??:??
A2      : 0x00000000  A3      : 0x400e8b78  A4      : 0x00061a80  A5      : 0x3ffbf7e4  
0x400e8b78 - core::panicking::panic_fmt
    at ??:??
A6      : 0x00000004  A7      : 0x00000001  A8      : 0x800d3309  A9      : 0x3ffbde10  
A10     : 0x00000103  A11     : 0x00000111  A12     : 0x000000ff  A13     : 0x3ffb3d78  
0x3ffb3d78 - esp_idf_hal::interrupt::ISR_YIELDER.1
    at ??:??
A14     : 0x3ffb3e94  A15     : 0x3ffbf460  SAR     : 0x00000004  EXCCAUSE: 0x00000001  
0x3ffb3e94 - std::rt::cleanup::CLEANUP
    at ??:??
EXCVADDR: 0x00000000  LBEG    : 0x4000c2e0  LEND    : 0x4000c2f6  LCOUNT  : 0x00000000  

Backtrace: 0x400d39c0:0x3ffbde30 0x400d7e97:0x3ffbded0 0x400d81ab:0x3ffbe230 0x400e118e:0x3ffbe280 0x4016a757:0x3ffbe2a0 0x4008cc61:0x3ffbe2d0
Vollbrecht commented 1 month ago

well take() can only called once. As it "takes out the peripheral tree", From that central struct you then hand out the handles where you need. If you call it a second time you will get a None. And since you are unwrapping it just panics

Serhii-the-Dev commented 1 month ago

well take() can only called once. As it "takes out the peripheral tree", From that central struct you then hand out the handles where you need. If you call it a second time you will get a None. And since you are unwrapping it just panics

Thank you, I've just put the drivers creation on top of the main with a single call to the Peripherals::take and now it's working. Sorry for a false alarm, I'm closing the issue.