Add embedded-hal-async support
#7
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Please also see my pull request which added support for embedded-hal-1.0 and async support gated behind a feature: #5 |
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
Now that my changes have been published upstream, we can remove the Git patch. This PR should now be okay to merge!
This way, the naming scheme is more consistent with the async driver I added to the `sgp30` crate. See: dbrgn/sgp30-rs@cb769b0
It's about the error and not the I2C implementation type.
With the driver struct already named Sht4xAsync, its source file should follow the same order.
Sensor data extraction is already shared and the serial number extraction code should be shared as well.
It's written in the sensors data sheet but I did not remember this after looking into the code after quite some time.
sirhcel
force-pushed
the
eliza/embedded-hal-async
branch
from
8e2519a to
40cef0d
Compare
|
Thank you very much for your PR @hawkw! I took me a while to wrap my head around how to provide a sync and async driver variant from a single crate. I wished that more code could be shared but this seems not conveniently possible as of now. |
Yeah, I also wished that, but unfortunately the story for this sort of thing is still a bit rough. Thanks for taking the time to merge my PR! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit adds support for the
embedded-hal-asynccrate in additionto
embedded-hal. I've done this by adding a separateAsyncSht4xtype, based on the assumption that most projects won't need to use both
the blocking
embedded-haltraits and theembedded-hal-asynctraitsat the same time, and providing
async fnmethods on a separate typewith the same names as the blocking ones seemed a bit nicer than having
one type that has both
fn measureandasync fn measure_asyncand soon.
Support for
embedded-hal-asyncis gated behind theembedded-hal-asyncfeature flag, so the dependency is not enabled bydefault.
Note that this branch depends on my PR #6, which updates this crate to
use
embedded-halv1.0, and currently contains the commit from thatchange as well. Once #6 has merged, this branch will need to be rebased
onto the main branch.
It also depends on my upstream PR adding
embedded-hal-asyncsupport tosensirion-i2c-rs, Sensirion/sensirion-i2c-rs#30, which has beenmerged, but hasn't been published to crates.io yet. Currently, this
branch adds a Cargo
[patch]to use a Git dep onsensirion-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! :)