hauju / scd4x-rs

Rust driver for the Sensirion SCD4x sensors.
Apache License 2.0
5 stars 12 forks source link

Add `embedded-hal-async` support #13

Open hawkw opened 5 months ago

hawkw commented 5 months ago

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 #12

hauju commented 3 months ago

Hi, thx for you contribution :) Will add an example after merge. Currently the pipeline fails due to misconfiguration in the features section.