Skip to content

Conversation

@douglascgh
Copy link
Contributor

Certain functionalities are only defined as async traits and not available in blocking. Expanding the relevant function so the macro also generates blocking version so it can be adopted to an environment where async support is not available such as driver running in Zephyr. The macro that generates async function has been expanded to also generate blocking function for traits that is relevant for both blocking and async.

The patch also expands the current set to cover the newly added blocking traits.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request adds blocking versions of threshold-related traits that were previously only available in async form, enabling use in environments without async support (such as Zephyr). The implementation consolidates the trait generation logic into a unified macro that supports both blocking and async modes.

Changes:

  • Added a new decl_threshold_traits! macro in the blocking crate that generates both blocking and async threshold traits based on a mode parameter
  • Added blocking ThresholdSet and Hysteresis traits for Temperature and RelativeHumidity sensors with comprehensive test coverage
  • Refactored the async crate to re-export and use the unified macro from the blocking crate
  • Fixed spelling errors in error messages and documentation

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
embedded-sensors/src/sensor.rs Added unified macro for generating threshold traits, fixed spelling errors in error messages
embedded-sensors/src/temperature.rs Added blocking threshold traits using the new macro, implemented comprehensive tests
embedded-sensors/src/humidity.rs Added blocking threshold traits using the new macro, implemented comprehensive tests
embedded-sensors/Cargo.toml Added paste dependency required by the macro
embedded-sensors-async/src/sensor.rs Removed duplicate macro, now re-exports from blocking crate, fixed spelling error
embedded-sensors-async/src/temperature.rs Updated to use unified macro with async mode parameter, consolidated imports
embedded-sensors-async/src/humidity.rs Updated to use unified macro with async mode parameter, consolidated imports
Cargo.lock Updated with paste dependency

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

kurtjd
kurtjd previously approved these changes Jan 28, 2026
@jerrysxie jerrysxie added the enhancement New feature or request label Jan 28, 2026
Copy link
Contributor

@jerrysxie jerrysxie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kurtjd @douglascgh semver failed and this is a breaking change. We need to bump the version and announce the breaking changes on Zulip: https://opendevicepartnership.zulipchat.com/#narrow/channel/495405-embedded-controller

@douglascgh
Copy link
Contributor Author

@kurtjd @douglascgh semver failed and this is a breaking change. We need to bump the version and announce the breaking changes on Zulip: https://opendevicepartnership.zulipchat.com/#narrow/channel/495405-embedded-controller

It does a require a version update due to the macro changes but it technically isn't a breaking change as the public trait remains the same and only additional trait added. I would still make the announcement so it is obvious. @kurtjd did mentioned some enhancement that he is thinking about as a follow up to this and I expect those to be breaking.

@kurtjd
Copy link
Contributor

kurtjd commented Jan 28, 2026

@kurtjd @douglascgh semver failed and this is a breaking change. We need to bump the version and announce the breaking changes on Zulip: https://opendevicepartnership.zulipchat.com/#narrow/channel/495405-embedded-controller

It does a require a version update due to the macro changes but it technically isn't a breaking change as the public trait remains the same and only additional trait added. I would still make the announcement so it is obvious. @kurtjd did mentioned some enhancement that he is thinking about as a follow up to this and I expect those to be breaking.

Yes the plan is to rework the design in the near future, but will still publish this to crates.io in the meantime.

@kurtjd kurtjd merged commit 230c581 into OpenDevicePartnership:main Jan 28, 2026
10 checks passed
@douglascgh douglascgh deleted the topic/add-set-function-blocking-version-consolidate-with-test branch January 28, 2026 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BREAKING CHANGE enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants