Support 3-wire-SPI #2919
Support 3-wire-SPI #2919
Conversation
| || (address != Address::None && address.mode() != DataMode::SingleThreeWire) | ||
| || data_mode != DataMode::SingleThreeWire) | ||
| { | ||
| return Err(Error::ArgumentsInvalid); |
There was a problem hiding this comment.
Should we use Unsupported instead?
There was a problem hiding this comment.
I was considering that (and actually I used it first) but the description suggests something else (well I could change the description ofc) - but on the other hand looking at how it's currently used it might be fine - I don't have a preference - happy to change it
There was a problem hiding this comment.
I don't think we want to have two different error variants that mean the same thing. Why something isn't supported is, in my opinion, not that important.
There was a problem hiding this comment.
really no preference on my side - maybe there are more opinions on that - if not I will remove ArgumentsInvalid
(maybe rename Unsupported to OperationUnsupported?)
| /// Enables both input and output functionality for the pin, and connects it | ||
| /// to the MOSI signal and SIO0 input signal. | ||
| pub fn with_mosi<MOSI: PeripheralOutput>(self, mosi: impl Peripheral<P = MOSI> + 'd) -> Self { | ||
| pub fn with_sio0<MOSI: PeripheralOutput>(self, mosi: impl Peripheral<P = MOSI> + 'd) -> Self { |
There was a problem hiding this comment.
I think we need some more comments in both functions to tell the user when they need MOSI or SIO0 for what they're trying to achieve
Thank you for your contribution!
We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:
Submission Checklist 📝
cargo xtask fmt-packagescommand to ensure that all changed code is formatted correctly.CHANGELOG.mdin the proper section.Extra:
Pull Request Details 📖
Description
Implement 3-wire half-duplex SPI.
Closes #2687
Closes #1826
Testing
While there is an adapted hil-test I used a C3 to simulate a slave-device and checked the implementation works for me