I2c unify

[Dirbaio]

Depends on #336

Equivalent of #323 but for I2c.

I think for i2c unifying everything in a single trait makes the most sense. The i2c bus is specified to be bidirectional, I believe no hardware out there can "only write" or "only read" (and writing requires reading ACK bits anyway!).

[rust-highfive]

r? @eldruin

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

[eldruin]

Seems like a good idea, although the trait has a lot of methods now.
Are we sure it is always possible to implement the iterator methods?
Also, for consistency, if we rename the exec method in SPI batch we should do so here as well.

[Dirbaio]

Are we sure it is always possible to implement the iterator methods?

For stm32, rp2040 yes.

For nRF it'd need to drain the iterator into a temp buffer then DMA that out. However there are other cases that nRF has to do similar copies to temp bufs (for example, it can't DMA out of flash, so if TXing from flash it has to be copied to RAM) so I don't think it's that bad.

If the iterator methods were separate traits, we'd have the usual problem: some HALs wouldn't impl them, so driver authors wouldn't use them since they want their driver to work everywhere. IMO they should be either in the main trait, or removed.

Also, for consistency, if we rename the exec method in SPI batch we should do so here as well.

Agreed they should be consistent! I'm not 100% happy with the batch name though. I liked "Transactional", it emphasizes it's a single transaction, not equivalent to just doing multiple read/write calls. Perhaps transaction? exec_transaction?

[burrbull]

For stm32

Yes for writing. Probably not for reading.

For correct ending of read operation we need to know length of remaining piece.

https://github.com/stm32-rs/stm32f1xx-hal/blob/e790b27663c03362b7dfac924f55b02d00869c08/src/i2c/blocking.rs#L291

[Dirbaio]

There's no iterator-based reading.

[burrbull]

For this reason it is not there. Same as Transactional.

[Dirbaio]

@burrbull not sure if I understand. The trait proposed in this PR has a write_iter, but no read_iter, so AFAIK all the methods should be implementable for stm32s. Are you saying a method is not implementable, or just "if there was a read_iter method, it wouldn't be"?

[burrbull]

The trait proposed in this PR has a write_iter, but no read_iter

Oh, sorry. I was sure I'd seen it.

[Dirbaio]

Renamed exec to transaction, naming is now consistent with #323

[eldruin]

Great work, thank you!
bors r+

[bors]

👎 Rejected by too few approved reviews

[eldruin]

bors r+

[bors]

Build succeeded:

• ci-linux (1.46.0, x86_64-unknown-linux-gnu)
• ci-linux (stable, thumbv6m-none-eabi)
• ci-linux (stable, thumbv7m-none-eabi)
• ci-linux (stable, x86_64-unknown-linux-gnu)
• ci-linux-test (1.46.0, x86_64-unknown-linux-gnu)
• ci-linux-test (stable)
• fmt