Support latest version of embedded-sensors-hal & embedded-sensors-hal-async #32
Conversation
douglascgh
requested review from
benonivainqueur,
Copilot,
felipebalbi and
kurtjd
| #[cfg(all(feature = "embedded-sensors-hal", not(feature = "async")))] | ||
| impl<I2C: embedded_hal::i2c::I2c> embedded_sensors_hal::temperature::TemperatureThresholdSet for Tmp108<I2C> { | ||
| fn set_temperature_threshold_low( | ||
| &mut self, | ||
| threshold: embedded_sensors_hal::temperature::DegreesCelsius, | ||
| ) -> Result<(), Self::Error> { | ||
| self.set_low_limit(threshold).map_err(Error::Bus) | ||
| } | ||
|
|
||
| fn set_temperature_threshold_high( | ||
| &mut self, | ||
| threshold: embedded_sensors_hal::temperature::DegreesCelsius, | ||
| ) -> Result<(), Self::Error> { | ||
| self.set_high_limit(threshold).map_err(Error::Bus) | ||
| } | ||
| } |
There was a problem hiding this comment.
It doesn't really make sense to have threshold set on blocking. Threshold is used to assert the interrupt line on the GPIO, will you dedicate a task to block polling for that GPIO to be asserted? Not sure that's a good idea. Glad to hear your thoughts.
There was a problem hiding this comment.
I agree, especially looking into how our current thermal service is implemented and in Embassy in general. This is more meant for an environment that does not support async but still has the ability to adopt a rust based driver. In that environment the GPIO assert is initially handled outside of the driver and eventually routed to an application/service. So adding blocking function is not useful for ODP service but would be useful for a non async rtos. Hoping this allows tmp108 to be adopted as is.
There was a problem hiding this comment.
Pull request overview
This pull request updates the embedded-sensors-hal and embedded-sensors-hal-async dependencies to support newer trait versions that include threshold and hysteresis configuration interfaces. The PR adds blocking trait implementations that mirror the existing async implementations and includes comprehensive tests for both.
Changes:
- Updates
embedded-sensors-halfrom 0.1.0 to 0.1.1 andembedded-sensors-hal-asyncfrom 0.3.0 to 0.4.0 - Implements
TemperatureThresholdSetandTemperatureHysteresistraits for the blocking (non-async) version of the driver - Adds blocking tests for threshold and hysteresis settings, and an async test for hysteresis settings
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Cargo.toml | Updated dependency versions for embedded-sensors-hal (0.1.0 → 0.1.1) and embedded-sensors-hal-async (0.3.0 → 0.4.0) |
| Cargo.lock | Updated dependency versions and checksums, added paste dependency for embedded-sensors-hal |
| src/lib.rs | Added blocking trait implementations for TemperatureThresholdSet and TemperatureHysteresis, plus comprehensive tests for both blocking and async versions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2 participants
Adopt the embedded-sensors-hal version 0.1.1 and embedded-sensors-hal-async 0.4.0. The async version remains the same but the blocking version not has similar threshold and hysteresis traits. The tmp108 driver now provides support for blocking trait as well.
The PR also port over existing async threshold test to a blocking version. Additional test for hysteresis is also added.