async/spi: Add a transaction helper macro #404
Conversation
|
r? @ryankurte (rust-highfive has picked a reviewer for you, use r? to override) |
Clippy doesn't like it when you `drop` a reference
There was a problem hiding this comment.
I've updated a driver to use the macro, and it worked very nicely! HUGE improvement in ergonomics. https://github.com/embassy-rs/cyw43/pull/19/files
I've commented a few naming/doc nits, LGTM otherwise!
embedded-hal-async/src/spi.rs
Outdated
| @@ -35,13 +35,117 @@ where | |||
| Word: Copy + 'static, | |||
| = impl Future<Output = Result<(), T::Error>>; | |||
|
|
|||
| #[macro_export] | |||
| /// This macro is a workaround for the workaround in [`SpiDevice::transaction`] | |||
There was a problem hiding this comment.
Perhaps a more descriptive docs on why you might want to use it? Something like "Do an SPI transaction on a bus. This is a safe wrapper for [SpiDevice::transaction], which handles dereferencing the raw pointer for you."
embedded-hal-async/src/spi.rs
Outdated
| /// .await | ||
| /// # } | ||
| /// ``` | ||
| macro_rules! spi_transaction_helper { |
There was a problem hiding this comment.
I wonder if the name should be simply spi_transaction. I think users will end up using this 100% of the time instead of the unsafe transaction() method.
| @@ -69,9 +173,13 @@ pub trait SpiDevice: ErrorType { | |||
| /// On bus errors the implementation should try to deassert CS. | |||
| /// If an error occurs while deasserting CS the bus error should take priority as the return value. | |||
| /// | |||
| /// # Safety | |||
There was a problem hiding this comment.
Mention that the transaction! safe wrapper macro exist and that using it if possible is recommended.
Based on #403
This macro allows users to use
SpiDevice::transactionin a completely safe way, without needing to dereference a raw pointer.The macro is exported as
embedded_hal_async::spi_transaction_helperbecause exported macros always go in the root of the crate.It is also publicly reexported as
embedded_hal_async::spi::transaction_helperbecause 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.