`I2CTarget` doesn't properly set `is_restarted` on requests

[ajmirsky]

CircuitPython version

Adafruit CircuitPython 9.0.4 on 2024-04-16; Challenger RP2040 LTE with rp2040

Code/REPL

import board
from i2ctarget import I2CTarget

import adafruit_logging as logging
from logging import NamedStreamHandler


# https://www.i2c-bus.org/repeated-start-condition/
# https://learn.adafruit.com/working-with-i2c-devices/repeated-start#whats-the-issue-3111703
def emulate_mma8451():
    logger = logging.getLogger('i2ctarget')
    logger.setLevel(logging.INFO)
    logger.addHandler(NamedStreamHandler())

    register_index = None
    register_count = 0
    print("\n\nmma8451 emulation starting...")

    # initialize our emulated 8451 at address 0x1da
    # with I2CTarget(board.D10, board.D11, (0x1d,)) as device:
    with I2CTarget(board.SCL, board.SDA, (0x1d,)) as device:
        while True:
            # loop until the target receives a request
            i2c_target_request = device.request()
            if not i2c_target_request:
                continue

            with i2c_target_request:

                # write request
                if not i2c_target_request.is_read:

                    # first byte, register index. second byte, data.
                    index = i2c_target_request.read(1)[0]
                    data = i2c_target_request.read(1)

                    if not data:
                        logger.info(f'no data, write request is part of a write-then-read transaction: 0x{index:02x}')
                        register_index = index
                        if register_index == 0x01:
                            register_count = 0
                        continue

                    #  for our emulated 8451, write transactions are a noop
                    logger.info(f"write request to index 0x{index:02x}: 0x{data:02x}")

                # read request
                else:
                    # respond to the who-am-i register
                    if register_index == 0x0D:
                        i2c_target_request.write(bytes([0x1A]))
                    # respond to the REG_OUT_X_MSB command
                    elif register_index == 0x01:
                        # these should always be repeated start reads
                        if not i2c_target_request.is_restart:
                            logger.error(f"reads from 0x01 should always be restart: {register_count}")
                        else:
                            logger.info(f"reads from 0x01 counter: {register_count}")
                        i2c_target_request.write(bytes([0x00]))
                        register_count += 1
                    else:
                        i2c_target_request.write(bytes([0x00]))
                        logger.info(f"reading from index 0x{register_index:02x}")


if __name__ == "__main__":
    emulate_mma8451()

Behavior

On a second microcontroller, run the mma8451 example :

import board
import busio
import adafruit_mma8451
from time import sleep

i2c = busio.I2C(board.SCL, board.SDA)
sensor = adafruit_mma8451.MMA8451(i2c)
while True:
    sleep(1)
    x, y, z = sensor.acceleration
    print('Acceleration: x={0:0.3f} m/s^2 y={1:0.3f} m/s^2 z={2:0.3f} m/s^2'.format(x, y, z))
    orientation = sensor.orientation
    print('Orientation: {0}'.format(orientation))

The output on the emulated I2CTarget is:

mma8451 emulation starting...
1080.440: INFO - i2ctarget : no data, write request is part of a write-then-read transaction: 0x0d
1080.445: INFO - i2ctarget : write request to index 43: bytearray(b'@')
1080.449: INFO - i2ctarget : no data, write request is part of a write-then-read transaction: 0x2b
1080.452: INFO - i2ctarget : reading from index 0x2b
1080.456: INFO - i2ctarget : write request to index 14: bytearray(b'\x01')
1080.458: INFO - i2ctarget : write request to index 43: bytearray(b'\x02')
1080.462: INFO - i2ctarget : write request to index 45: bytearray(b'\x01')
1080.466: INFO - i2ctarget : write request to index 46: bytearray(b'\x01')
1080.469: INFO - i2ctarget : write request to index 17: bytearray(b'@')
1080.473: INFO - i2ctarget : write request to index 42: bytearray(b'\x05')
1081.456: INFO - i2ctarget : no data, write request is part of a write-then-read transaction: 0x01
1081.458: ERROR - i2ctarget : reads from 0x01 should always be restart: 0
1081.462: ERROR - i2ctarget : reads from 0x01 should always be restart: 1
1081.465: ERROR - i2ctarget : reads from 0x01 should always be restart: 2
1081.470: ERROR - i2ctarget : reads from 0x01 should always be restart: 3
1081.473: ERROR - i2ctarget : reads from 0x01 should always be restart: 4
1081.477: ERROR - i2ctarget : reads from 0x01 should always be restart: 5
1081.481: INFO - i2ctarget : no data, write request is part of a write-then-read transaction: 0x0e
1081.484: INFO - i2ctarget : reading from index 0x0e
1081.487: INFO - i2ctarget : no data, write request is part of a write-then-read transaction: 0x10
1081.491: INFO - i2ctarget : reading from index 0x10

Description

When adding documentation and examples for I2CTarget, I noticed that is_restarted didn't get set. So I created the emulated mma8451 and used Adafruit CircuitPython 9.0.0 on 2024-03-19; Seeeduino XIAO RP2040 with rp2040 to run the mma8451 example.

Additional information

I think this is limited to the RP2040 (although don't have a different chip to test against). The RP2040 data sheet, page 480 says that the RESTART_DET is only set when IC_SLV_RESTART_DET_EN=1. Although I can't find in the data sheet, where IC_SLV_RESTART_DET_EN should get set.

In the pico-sdk, it lists IC_SLV_RESTART_DET_EN in the description section, but doesn't seem to mention it anywhere else.

[ajmirsky]

As I research further (and read more carefully), IC_SLV_RESTART_DET_EN is a value set when configuring Synopsis's I2C core used on the RP2040.

Since it's listed as set in hw, it points back to a problem with how circuitpython checks the IC_RAW_INTR_STAT register and I examine more carefully the register is masked with I2C_IC_RAW_INTR_STAT_RD_REQ_RESET (defined as 0x0) which will prevent is_restarted from ever getting set.

Will file a pull request shortly with the change to fix this. Apologies for the spurious messages upon my quest to figure this out : )