IO implementation for embedded-io, rebased and updated#126
IO implementation for embedded-io, rebased and updated#126tshakah wants to merge 7 commits intorust-embedded:mainfrom
Conversation
This applies _most_ of the final comments from the review on rust-embedded#101. I phrased one of the changes slightly differently.
|
looks good to me! just need to bump the rust version we're testing from |
|
I've updated CI to run 1.81 |
asasine
left a comment
There was a problem hiding this comment.
Couple of changes for correctness but not gonna block do just comments
| let len = transaction.response.len(); | ||
| buffer[..len].copy_from_slice(&transaction.response[..len]); | ||
|
|
||
| match transaction.expected_err { | ||
| Some(err) => Err(err), | ||
| None => Ok(len), | ||
| } |
There was a problem hiding this comment.
This doesn't match up with the comment on with_error. The expectation response buffer is copied into the buffer argument, even if there's an error.
There was a problem hiding this comment.
(@tshakah) i don't mind so much whether the slice is written or not (irl an error can occur before/during/after the write, and you don't always have space / allocators to double buffer) but it'd be good to have the comment match the behaviour.
|
|
||
| #[tokio::test] | ||
| #[cfg(feature = "embedded-hal-async")] | ||
| async fn test_async_spi_mock_write() { |
There was a problem hiding this comment.
spi looks out of place in this test name.
| let err = io.read(&mut [10, 12]).unwrap_err(); | ||
| assert_eq!(err, ErrorKind::ConnectionReset); |
There was a problem hiding this comment.
There should be an assert added here that the mutable slice argument remains unmodified.
This is just a rebase of #101 with most of the review comments applied, as I went to use
embedded-hal-mockfor a test today and found I needed those features.The only difference from the review is I phrased the optional feature sentence a little differently:
Async is supported with the optionalembedded-hal-asyncfeature.