Add maybe_async

[brunob45]

I'm using embassy, and the other repo I found (embedded-fatfs) is not supporting partition tables on the SD card.
Here is my proposal to add maybe_async to this repo, so it can be used "as-is", or asynchronously.

I also needed to make BlockDevice mutable to support DMA on STM32.

All comments are welcomed.

[brunob45]

Related to #50

[thejpster]

It might be worth looking at #148 where I put a BlockCache in a refcell. That might help you have a mutable block device without making all the &self methods back to &mut self (which forces you to use raw handles if you want to open multiple files at once).

[thejpster]

So for this to fly, I would need the examples to build-as is, so that we know we're not breaking existing code (at least more than I have already broken it). But perhaps also an async example would be useful. And it has to build on stable.

[brunob45]

That's fair, I'll work on that

Otherwise, I know there is a lot of debate around maybe_async, and I'd understand if this PR is not the way forward for this library. What's your thoughts on this?

[thejpster]

I'll judge it by the outcome. If the examples build unchanged on stable (and it works on no_std) then it should continue to work for my use case and I can support the changes.

[thejpster]

#148 was merged, so this will need a rebase.

[brunob45]

@thejpster all fixed! rebase done, working on github workflow

[brunob45]

Run workflow? build-test is successful, formatting is creating lots of diffs

[thejpster]

We'll want to find a way to do a build in both async mode and non-async-mode. I think you'll need to mark the existing examples as requiring the is_sync feature, and make a new async example which only builds when that isn't set? Actually I don't even know if that's possible. The async example might have to be in a crate all of its own.

[thejpster]

Run workflow?

As this PR is not from an org member, one of us has to manually approve the workflow run every time - in case you submit a change which copies a Github secret key to pastebin or something. Sorry about that - bear with us. Alternatively you can look at the workflow files and get a good idea about what you need to run locally to replicate the CI.

[thejpster]

formatting is creating lots of diffs

We just test for cargo fmt --check so you probably just need to run cargo fmt locally.

[thejpster]

Expected — Waiting for status to be reported

unfortunately it looks like adding the features to the matrix has renamed the build jobs, and so now the branch protection rule is waiting for a job that doesn't exist. I guess I'll have to manually override that check when this is ready to go, and then update the branch protection rule.

GHA is such a pain to use.

[brunob45]

Thanks for the information! For a moment I thought the workflows were triggered by commenting here...

I'll try to put the is_sync feature outside the matrix, that should resolve the renaming of workflow

I just pushed the result of cargo fmt

[brunob45]

Next I'll work on async examples

[brunob45]

Oh, I ran cargo fmt on windows... Is it different than Linux?

[thejpster]

Try using 1.81. Maybe GitHub hasn't updated the image yet.

[brunob45]

I had a rustfmt.toml file in the parent directory, limiting the line length to 120. I removed it and it fixed the issue

[ninjasource]

We'll want to find a way to do a build in both async mode and non-async-mode. I think you'll need to mark the existing examples as requiring the is_sync feature, and make a new async example which only builds when that isn't set? Actually I don't even know if that's possible. The async example might have to be in a crate all of its own.

IMO, this is the biggest issue with using the maybe_async crate. It breaks cargo's rules about features being additive. I don't think that it would be inconceivable for a user to need both blocking and non-blocking or one crate to use blocking (say for log flushing which may have a blocking api) and another non-blocking. Ideally the macro would generate separately named functions so that crates can play nicely with each other but alas it doesn't.

[ivmarkov]

We'll want to find a way to do a build in both async mode and non-async-mode. I think you'll need to mark the existing examples as requiring the is_sync feature, and make a new async example which only builds when that isn't set? Actually I don't even know if that's possible. The async example might have to be in a crate all of its own.

IMO, this is the biggest issue with using the maybe_async crate. It breaks cargo's rules about features being additive. I don't think that it would be inconceivable for a user to need both blocking and non-blocking or one crate to use blocking (say for log flushing which may have a blocking api) and another non-blocking. Ideally the macro would generate separately named functions so that crates can play nicely with each other but alas it doesn't.

I've found a workaround in simply not using the is-sync feature of the maybe-async crate at all, and then also using the duplicate crate which allows you to re-export a single, templatized module twice:

• once, as blocking
• one more time, as async

... so you are effectively ending up simultaneously with two versions of the code, each in its submodule. Modulo the two submodules which of course have different names, the lib otherwise is having a mostly identical api for blocking and async.

A bit like having embedded-hal and embedded-hal-async in the same crate under two different modules.

Note that I've only tried this with smaller drivers where I could fit all "maybe-async" code in a single module. Not sure how it would work for a larger codebase split across many modules.

[ivmarkov]

Here's the relevant comment in the maybe-async crate that I follow myself: fMeow/maybe-async-rs#6 (comment)

[elpiel]

Thank you very much for working on this @brunob45 .
I wonder if you can also tweak the SdCard impl as I personally want to use the raw read/write support instead of the fat32 one.