esp-rs / esp-idf-svc

Type-Safe Rust Wrappers for various ESP-IDF services (WiFi, Network, Httpd, Logging, etc.)
https://docs.esp-rs.org/esp-idf-svc/
Apache License 2.0
332 stars 183 forks source link

EspSystemEventLoop async subscription prevents AsyncWifi::start() from finishing #480

Closed Veanir closed 2 months ago

Veanir commented 2 months ago

Unexpected behaviour:

Program gets stuck on wifi.start().await? whenever the EspSystemEventLoop is subscribed to with EspEventLoop::subscribe_async().

How to reproduce:

Run this slightly modified example.

//! Example of using async wifi.
//!
//! Add your own ssid and password

use core::convert::TryInto;

use esp_idf_svc::hal::prelude::Peripherals;
use esp_idf_svc::hal::task::block_on;
use esp_idf_svc::log::EspLogger;
use esp_idf_svc::timer::EspTaskTimerService;
use esp_idf_svc::wifi::{
    AsyncWifi, AuthMethod, ClientConfiguration, Configuration, EspWifi, WifiEvent,
};
use esp_idf_svc::{eventloop::EspSystemEventLoop, nvs::EspDefaultNvsPartition};

use log::info;

const SSID: &str = "YOUR_SSID";
const PASSWORD: &str = "YOUR_PASSWORD";

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

    let peripherals = Peripherals::take()?;
    let sys_loop = EspSystemEventLoop::take()?;
    let timer_service = EspTaskTimerService::new()?;
    let nvs = EspDefaultNvsPartition::take()?;

    esp_idf_svc::hal::task::block_on(async move {
        let mut wifi = AsyncWifi::wrap(
            EspWifi::new(peripherals.modem, sys_loop.clone(), Some(nvs)).unwrap(),
            sys_loop.clone(),
            timer_service,
        )
        .unwrap();

        let mut _subscription = sys_loop.subscribe_async::<WifiEvent>().unwrap(); // <------

        block_on(connect_wifi(&mut wifi)).unwrap(); // <------
        let ip_info = wifi.wifi().sta_netif().get_ip_info().unwrap();

        info!("Wifi DHCP info: {:?}", ip_info);

        info!("Shutting down in 5s...");

        std::thread::sleep(core::time::Duration::from_secs(5));
    });

    Ok(())
}

async fn connect_wifi(wifi: &mut AsyncWifi<EspWifi<'static>>) -> anyhow::Result<()> {
    let wifi_configuration: Configuration = Configuration::Client(ClientConfiguration {
        ssid: SSID.try_into().unwrap(),
        bssid: None,
        auth_method: AuthMethod::WPA2Personal,
        password: PASSWORD.try_into().unwrap(),
        channel: None,
        ..Default::default()
    });

    wifi.set_configuration(&wifi_configuration)?;

    wifi.start().await?;//GETS STUCK HERE
    info!("Wifi started");

    wifi.connect().await?;
    info!("Wifi connected");

    wifi.wait_netif_up().await?;
    info!("Wifi netif up");

    Ok(())
}

notes

when subscription is performed after connecting to wifi everything is okay.

        block_on(connect_wifi(&mut wifi)).unwrap();
        let ip_info = wifi.wifi().sta_netif().get_ip_info().unwrap();

        let mut _subscription = sys_loop.subscribe_async::<WifiEvent>().unwrap();

Also when subscription is based on callback (EspEventLoop::subscribe()) it works fine even when it's performed before starting wifi.

ivmarkov commented 2 months ago

@Veanir I'm not sure we can qualify this behavior as a bug.

The thing is, once you call subscribe_async, then you MUST make sure that you poll the resulting future, or else the system event loop will get stuck, as we put a callback into it that needs to send its data to somewhere, and if you don't poll, it will wait forever.

The easiest way to fix this in your program is to call subscription.next().await from within your block_on executor, together with whatever else you do there (I.e. waiting on wifi start).

ivmarkov commented 2 months ago

You are also using block_on inside block_on. In 99.99% of the use cases there is no good reason to do that.

Veanir commented 2 months ago

Thank you very much for fast reply!

I was trying to do something like:

        let mut subscription = self.system_event_loop.subscribe_async::<EspWifiEvent>()?;
        loop {
            futures::select! {
                event = self.channels.wifi_credentials_rx.recv().fuse() => {
                    let credentials = match event {
                        Some(event) => event,
                        None => continue,
                    };
                    log::info!("Received WiFi credentials: {:?}", credentials);

                    self.connect_wifi(credentials).await?;
                }
                event = subscription.recv().fuse() => {
                    let event = match event {
                        Ok(event) => event,
                        Err(_) => continue,
                    };

                    match event {
                        EspWifiEvent::StaConnected => {
                            self.state = WifiState::Connected;
                            self.connect_websocket()?;
                        }
                        EspWifiEvent::StaDisconnected => {
                            self.state = WifiState::Disconnected;
                            self.ws_client = None;
                        }
                        _ => {}
                    }
                }
            }
        }

But I guess that I would need to keep polling subscription in different thread in order to make it work.

You saved me a lot of time, as I'm still not familiar with underlying mechanisms.

ivmarkov commented 2 months ago

Also, just a suggestion: avoid select!. Use embassy_futures::select instead.

Veanir commented 2 months ago

Btw why do you mix blocking calls (connect_websocket) with async calls?

There is no reason other than that I don't really have much experience with async rust... And to be honest not much expertise in anything asynchronous :D I'm just brute forcing my way through async rust

Thanks for suggestions