esp-hal icon indicating copy to clipboard operation
esp-hal copied to clipboard

calling `write_read` with an empty write buffer repeatedly retries I2C reads

Open TheButlah opened this issue 3 years ago • 4 comments

With this code on an ESP32-C3:

i2c
  .write_read(ADDR_DOESNT_MATTER, &[], &mut [0; RECV_LEN_DOESNT_MATTER])
  .expect("Failed i2c");

I get this signal trace: repeated.zip

image

The code panics due to the NACK and then suspends execution in a loop inside my panic handler - I have triple checked this with both probe-rs and defmt logs. However, even though execution is suspended, the hardware I2C continues to send bogus I2C transmissions. So my only conclusion is that the registers that control the hardware I2C have been left in a state that repeatedly sends incorrect data.

Probably, it is due to this code in esp-hal: image

The write command is issued, but the data to write is never actually set because bytes is empty.

TheButlah avatar Sep 28 '22 18:09 TheButlah

Thanks for opening this issue!

I did some tests on my side, too.

This is with a no bytes write and something on the bus that ACKs: empty_write_ack

This is with a one bytes write and something that ACKs: non_empty_write_ack

This is with a no bytes write and nothing that ACKs: (i.e. I think that is your case) empty_write_no_ack

This is with a one byte write and nothing that ACKs: non_empty_write_no_ack

So, what is repeated seems to be the attempt to read.

For the last case already the write operation fails and the transfer is stopped.

bjoernQ avatar Sep 29 '22 09:09 bjoernQ

We can get rid of those read attempts by adding self.reset(); to execute_transmission before returning return Err(Error::AckCheckFailed); (maybe we should do that in all error cases)

bjoernQ avatar Sep 29 '22 09:09 bjoernQ

Just out of curiosity: is this related to your problem using MPU6050? I ordered a MPU6050 break-out board but which should hopefully arrive by the end of the week

bjoernQ avatar Oct 04 '22 06:10 bjoernQ

Yes, it was. I added this PR in which originally I was writing an empty, zero length buffer as an attempt to reset any internal state in the I2C peripheral (because IDK what I'm doing lmao)

I now write a single 0x00 instead, and that results in successful communication with the MPU6050 after one or two retries

TheButlah avatar Oct 04 '22 08:10 TheButlah

Can this issue be closed now that #233 has been merged by any chance?

jessebraham avatar Nov 09 '22 16:11 jessebraham

this problem should be fixed now - will close it @TheButlah feel free to re-open if you disagree

bjoernQ avatar Nov 09 '22 16:11 bjoernQ