-
Notifications
You must be signed in to change notification settings - Fork 4
Implement blocking embedded-io traits #10
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
base: master
Are you sure you want to change the base?
Conversation
…tructs available on crate root.
This commit makes embedded-io-async an optional dependency and creates a feature ``async` that enables it. It also implements the blocking Read/BufRead/Write traits from embedded-io and creates corresponding tests.
|
@rmja +1 for this. Do you have any appetite for these changes? I would love to use something like this for my port of the |
|
Is the recommendation nowadays to split the crates in two (like embedded-io or embedded-io-async), or is it to separate on features? |
|
From my experience, there is still a pretty even split. Having them together can create churn if one side has a lot more changes than the other; but conversely, having them split may mean that consumers have to rely on both depending on their in downstream users. I think, given that you have the async traits, it would be better to have both here and enable the async versions as a feature. Publishing a |
|
It is currently being investigated whether this should be included in |
|
Are you referring to rust-embedded/embedded-hal#627 ? That seems to be dead, but if there is discussion elsewhere I can jump in there |
|
I didn't realize it had already been discussed. Seems like maybe splitting into buffered-io and buffered-io-async would be the best move? |
|
I think I agree. Please feel free to update this PR or create a new, then I will make sure that it gets merged and published. |
This PR implements the blocking traits from embedded-io. It refactors out the
asynchmodule and makes the asynch traits available by a feature flag instead. Feel free to merge the PR, but it would require a breaking change.