I2C does not follow the embedded_hal::i2c::I2c::transaction contract

[dimpolo]

Hi everyone. Could you have a look at bundling same operations in your I2C implementation?
This is required for the pn532 crate to work correctly, see WMT-GmbH/pn532#27

Two consecutive reads should be bundled together.
Two consecutive writes should be bundled together.

Quote from the docs (emphasis mine):

Transaction contract:

• Before executing the first operation an ST is sent automatically. This is followed by SAD+R/W as appropriate.
• Data from adjacent operations of the same type are sent after each other without an SP or SR.
• Between adjacent operations of a different type an SR and SAD+R/W is sent.
• After executing the last operation an SP is sent automatically.
• If the last operation is a Read the master does not send an acknowledge for the last byte.
• ST = start condition
• SAD+R/W = slave address followed by bit 1 to indicate reading or 0 to indicate writing
• SR = repeated start condition
• SP = stop condition

[dimpolo]

Related: rust-embedded/linux-embedded-hal#82
Possible fix: https://github.com/rust-embedded/rust-i2cdev/pull/85/files

[golemparts]

Thank you for opening this issue. If anyone is willing to implement and test this I'm happy to merge in a PR, as I won't be able to work on this myself anytime soon due to other priorities.

[GChalony]

Hey @golemparts, I'm happy to give it a shot. However I had a look at the code and I'm unsure how to fix it.

• In arduino's implementation, they have separate functions to send the START byte, the data and then the STOP byte
• I've tracked down linux-embedded-hal's implementation to i2cdev::transfer but I don't really understand what they are doing

The only idea I have is to merge consecutive READ or WRITE with temporary buffers of summed sizes and then sending them. What do you think ?

[GChalony]

Well I've had a look at the linux documentation

Note that only a subset of the I2C and SMBus protocols can be achieved by
the means of read() and write() calls. In particular, so-called combined
transactions (mixing read and write messages in the same transaction)
aren't supported. For this reason, this interface is almost never used by
user-space programs.

So we cannot comply with the embedded_hal spec if we're only reading / writing to the file. Maybe we could use rust-i2cdev as a dependency ? Them seem to do it properly, and that's what is used by linux-embedded-hal

[golemparts]

Thanks for your offer to work on this @GChalony!

So we cannot comply with the embedded_hal spec if we're only reading / writing to the file. Maybe we could use rust-i2cdev as a dependency ? Them seem to do it properly, and that's what is used by linux-embedded-hal

I prefer not to add any dependencies for core functionality, especially something this small. As rust-i2cdev uses the same underlying ioctl interface as RPPAL does, you should be able to implement the same solution using REQ_RDRW. Take a look at ioctl.rs and at write_read and i2c_write_read for an example on how it's used in a single write+read operation using this sequence:

START → Address + Write Bit → Outgoing Bytes → Repeated START → Address + Read Bit → Incoming Bytes → STOP

It's been a while since I last worked on that section of the code though. After skimming through I came across this note:

// NOTE: REQ_RDWR - Only a single read operation is supported as the final message (see i2c-bcm2835.c)

Granted, that note is over 7 years old, so things might have changed, but at least on older Pi devices the required functionality needed to implement a transaction with any number of reads or writes in any order isn't fully supported, and will return an error if there is more than a single read message, and the read isn't the final operation. See i2c-bcm2835.c.