Fix checking address on I2C transaction start/end#121
Fix checking address on I2C transaction start/end#121eldruin wants to merge 1 commit intorust-embedded:mainfrom
Conversation
How would you pass the address argument to the read/write functions in that case? |
|
The address is already checked in the individual steps. [ I2cTrans::transaction_start(ADDR),
I2cTrans::write(ADDR, vec![0]),
I2cTrans::read(ADDR, &mut data),
I2cTrans::transaction_end(ADDR) ]vs. [ I2cTrans::transaction_start(),
I2cTrans::write(ADDR, vec![0]),
I2cTrans::read(ADDR, &mut data),
I2cTrans::transaction_end() ] |
|
Removing the address from the transaction start is opposite to how the underlying bus works for multiple operations. The address is sent as part of start/repeated start, and not as part of the write/read operations. If anything, the address should be only included in the starts and removed from everything else. |
|
According to actual I2C specifications, the address shall be altered let is_rw = match e.expected_mode {
Mode::Write => 0u8,
Mode::Read => 1u8,
};
let addr = (e.expected_address << 1) | is_rw;But technically, this would be done in the hal implementation, so this is superfluous in mock tests. I say this because an I2C address cannot exceed 7 bits. This is something that mocks should be checking. I do agree with @asasine though. Repeatedly specifying the address for parts of the same transaction in I2C expectations does not make sense to me. |
I have noticed that the addresses supplied in the transaction start/end are not checked. Hence this PR.
Although I am wondering now if the transaction start/end should have an address parameter at all. Maybe we should remove it altogether.