I2C binary command 0x08 (write_then_read) does not function correctly

[ChristopherSamSoon]

The binary command 0x08 does not function correctly if we want to write to and then read from and I2C device

In order to perform a write followed by a read operation, I2C requires that a repeated start condition is to be placed on the bus after the write operation. After the repeated start condition has been sent, the address (could be another address) is then resent before reading operation begins. This is because the address encodes the operation type (write or read)

Currently the command for 0x08 is in the following format

0x08 NumWriteMSB NumWriteLSB NumReadMSB NumReadLSB WritePayload The first byte of WritePayload is assumed to contain the device address

The actual data stream placed on the bus by buspirate is

I2cStart , WritePayload, ReadAttempt, I2CStop

Because no repeated start is sent, 0xFF always received during read operation as the I2C device is not placing any data on the bus

The correct byte stream that should be sent by bus pirate is

I2cStart , WritePayload, I2CRestart, [1st Byte of write Payload with Read bit set] , ReadAttempt, I2CStop

Of course, repeated start should not be sent if the number of write bytes is 1 !! (as it would be just be the device address in this case), or if the number of desired reads is zero

[agatti]

Hi there! Does this issue occur with pre-7.0 versions of the firmware or has it always been like this?

[ChristopherSamSoon]

This issue was noticed in firmware release 7.1 (community firmware edition). I also expect the pre-7.0 versions to have similar behaviour. However, I did not test the pre-7.0 versions.

In regards to my above post, I would like to make a correction:

"In order to perform a write followed by a read operation, I2C requires that a repeated start stop-start or repeated start condition is to be placed on the bus after the write operation"

On an independent issue, the software mode for I2C does not support clock stretching on the SCL line (I also expect pre-7.0 firmware to have this problem as well). When I used the bus pirate to communicate to the I2C device that is slow to response to I2C reads/writes (aka I2C MCU responding to I2C interrupts), the buspirate went hay-wire and synchronization was lost after some time.

In light of this, I have independently re-written command 0x08 to fix the read_write problem as mentioned above and also adapted it to use hardware I2C instead of software. I can also paste the code here if it is helpful for you. However, the real fix for this one is to re-write the software I2C mode to support clock stretching

Best!

Christopher

[agatti]

Taking a look at your code would be helpful indeed, however adding clock stretching for SW I2C might be left out for 7.1 at this point and will have to be pushed to 7.2. I'll create a specific issue to track this very problem then.

[ChristopherSamSoon]

Below is the code snippet of function void binary_io_enter_i2c_mode(void) in i2c.h for command 0x08

As you can see I have re-written the portion to use hardware_i2c, but it can easily be replaced with software i2c function calls (even though clock stretching will not be supported. Not shown inside is the setup code that allows the buspirate to enter hardware mode

case 8: // write-then-read hardware
        // get the number of commands that will follow
        fw = user_serial_read_byte();
        fw = fw << 8;
        fw |= user_serial_read_byte();
        // get the number of reads to do
        fr = user_serial_read_byte();
        fr = fr << 8;
        fr |= user_serial_read_byte();

        // check length and report error
        if (fw > BP_TERMINAL_BUFFER_SIZE || fr > BP_TERMINAL_BUFFER_SIZE) {
        I2C_write_read_errorH: // use this for the read error too
          REPORT_IO_FAILURE();
          break;
        }

        // get bytes
        for (j = 0; j < fw; j++) {
          bus_pirate_configuration.terminal_input[j] = user_serial_read_byte();
        }

        // start

        hardware_i2c_start();
        i2Caddress = bus_pirate_configuration.terminal_input[0];

        for (j = 0; j < fw; j++) {
          // get ACK
          // if no ack, goto error
          hardware_i2c_write(bus_pirate_configuration.terminal_input[j]);// send byte
          uTemp8 = hardware_i2c_get_ack();
          if (uTemp8 != I2C_ACK_BIT)
            goto I2C_write_read_errorH;
        }

        if(fr > 0){ // do we want to read ?
            if(fw > 1) { //previous write was address and data ?
                //Send restart
                // intead of a repeated start, a I2C stop, followed by a I2C start can be used instead
                hardware_i2c_repeated_start();
                hardware_i2c_write(i2Caddress | 0x01); // send address again
                uTemp8 = hardware_i2c_get_ack();
                if (uTemp8 != I2C_ACK_BIT)
                    goto I2C_write_read_errorH;
            }

            fw = fr - 1;
            for (j = 0; j < fr; j++) { // read bulk bytes from SPI

              bus_pirate_configuration.terminal_input[j] = hardware_i2c_read();

              if (j < fw) {
                hardware_i2c_send_ack(I2C_ACK_BIT);
              } else {
                hardware_i2c_send_ack(I2C_NACK_BIT);
              }
            }            
        }
        // I2C stop
        hardware_i2c_stop();
        REPORT_IO_SUCCESS();

        for (j = 0; j < fr; j++) { // send the read buffer contents over serial
          user_serial_transmit_character(bus_pirate_configuration.terminal_input[j]);
        }
        break; // 00001001 xxxxxxxx

`

[agatti]

Thank you, I'll take a look at this as soon as possible.

[badrbouslikhin]

Hi! We have the exact same problem on our BusPirate running version 6.0 FW. Has it been fixed on the latest version? Thank you.

[ubidefeo]

Hi everyone! I can confirm this has been affecting 7.0 as well. I'm communicating with an STMPE811 touch screen controller which requires a restart condition to read after writing device+register addresses

It works well from an Arduino Micro and UNO but fails miserably from the BP4 running 7.1 (upgraded yesterday to see if it fixed the issue).

The read values I get back are messed up and often not consistent. Hope it gets fixed in a future release. @ChristopherSamSoon , any chance you can release your fix as a patch? :)

[ChristopherSamSoon]

Hi All !

Ok, I just forked the repo and implemented the fix for command 0x08. Link : https://github.com/ChristopherSamSoon/Bus_Pirate

Inside the forked repo, you will be able to find the compiled binary hex file Tested with buspirate-v4.

I will open a pull request for the fix to be merged to the base branch.

On a different note: The software I2C mode clock frequency is 1kHz max even if you select 400kHz. This is a bug and I think there is already a seperate open issue regarding that

[ubidefeo]

thank you, @ChristopherSamSoon I'll clone that and compile it on my machine so I can test with the specific chip I'm using. will keep you posted on the results :)

[ChristopherSamSoon]

@ubidefeo , Let us know if this fix your problem! We can help you if you have any further issues

[informer2016]

@agatti Hi agatti, please verify this new pull request by @ChristopherSamSoon - https://github.com/BusPirate/Bus_Pirate/pull/93

[agatti]

Can anyone test this so I can close the ticket? I don't have any working I2C devices at the moment...

[wakass]

This was never merged! It seems to work for me at least, using pybuspiratelite.