-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for embedded-hal-async
#30
Conversation
Now that the `embedded-hal-async` crate has released version 1.0, it would be nice to be able to use the async versions of `embedded-hal`'s traits for writing drivers for Sensirion I2C devices. This commit adds a new `i2c_async` module which provides versions of the APIs in the `i2c` module that use `embedded-hal-async`'s async `I2c` trait rather than `embedded-hal`'s blocking `I2c' trait. The new module is feature-flagged, so that users who don't need `embedded-hal-async` support can avoid the additional dependency, and it's off by default. Ideally, it would be nice if the blocking `embedded-hal` APIs could also be feature-flagged, so that async-only users don't have to pull them in, but that would be a breaking change. I've also taken the liberty of adding support for the `doc_cfg` and `doc_auto_cfg` RustDoc features when building on docs.rs, so that the documentation displays that the new module is feature-flagged. I've changed the `Error` type to require that `I: embedded_hal::i2c::ErrorType` rather than `I: I2c`, so that errors returned by `embedded_hal_async::i2c::I2c` can also be used here. This shouldn't be a breaking change, as far as I can tell, because `I2c` has a `ErrorType` bound, and all types that implement `I2c` must also implement `ErrorType` --- all the same types should still be valid here. Closes Sensirion#24
Now that the `embedded-hal-async` crate has released version 1.0, it would be nice to be able to use it with this crate. Therefore, this commit adds feature-flagged support for `embedded-hal-async`, in addition to the blocking `embedded-hal`. The `embedded-hal-async` feature flag enables a new `AsyncScd4x` type, which is identical to the existing `Scd4x` struct, except that it uses the `embedded_hal_async` versions of the `I2c` and `DelayNs` traits. ## Notes I decided to implement this as a new struct, rather than by adding async methods to the existing `Scd4x` type, because it seemed kind of unfortunate to have to add a `_async` suffix (or something similar) to the names of the async methods, which would be necessary if they were defined on the same type as the blocking methods. I expect that typically, most projects will be using either the blocking interface *or* the async interface, so it's probably not really necessary to provide one type that can do both...but, if the maintainers of this crate disagree, I'm happy to change it! I've also refactored the existing code a bit to factor out the parts of the `Scd4x` type that don't actually perform I2C operations or delays, so that we can reuse the same functions for encoding and decoding I2C messages from the sensor. This way, both the blocking and async drivers depend on the same functions, so that future changes to this code doesn't have to update the same code twice. Also, please note that this PR isn't actually ready to merge yet: I've had to make an upstream change to the `sensirion-i2c` crate to add `embedded-hal-async` support there, as well (Sensirion/sensirion-i2c-rs#30), so this PR currently has a Git dependency on my fork of that crate. Once the upstream changes merge, I'll update this PR to depend on the released version of `sensirion-i2c` instead, so we can remove the Git dependency. Until then, though, we should probably wait until upstream changes merge before merging this PR. Closes hauju#12
Thank you very much for your contribution |
This commit adds support for the `embedded-hal-async` crate in addition to `embedded-hal`. I've done this by adding a separate `AsyncSgp30` type, based on the assumption that most projects won't need to use both the blocking `embedded-hal` traits and the `embedded-hal-async` traits at the same time, and providing `async fn` methods on a separate type with the same names as the blocking ones seemed a bit nicer than having one type that has both `fn measure` and `async fn measure_async` and so on. I've also factored out some of the no-IO code for packing and unpacking Rust to/from bytes, so that it can be shared by both the async and blocking driver types. Support for `embedded-hal-async` is gated behind the `embedded-hal-async` feature flag, so the dependency is not enabled by default. Note that this branch depends on my PR dbrgn#18, which updates this crate to use `embedded-hal` v1.0. It also depends on my upstream PR adding `embedded-hal-async` support to `sensirion-i2c-rs`, Sensirion/sensirion-i2c-rs#30, which has been [merged], but hasn't been published to crates.io yet. Currently, this branch adds a Cargo `[patch]` to use a Git dep on `sensirion-i2c-rs`. So, this change cannot be released to crates.io until upstream publishes a new release of `sensirion-i2c-rs`. Hopefully they do that soon! :) [merged]: Sensirion/sensirion-i2c-rs@f7b9f3a
This commit adds support for the `embedded-hal-async` crate in addition to `embedded-hal`. I've done this by adding a separate `AsyncSht4x` type, based on the assumption that most projects won't need to use both the blocking `embedded-hal` traits and the `embedded-hal-async` traits at the same time, and providing `async fn` methods on a separate type with the same names as the blocking ones seemed a bit nicer than having one type that has both `fn measure` and `async fn measure_async` and so on. Support for `embedded-hal-async` is gated behind the `embedded-hal-async` feature flag, so the dependency is not enabled by default. Note that this branch depends on my PR sirhcel#6, which updates this crate to use `embedded-hal` v1.0, and currently contains the commit from that change as well. Once sirhcel#6 has merged, this branch will need to be rebased onto the main branch. It also depends on my upstream PR adding `embedded-hal-async` support to `sensirion-i2c-rs`, Sensirion/sensirion-i2c-rs#30, which has been [merged], but hasn't been published to crates.io yet. Currently, this branch adds a Cargo `[patch]` to use a Git dep on `sensirion-i2c-rs`. So, this change cannot be released to crates.io until upstream publishes a new release of `sensirion-i2c-rs`. Hopefully they do that soon! :) [merged]: Sensirion/sensirion-i2c-rs@f7b9f3a
This commit adds support for the `embedded-hal-async` crate in addition to `embedded-hal`. I've done this by adding a separate `AsyncSgp30` type, based on the assumption that most projects won't need to use both the blocking `embedded-hal` traits and the `embedded-hal-async` traits at the same time, and providing `async fn` methods on a separate type with the same names as the blocking ones seemed a bit nicer than having one type that has both `fn measure` and `async fn measure_async` and so on. I've also factored out some of the no-IO code for packing and unpacking Rust to/from bytes, so that it can be shared by both the async and blocking driver types. Support for `embedded-hal-async` is gated behind the `embedded-hal-async` feature flag, so the dependency is not enabled by default. Note that this branch depends on my PR dbrgn#18, which updates this crate to use `embedded-hal` v1.0. It also depends on my upstream PR adding `embedded-hal-async` support to `sensirion-i2c-rs`, Sensirion/sensirion-i2c-rs#30, which has been [merged], but hasn't been published to crates.io yet. Currently, this branch adds a Cargo `[patch]` to use a Git dep on `sensirion-i2c-rs`. So, this change cannot be released to crates.io until upstream publishes a new release of `sensirion-i2c-rs`. Hopefully they do that soon! :) [merged]: Sensirion/sensirion-i2c-rs@f7b9f3a
Hey @psachs, thanks for maintaining this crate! I noticed that this PR was merged, but no release was pushed to crates.io so far. Would you mind doing such a release? That would allow async support for the SGP30 to get merged: dbrgn/sgp30-rs#19 |
@dbrgn I just release v0.4.0 with the async support. You should now be able to merge the sgp30 async support as well |
@psachs great, thanks! |
This commit adds support for the `embedded-hal-async` crate in addition to `embedded-hal`. I've done this by adding a separate `AsyncSgp30` type, based on the assumption that most projects won't need to use both the blocking `embedded-hal` traits and the `embedded-hal-async` traits at the same time, and providing `async fn` methods on a separate type with the same names as the blocking ones seemed a bit nicer than having one type that has both `fn measure` and `async fn measure_async` and so on. I've also factored out some of the no-IO code for packing and unpacking Rust to/from bytes, so that it can be shared by both the async and blocking driver types. Support for `embedded-hal-async` is gated behind the `embedded-hal-async` feature flag, so the dependency is not enabled by default. Note that this branch depends on my PR dbrgn#18, which updates this crate to use `embedded-hal` v1.0. It also depends on my upstream PR adding `embedded-hal-async` support to `sensirion-i2c-rs`, Sensirion/sensirion-i2c-rs#30, which has been [merged], but hasn't been published to crates.io yet. Currently, this branch adds a Cargo `[patch]` to use a Git dep on `sensirion-i2c-rs`. So, this change cannot be released to crates.io until upstream publishes a new release of `sensirion-i2c-rs`. Hopefully they do that soon! :) [merged]: Sensirion/sensirion-i2c-rs@f7b9f3a
This commit adds support for the `embedded-hal-async` crate in addition to `embedded-hal`. I've done this by adding a separate `AsyncSgp30` type, based on the assumption that most projects won't need to use both the blocking `embedded-hal` traits and the `embedded-hal-async` traits at the same time, and providing `async fn` methods on a separate type with the same names as the blocking ones seemed a bit nicer than having one type that has both `fn measure` and `async fn measure_async` and so on. I've also factored out some of the no-IO code for packing and unpacking Rust to/from bytes, so that it can be shared by both the async and blocking driver types. Support for `embedded-hal-async` is gated behind the `embedded-hal-async` feature flag, so the dependency is not enabled by default. Note that this branch depends on my PR dbrgn#18, which updates this crate to use `embedded-hal` v1.0. It also depends on my upstream PR adding `embedded-hal-async` support to `sensirion-i2c-rs`, Sensirion/sensirion-i2c-rs#30, which has been [merged], but hasn't been published to crates.io yet. Currently, this branch adds a Cargo `[patch]` to use a Git dep on `sensirion-i2c-rs`. So, this change cannot be released to crates.io until upstream publishes a new release of `sensirion-i2c-rs`. Hopefully they do that soon! :) [merged]: Sensirion/sensirion-i2c-rs@f7b9f3a
Now that the
embedded-hal-async
crate has released version 1.0, it would be nice to be able to use the async versions ofembedded-hal
's traits for writing drivers for Sensirion I2C devices. This commit adds a newi2c_async
module which provides versions of the APIs in thei2c
module that useembedded-hal-async
's asyncI2c
trait rather thanembedded-hal
's blockingI2c
trait.The new module is feature-flagged, so that users who don't need
embedded-hal-async
support can avoid the additional dependency, and it's off by default. Ideally, it would be nice if the blockingembedded-hal
APIs could also be feature-flagged, so that async-only users don't have to pull them in, but that would be a breaking change. I've also taken the liberty of adding support for thedoc_cfg
anddoc_auto_cfg
RustDoc features when building on docs.rs, so that the documentation displays that the new module is feature-flagged.Closes #24