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 SpiDevice generic over Word size and implement u16 transfer #2893

Merged
merged 8 commits into from
May 20, 2024

Conversation

Ragarnoy
Copy link
Contributor

It's not particularly complicated, this allows to be more in line with other implementations like ExclusiveDevice which also has an u16 word size

@Dirbaio
Copy link
Member

Dirbaio commented Apr 30, 2024

I think it should be possible to avoid adding the generic param to the struct itself, using it on impls only. This way you don't have to choose a word size when creating the SpiDevice, it'll just transparently implement all transfer sizes the underlying bus implements.

@Dirbaio
Copy link
Member

Dirbaio commented Apr 30, 2024

Also, SpiDevice should not implement the embedded-hal v0.2 SPI traits. Those are equivalent to the 1.0 SpiBus, ie they're for a bare bus without any CS pin.

@Ragarnoy
Copy link
Contributor Author

Also, SpiDevice should not implement the embedded-hal v0.2 SPI traits. Those are equivalent to the 1.0 SpiBus, ie they're for a bare bus without any CS pin.

Yeah, I was quite confused about that but went along with it. The traits that were used (Transfer, Write) do not exist anymore, so I removed them, is this correct ?

@Dirbaio
Copy link
Member

Dirbaio commented Apr 30, 2024

oh, they were already there. my bad, I misread the diff and thought you had added them. hmmm :|

@Ragarnoy Ragarnoy requested a review from Dirbaio May 1, 2024 00:27
@Dirbaio Dirbaio merged commit 34a0f74 into embassy-rs:main May 20, 2024
3 of 6 checks passed
@mattico
Copy link
Contributor

mattico commented May 20, 2024

Thanks @Ragarnoy, I was going to implement this today!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants