Add support for embedded-hal-async traits

[quentinmit]

The rust embedded community is moving to a set of async traits for i2c. I implemented this as a pair of features, nb and async, to choose the type of desired futures. I thought about trying to use macros to generate both flavors of code, but I decided it was better to just duplicate the files for now. Presumably at some point in the future the nb version will just become deprecated.

Note that embassy provides an adapter to convert a blocking I2c into an async I2c, so this new async version can also be used with all the other legacy hals that don't provide the async traits.

I also noticed that the crate doesn't pass cargo test on non-Linux because it tries to build the example and linux-embedded-hal doesn't build on non-Linux. I couldn't figure out how to tell cargo that it shouldn't try to build the example on non-Linux platforms, but this PR doesn't make that any better or worse.

[quentinmit]

It looks like you're running CI tests on a version of Cargo from 2018 that doesn't support dep:* features. I just pushed a change to remove that and go back to using overloaded feature names.

[eldruin]

This is interesting, thanks!
A few thoughts, mostly from the perspective of an user:

• One thing that struck me as misleading is that you need to use the "nb" (non-blocking) feature, yet all configuration methods are blocking. I do not know how to improve this, though.
• Another thing is that building without the nb feature makes this driver unusable, although there are plenty of blocking methods. Again, I could not think of a way to improve this.
• The async version would need some examples. Maybe with some simple executor.
• The CI would need to be extended to test the async feature.
• Could you fix the formatting?

[quentinmit]

This is interesting, thanks! A few thoughts, mostly from the perspective of an user:

• One thing that struck me as misleading is that you need to use the "nb" (non-blocking) feature, yet all configuration methods are blocking. I do not know how to improve this, though.

AIUI the way to interpret this is not "non-blocking feature", it's the "use the nb crate feature". It's a pretty common convention that feature names should match the name of the dependency they add, if possible.

• Another thing is that building without the nb feature makes this driver unusable, although there are plenty of blocking methods. Again, I could not think of a way to improve this.

That's true; I made nb default so users wouldn't need to necessarily ever interact with it. It's also not great that the driver doesn't build with both features on.

• The async version would need some examples. Maybe with some simple executor.

Yeah, it's tough until linux_embedded_hal supports the async traits. I could write something that uses embassy's adapter, I suppose.

• The CI would need to be extended to test the async feature.

Certainly not happening on a 2018 Rust :)

• Could you fix the formatting?

Done.

[eldruin]

Thanks!
I would be fine to upgrade the MSRV for this, then we can test this on CI.
Documentation would need to be added about these features.
About the examples, they can be incomplete. Something for the docs that compiles but it does not necessarily run.

As a TODO list so that we do not forget anything:

• [ ] Add tests
• Upgrade MSRV, test feature on CI
• Add documentation of features to lib.rs and a brief reference in the README.
• Examples
• Changelog

[quentinmit]

Thanks! I would be fine to upgrade the MSRV for this, then we can test this on CI. Documentation would need to be added about these features. About the examples, they can be incomplete. Something for the docs that compiles but it does not necessarily run.

I bumped the MSRV to 1.56.0, so you can trigger another CI run which will hopefully pass.

I also started using the maybe-async-cfg crate, which provides a procedural macro to avoid needing a duplicate copy of all the code - I hope this makes the diff much smaller/easier to review. This also means that both features can be enabled side-by-side.

I added a single example to the top level of the library; since every function is exactly the same, hopefully that will be enough. Unfortunately neither linux-embedded-hal nor embedded-hal-mock support embedded-hal-async yet, so the example uses the embassy crate which targets real hardware.

[eldruin]

Great work!
Sorry it took me quite a while to go through the whole PR.
Could you also update the clippy version in CI and add a job building and running clippy with the async feature?

The CI fails on bare-metal targets because of linux-embedded-hal. For no-std targets I have a step in the CI where I comment out linux-embedded-hal, which is the reason for the weird error message. The build would fail nevertheless since this cannot be compiled there so another way to manage the dependencies would be necessary.
Now that we have a newer MSRV, using the "dep" namespace here would also be possible.

[eldruin]

linux-embedded-hal should only be a dev-dependency since it is only used for the examples

[quentinmit]

It's not possible to have an optional dev-dependency, so if I leave it there this crate is not buildable on macOS...

[eldruin]

We can do:

[target.'cfg(target_os = "linux")'.dev-dependencies]
linux-embedded-hal = "0.3"

The examples will still not build, though.

[quentinmit]

If I do that, there's no way to indicate that the example requires it. At least with it as an optional dependency cargo will avoid building the example if the feature isn't enabled.

[quentinmit]

Alright, the best I can come up with is to use cfg guards to make linux.rs into a noop.

[eldruin]

If requiring linux-embedded-hal, bare-metal targets will not build

[quentinmit]

See below.