Support for asynchronous I/O

[TilBlechschmidt]

I am currently experimenting with the nRF52 and using the embassy project. I intend to use an SD Card in the project. However, this library does not appear to support asynchronous APIs. This includes two levels:

1. DMA related SPI store/fetch operations
   • You give it a tx+rx buffer, it at some point interrupts you once everything has been sent/received
   • Allows the user/application to do other work in the meantime
   • Currently unsupported by the SdMmcSpi struct as it solely uses byte-by-byte synchronous send/receive methods
2. Asynchronous block devices
   • Most disk I/O is inherently asynchronous as the CPU waits for the device
   • In a majority of use cases microcontrollers might not have much else to do
   • In some cases, other, time-critical tasks need to run (e.g. handling wireless transmissions)

---

My question here is: Would you be interested in integrating support for asynchronous SPI backends? Follow-up question: If yes, do you have any thoughts on how to do that besides just "implementing everything twice"

I can definitely see both arguments; for integrating it to have a unified place to look for when using SD cards; against stuffing everything into one crate even though it is basically its own thing (sadly, we can't simply pick and choose between async/sync APIs like we can with traits/generics).

[thejpster]

1. personally it doesn’t appeal, but someone could, sure.
2. No, not really. async is pervasive in a codebase.

[ninjasource]

Hi, I've been experimenting with implementing this and have the following to report.

• First, the good news. I got async file reads working and it's running about 8x faster than the blocking version on my nrf52480 chip using Embassy and DMA. The added benefit being that it does not hold up the mcu which was the reason why I needed the functionality in the first place (I'm working with audio).
• Now the bad news. This pretty much duplicates almost all of the codebase to the point where putting this in a separate repo or separate member of the same repo makes more sense to me than polluting this repo with feature flags in an effort to reuse some of the code. As mentioned previously, async is indeed very pervasive and duplicating everything like this is distasteful. I can't really see a way around this unfortunately.
• Dependency wise, the change essentially swaps embedded-hal with embedded-hal-async which is alpha right now but it is a member of the embedded-hal repo so it should be standard. This means that a nightly toolchain is needed with #![feature(async_fn_in_trait, impl_trait_projections)] features enabled.
• This is a proof of concept and I just wanted to judge the appetite for eventually merging something like this to the main develop branch. It works for my use case but I am happy to put in the time to finish it up properly and welcome any criticism or suggestions. I only really optimised the bulk read because that's where most of the time was spent for me. Hell, if you guys think this is one big bad idea then I won't be offended!
• I experimented with a few approaches, most notably sharing the filesystem module because that has nothing to do with IO but I found myself making things public that were meant to be private and I also thought that anyone looking at these changes would appreciate being able to simply do a folder compare against the blocking and async versions to see what the differences were instead of wading through my effort to deduplicate things.
• Here is the fork: https://github.com/ninjasource/embedded-sdmmc-rs/tree/add-async-support
• Here is an example project using Embassy as the runtime: https://github.com/ninjasource/audio-i2s-sdmmc-nrf52840-async

[thejpster]

I was thinking of moving the FAT32 code out into another crate, but even then that doesn't help because it does read operations and so has to have both sync and async versions.

It may be easier to fork the project until the generic keywords feature comes along.

[ninjasource]

Oh wow, this is the first time I've heard of the generics keywords initiative. Sounds very promising indeed! I will try to keep this fork up to date and hopefully others will find it useful for the time being.

Agreed, moving the FAT32 code out wouldn't help because of all that IO in the volume module which happens to make up most of the code.

The filesystem code can be shared between blocking and async but it would need a little refactoring to not expose the internals outside the crate. I ultimately abandoned my efforts there because it became ugly. It's a pity that pub(crate) is not visible between members of the same project.

[elpiel]

To join in the discussion, I like the approach which some other crates take on blocking vs async api - the default module async api with a second blocking module. Maybe this would make more sense for the crate as well, but in the other way around? i.e. blocking api by default and async one when a feature is enabled. This would leave both options on the table for crate users.

@ninjasource I tried your branch but currently it fails for me because of #33 I believe but it does a panic instead of getting this error. I took a look at the code and it seems that one of the loops was removed which might be causing this?

PS: I'm using embassy

0.000912 INFO  Init SD card...
└─ netbox_hardware::micro_sd::async_api::acquire_device::{async_fn#0} @ src/micro_sd.rs:103
0.001855 ERROR panicked at 'assertion failed: `(left == right)`'
diff < left / right >
<6
>1
└─ embassy_rp::spi::{impl#3}::transfer_inner::{async_fn#0} @ /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-rp/src/fmt.rs:24
0.001937 ERROR panicked at 'explicit panic', /home/elpiel/.cargo/registry/src/github.com-1ecc6299db9ec823/defmt-0.3.2/src/lib.rs:367:5
└─ panic_probe::print_defmt::print @ /home/elpiel/.cargo/registry/src/github.com-1ecc6299db9ec823/panic-probe-0.3.0/src/lib.rs:91
────────────────────────────────────────────────────────────────────────────────
stack backtrace:
   0: HardFaultTrampoline
      <exception entry>
   1: lib::inline::__udf
        at ./asm/inline.rs:181:5
   2: __udf
        at ./asm/lib.rs:51:17
   3: cortex_m::asm::udf
        at /home/elpiel/.cargo/registry/src/github.com-1ecc6299db9ec823/cortex-m-0.7.7/src/asm.rs:43:5
   4: rust_begin_unwind
        at /home/elpiel/.cargo/registry/src/github.com-1ecc6299db9ec823/panic-probe-0.3.0/src/lib.rs:72:9
   5: core::panicking::panic_fmt
        at /rustc/b7bc90fea3b441234a84b49fdafeb75815eebbab/library/core/src/panicking.rs:65:14
   6: core::panicking::panic
        at /rustc/b7bc90fea3b441234a84b49fdafeb75815eebbab/library/core/src/panicking.rs:114:5
   7: __defmt_default_panic
        at /home/elpiel/.cargo/registry/src/github.com-1ecc6299db9ec823/defmt-0.3.2/src/lib.rs:367:5
   8: defmt::export::panic
        at /home/elpiel/.cargo/registry/src/github.com-1ecc6299db9ec823/defmt-0.3.2/src/export/mod.rs:133:14
   9: embassy_rp::spi::Spi<T,embassy_rp::spi::Async>::transfer_inner::{{closure}}
        at /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-rp/src/spi.rs:388:9
  10: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
        at /rustc/b7bc90fea3b441234a84b49fdafeb75815eebbab/library/core/src/future/mod.rs:92:19
  11: embassy_rp::spi::Spi<T,embassy_rp::spi::Async>::transfer::{{closure}}
        at /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-rp/src/spi.rs:378:50
  12: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
        at /rustc/b7bc90fea3b441234a84b49fdafeb75815eebbab/library/core/src/future/mod.rs:92:19
  13: embassy_rp::spi::eha::<impl embedded_hal_async::spi::SpiBus for embassy_rp::spi::Spi<T,embassy_rp::spi::Async>>::transfer::{{closure}}
        at /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-rp/src/spi.rs:589:39
  14: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
        at /rustc/b7bc90fea3b441234a84b49fdafeb75815eebbab/library/core/src/future/mod.rs:92:19
  15: embedded_sdmmc_async::sdmmc::SdMmcSpi<SPI,CS>::card_command::{{closure}}
        at /mnt/Disk/Projects/lechev.space/embedded-sdmmc-async/embedded-sdmmc-async/src/sdmmc.rs:320:13
  16: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
        at /rustc/b7bc90fea3b441234a84b49fdafeb75815eebbab/library/core/src/future/mod.rs:92:19
  17: $t.148
  18: embassy_executor::raw::TaskStorage<F>::poll
  19: core::cell::Cell<T>::get
        at /rustc/b7bc90fea3b441234a84b49fdafeb75815eebbab/library/core/src/cell.rs:452:18
  20: embassy_executor::raw::timer_queue::TimerQueue::update
        at /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-executor/src/raw/timer_queue.rs:36:12
  21: embassy_executor::raw::Executor::poll::{{closure}}
        at /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-executor/src/raw/mod.rs:388:17
  22: embassy_executor::raw::run_queue::RunQueue::dequeue_all
        at /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-executor/src/raw/run_queue.rs:69:13
  23: core::cell::Cell<T>::get
        at /rustc/b7bc90fea3b441234a84b49fdafeb75815eebbab/library/core/src/cell.rs:452:18
  24: embassy_executor::raw::timer_queue::TimerQueue::retain
        at /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-executor/src/raw/timer_queue.rs:72:16
  25: embassy_executor::raw::timer_queue::TimerQueue::next_expiration
        at /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-executor/src/raw/timer_queue.rs:49:9
  26: embassy_executor::raw::Executor::poll
        at /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-executor/src/raw/mod.rs:395:39
  27: embassy_executor::arch::Executor::run
        at /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-executor/src/arch/cortex_m.rs:55:17
  28: write_to_sd_card_async::__cortex_m_rt_main
        at src/bin/write_to_sd_card_async.rs:21:1
  29: main
        at src/bin/write_to_sd_card_async.rs:21:1
  30: Reset
  (HOST) ERROR the program panicked

[elpiel]

To join in the discussion, I like the approach which some other crates take on blocking vs async api - the default module async api with a second blocking module. Maybe this would make more sense for the crate as well, but in the other way around? i.e. blocking api by default and async one when a feature is enabled. This would leave both options on the table for crate users.

@ninjasource I tried your branch but currently it fails for me because of #33 I believe but it does a panic instead of getting this error. I took a look at the code and it seems that embassy-rp requires for transfer that both read and write to be of equal length, although embedded_hal_async documentation states that they can be of different length - https://github.com/embassy-rs/embassy/issues/1181

0.000912 INFO  Init SD card...
└─ netbox_hardware::micro_sd::async_api::acquire_device::{async_fn#0} @ src/micro_sd.rs:103
0.001855 ERROR panicked at 'assertion failed: `(left == right)`'
diff < left / right >
<6
>1
└─ embassy_rp::spi::{impl#3}::transfer_inner::{async_fn#0} @ /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-rp/src/fmt.rs:24
0.001937 ERROR panicked at 'explicit panic', /home/elpiel/.cargo/registry/src/github.com-1ecc6299db9ec823/defmt-0.3.2/src/lib.rs:367:5
└─ panic_probe::print_defmt::print @ /home/elpiel/.cargo/registry/src/github.com-1ecc6299db9ec823/panic-probe-0.3.0/src/lib.rs:91
────────────────────────────────────────────────────────────────────────────────
stack backtrace:
   0: HardFaultTrampoline
      <exception entry>
   1: lib::inline::__udf
        at ./asm/inline.rs:181:5
   2: __udf
        at ./asm/lib.rs:51:17
   3: cortex_m::asm::udf
        at /home/elpiel/.cargo/registry/src/github.com-1ecc6299db9ec823/cortex-m-0.7.7/src/asm.rs:43:5
   4: rust_begin_unwind
        at /home/elpiel/.cargo/registry/src/github.com-1ecc6299db9ec823/panic-probe-0.3.0/src/lib.rs:72:9
   5: core::panicking::panic_fmt
        at /rustc/b7bc90fea3b441234a84b49fdafeb75815eebbab/library/core/src/panicking.rs:65:14
   6: core::panicking::panic
        at /rustc/b7bc90fea3b441234a84b49fdafeb75815eebbab/library/core/src/panicking.rs:114:5
   7: __defmt_default_panic
        at /home/elpiel/.cargo/registry/src/github.com-1ecc6299db9ec823/defmt-0.3.2/src/lib.rs:367:5
   8: defmt::export::panic
        at /home/elpiel/.cargo/registry/src/github.com-1ecc6299db9ec823/defmt-0.3.2/src/export/mod.rs:133:14
   9: embassy_rp::spi::Spi<T,embassy_rp::spi::Async>::transfer_inner::{{closure}}
        at /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-rp/src/spi.rs:388:9
  10: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
        at /rustc/b7bc90fea3b441234a84b49fdafeb75815eebbab/library/core/src/future/mod.rs:92:19
  11: embassy_rp::spi::Spi<T,embassy_rp::spi::Async>::transfer::{{closure}}
        at /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-rp/src/spi.rs:378:50
  12: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
        at /rustc/b7bc90fea3b441234a84b49fdafeb75815eebbab/library/core/src/future/mod.rs:92:19
  13: embassy_rp::spi::eha::<impl embedded_hal_async::spi::SpiBus for embassy_rp::spi::Spi<T,embassy_rp::spi::Async>>::transfer::{{closure}}
        at /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-rp/src/spi.rs:589:39
  14: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
        at /rustc/b7bc90fea3b441234a84b49fdafeb75815eebbab/library/core/src/future/mod.rs:92:19
  15: embedded_sdmmc_async::sdmmc::SdMmcSpi<SPI,CS>::card_command::{{closure}}
        at /mnt/Disk/Projects/lechev.space/embedded-sdmmc-async/embedded-sdmmc-async/src/sdmmc.rs:320:13
  16: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
        at /rustc/b7bc90fea3b441234a84b49fdafeb75815eebbab/library/core/src/future/mod.rs:92:19
  17: $t.148
  18: embassy_executor::raw::TaskStorage<F>::poll
  19: core::cell::Cell<T>::get
        at /rustc/b7bc90fea3b441234a84b49fdafeb75815eebbab/library/core/src/cell.rs:452:18
  20: embassy_executor::raw::timer_queue::TimerQueue::update
        at /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-executor/src/raw/timer_queue.rs:36:12
  21: embassy_executor::raw::Executor::poll::{{closure}}
        at /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-executor/src/raw/mod.rs:388:17
  22: embassy_executor::raw::run_queue::RunQueue::dequeue_all
        at /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-executor/src/raw/run_queue.rs:69:13
  23: core::cell::Cell<T>::get
        at /rustc/b7bc90fea3b441234a84b49fdafeb75815eebbab/library/core/src/cell.rs:452:18
  24: embassy_executor::raw::timer_queue::TimerQueue::retain
        at /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-executor/src/raw/timer_queue.rs:72:16
  25: embassy_executor::raw::timer_queue::TimerQueue::next_expiration
        at /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-executor/src/raw/timer_queue.rs:49:9
  26: embassy_executor::raw::Executor::poll
        at /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-executor/src/raw/mod.rs:395:39
  27: embassy_executor::arch::Executor::run
        at /home/elpiel/.cargo/git/checkouts/embassy-9312dcb0ed774b29/aea5a0f/embassy-executor/src/arch/cortex_m.rs:55:17
  28: write_to_sd_card_async::__cortex_m_rt_main
        at src/bin/write_to_sd_card_async.rs:21:1
  29: main
        at src/bin/write_to_sd_card_async.rs:21:1
  30: Reset
  (HOST) ERROR the program panicked

[elpiel]

So after applying a fix to the branch to mitigate the wrong impl in embassy-rp I managed to make it work.

It did work for a while although much slower I must add that the blocking api but I will do more research on this part.

In my latest test, after ~ 1200 lines written to a single file I got a panic from embassy and I have yet to look into it or what caused it.

RTT error: Error communicating with probe: A core architecture specific error occurred
────────────────────────────────────────────────────────────────────────────────
Error: A core architecture specific error occurred

Caused by:
    0: Failed to read register DRW at address 0x0000000c
    1: An error specific to the selected architecture occurred
    2: Target device did not respond to request.

[elpiel]

I've steered away from the async version and we might use the littlefs2 crate instead.

However, I wanted to give a quick update: Writing works and the error I've encountered before is related to the debug probe rather than the crate itself.

[ninjasource]

Hi @elpiel, I'm glad you managed to figure out your error. My apologies for the silence but I don't have any use case for writing to the SD card so I couldn't reproduce your issue without doing a whole bunch of work. I personally think that using the generics keyword feature would be a better approach to having separate async and blocking interfaces due to all the duplicate that would entail. It's a long way off though. https://blog.rust-lang.org/inside-rust/2022/07/27/keyword-generics.html

[thejpster]

So Henrik did this already: https://github.com/kalkyl/sdmmc-embassy

[elpiel]

@thejpster this is a very old remote that you've shared and embassy doesn't have this traits crate anymore. The API should be based on embedded-hal-async IMO and be part of this crate. I don't see much benefit in splitting blocking and async API and fixing bugs on both at the same time.

@ninjasource has the most up-to-date branch that I amanged to test and it was much slower that the blocking version.

It can be found here https://github.com/ninjasource/embedded-sdmmc-rs/tree/add-async-support

The issues that I have encountered were/are realted to:

1. Bug that was fixed in SPI impl of embassy-rp
2. Probe software

[thejpster]

You will always have to fix bugs in both because the blocking and async APIs are fundamentally incompatible due to how Rust is currently designed.

The link was given to me by the author yesterday. If it requires updates you are welcome to reach out to the author.

[ninjasource]

Hi @elpiel,

@ninjasource has the most up-to-date branch that I amanged to test and it was much slower that the blocking version.

I actually thought the async version was faster but if its slower then I highly recommend you use the blocking version as the embedded-hal traits it uses are far more stable. I made the fork and async changes to solve a specific problem I was working on and thought I'd keep it up there for others to use.

Btw, the only reason for the performance difference is that the async version uses DMA so bulk data transfers are handed over to a dedicated controller, freeing up cpu to do other stuff. If you re experiencing problems with the async version you are welcome to post them as issues on my branch and I'll take a look. Async and embassy are constantly changing so this is not a stable space I'm afraid. Will get there eventually!

[peterkrull]

For anyone who stumbles across this issue in search for async, I have an up to date (at the time of writing) branch which supports async: https://github.com/peterkrull/embedded-sdmmc-rs/tree/develop-async

One difference to note is that File handles are not automatically closed on drop, since that would require the drop impl to be async. Instead that must be done manually. It also has experimental support for multi-block writes through const-generics.

[ilya-epifanov]

I'll just leave this here: https://github.com/fMeow/maybe-async-rs So only the backend bus implementations will need to be made separately, the rest of the code could be async but downgraded to blocking with a feature flag.

[elpiel]

I've been working on separating the SDCard implementation to 2 separate impls using a known pattern in Rust. Ofc this basically duplicates all the code and slaps async/await everywhere but the next step would be to remove the duplication using maybe-async or a similar crate for this.

You can see how the API looks like in this branch: https://github.com/LechevSpace/embedded-sdmmc-rs/pull/new/feat/async-feature

Additions or ideas for the implementation, feature name and how to use maybe-async to remove duplication are welcome!

PS: I personally would like to see the SdCard struct worked on first to figure out how to do it for the rest of the interfaces (like fat) as well.

[elpiel]

@thejpster I know we've discussed this multiple times but could you take a look at the branch I shared in my previous comment?

If you are ok with the API I can take a look at some of the crates to remove the duplication of code and allow both async and blocking impl to be in single implementation and enable one or the other with a feature.

The tl;dr: is that for blocking, the macros can remove all the async/await keywords.

[thejpster]

OK so it looks like that branch brings turns on some unstable feature gates when the asynch feature it set, and turns the asynch feature on by default. I'd like to know more about why those gates are required, and how that affects use on stable Rust.

Looking at fn num_bytes() as an example I see both outer and inner impls, for both async and sync Mode. So effectively the function is duplicated - one with .await sprinkled through it, and one without. I have no real opinion on the async API because I don't need the async API and I haven't used many other Rust async APIs in the past and so I don't know what would work well and what wouldn't. What I can say right now is that my appetite for merging a PR that basically duplicates all the code is pretty low.

Perhaps there are other Rust embedded async API experts who would weigh in on whether this is a good approach or not.

[elpiel]

I understand.

I wanted to show you the Public API first and then work on removing duplicates, removing nightly features and merging async and sync impls with a macro.

It's still a wip impl in the works

[thejpster]

People keep asking, and the maybe_async approach is plausible so I'm re-opening this issue.

I won't be writing the PR, but I'm not against merging the PR as long as it doesn't make the codebase significantly more complex to understand.

[elpiel]

@thejpster I did update my branch with async support to the latest 0.8 version:

https://github.com/LechevSpace/embedded-sdmmc-rs/pull/new/feat/async-feature-0.8

Notes on nightly features:

• Nightly feature for async_fn_in_trait can be removed if we set an MSRV on the crate to Rust 1.75
• Nightly feature for async_closure is used for taking a mutable reference to the underlying SPI device pub async fn spi<T, F>(&self, func: F) -> T but I believe we can remove this from the async api?! Maybe we can use the same approach with the initialization of the card and just leave the user of the crate handle shared busses? e.g. using https://docs.embassy.dev/embassy-embedded-hal/git/default/shared_bus/asynch/spi/index.html