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

[Proposal] I2S traits - take 2 #385

Closed
wants to merge 2 commits into from
Closed

Conversation

eldruin
Copy link
Member

@eldruin eldruin commented May 11, 2022

(This was all done and discussed in #204 but this crate continued its evolution before the merge. Here I just adapted the traits to how we do things now: 1 blocking trait with all methods, no nb variant as that will be deprecated eventually.
The proof implementations need to be adapted (sorry!) but this should be pretty easy)

I added some I2S traits to kick off the discussion started in #203.
The traits accept two words for left and right and are otherwise similar to SPI as of the 0.2.x version.

An interleaved version can be found in the i2s-interleaved branch. There the data should be passed interleaved. e.g. [left0,right0,left1,right1,...]

A version of these traits for the embedded-hal 0.2.x version can be found in the i2s-v0.2.x branch.

There is another MCU implementation in stm32f4xx-hal

TODO:

Thoughts?
cc. @maxekman @samcrow @YruamaLairba

@eldruin eldruin requested a review from a team as a code owner May 11, 2022 19:28
@rust-highfive
Copy link

r? @therealprof

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

@eldruin eldruin mentioned this pull request May 11, 2022
11 tasks
@YruamaLairba
Copy link

Hello, is the API still open for change ? I think an API based on [(W,W)] instead of left:[W], right:[W] or interleaved:[W] will prevent some stupid situation, for example :

  • an user may want only left channel and use left: [W;128] and right: [W;0], does implementation expect that ?
  • a left or right sample is corrupted. Naive implementation may just drop It, causing synchronization issue between channel.
  • a malicious user use an interleaved with odd number of elements, how methods should behave ?

@eldruin
Copy link
Member Author

eldruin commented Jun 30, 2022

Changes are still possible here. The reason I chose separate buffers for left and right instead of [(W,W)] is because they are much easier to deal with than an array of tuples and from my experience, both channels are probably being handled with separate buffers in the rest of the application already.

Of course this leads to situations like the ones you listed and I would expect any implementation to either deal with them in some way or impose additional restrictions like requiring that both buffers have the same length.

I will add some more documentation to the trait about this.

@eldruin
Copy link
Member Author

eldruin commented Nov 11, 2022

I published this trait in a separate embedded-i2s crate.
I would encourage anybody to continue the discussion over there.
A discussion about integration into embedded-hal would come much later.

@eldruin eldruin closed this Nov 11, 2022
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.

None yet

4 participants