Skip to content
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

Make the async SpiDevice trait unsafe to implement #403

Merged
merged 1 commit into from
Sep 4, 2022

Conversation

GrantM11235
Copy link
Contributor

This is necessary due to the raw pointer workaround.

@GrantM11235 GrantM11235 requested a review from a team as a code owner August 29, 2022 20:43
@rust-highfive
Copy link

r? @ryankurte

(rust-highfive has picked a reviewer for you, use r? to override)

Copy link
Member

@Dirbaio Dirbaio left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

It's very unfortunate unsafe trait is needed, but it's the correct thing to do, so 👍 . Everything else about the raw pointer workaround is unfortuate anyway... 🤷

bors r+

@bors bors bot merged commit 1f87498 into rust-embedded:master Sep 4, 2022
@GrantM11235 GrantM11235 deleted the unsafe-async-spidevice branch September 4, 2022 21:58
bors bot added a commit that referenced this pull request Sep 21, 2022
404: async/spi: Add a transaction helper macro r=Dirbaio a=GrantM11235

Based on #403

This macro allows users to use `SpiDevice::transaction` in a completely safe way, without needing to dereference a raw pointer.

The macro is exported as `embedded_hal_async::spi_transaction_helper` because exported macros always go in the root of the crate.
It is also publicly reexported as `embedded_hal_async::spi::transaction_helper` because I think that most people would expect to find it in the spi module.
It is not possible to apply `doc(hidden)` to the instance in the root of the crate because that would also hide the reexport, see rust-lang/rust#59368.

Co-authored-by: Grant Miller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Review is incomplete T-hal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants