search

WMT-GmbH / pn532

no_std implementation of the Pn532 protocol using embedded_hal traits
Apache License 2.0
20 stars 19 forks source link

Async doesn't work #11

Closed averyanalex closed 1 year ago

averyanalex commented 1 year ago

I just tried async version and it did't work. (I use esp32c3 with embassy). Process_async hangs here. I added debug println to spi interface: wait_ready called only 1 time.

Maybe pass async timer and use it for retries? (Call wait_ready, if pending - run something like Timer::after(Duration::from_millis(10)).await;). For IRQ version it is also possible to use something like wait_for_low. Also maybe better to implement response waiting in interfaces (And add AsyncInterface trait)? For irq - with wait_for_low, for non-irq - with timer.

dimpolo commented 1 year ago

Interesting. I imagine the process hangs, because we don't schedule the task to be woken up at any point.

How did you get embassy to run on the esp32? I'd like to replicate the issue :)

Maybe pass async timer and use it for retries? (Call wait_ready, if pending - run something like Timer::after(Duration::from_millis(10)).await;). For IRQ version it is also possible to use something like wait_for_low. Also maybe better to implement response waiting in interfaces (And add AsyncInterface trait)? For irq - with wait_for_low, for non-irq - with timer.

Yeah, I think you're right and some kind of redesign is required. Maybe we can use the (hopefully) soon to be stabilized async_fn_in_trait feature.

averyanalex commented 1 year ago

Interesting. I imagine the process hangs, because we don't schedule the task to be woken up at any point.

How did you get embassy to run on the esp32? I'd like to replicate the issue :)

Maybe pass async timer and use it for retries? (Call wait_ready, if pending - run something like Timer::after(Duration::from_millis(10)).await;). For IRQ version it is also possible to use something like wait_for_low. Also maybe better to implement response waiting in interfaces (And add AsyncInterface trait)? For irq - with wait_for_low, for non-irq - with timer.

Yeah, I think you're right and some kind of redesign is required. Maybe we can use the (hopefully) soon to be stabilized async_fn_in_trait feature.

I used code from esp-rs embassy hello world: https://github.com/esp-rs/esp-hal/blob/main/esp32c3-hal/examples/embassy_hello_world.rs. If you want, I can email you zip/tgz with code for esp32c3 (that hangs).

dimpolo commented 1 year ago

I used code from esp-rs embassy hello world: https://github.com/esp-rs/esp-hal/blob/main/esp32c3-hal/examples/embassy_hello_world.rs. If you want, I can email you zip/tgz with code for esp32c3 (that hangs).

Thanks! I wasn't aware that esp-rs had this feature.

I was able to reproduce the issue and write a short-term fix.

In the long term I'd like to investigate better async integration, making use of timers and wait_for_low integration as you mentioned

averyanalex commented 1 year ago

BTW maybe add support for async spi when it will be implemented in hals?