i2c read issues

[deanm1278]

Hello! I am having some i2c read problems and I think this is the correct place for the report. I2C Seems to read an extra byte that then gets discarded. This could cause problems with certain I2C devices. Calling from makecode (with extra code stripped out since this gets called in a library):

let buf = pins.createBuffer(2)
const i2c = this.i2c_device.begin()
i2c.readInto(buf)
 i2c.end()

reads an extra 0xFF byte at the end:

I am also having an issue where I2C reads will just stop seemingly at random and return a strange value. Running the same code as the above screenshot stops after a while:

You can see one byte was read, an ACK was received, but the i2c master stops reading and never sends another read...
The

You can see this happening if you run this example on makecode.adafruit.com with the crickit package with circuitplayground express and adafruit crickit hardware.

forever(function () {
    light.graph(crickit.signal1.analogRead())
})

[jamesadevine]

Hi @deanm1278,

I need to get myself one of those logic analysers, that is awesome. All I have are bulky old oscilloscopes! 😄

I think this is a bug with mbed caused by using not necessarily well-supported mbed APIs in our I2C abstraction (start and stop). Whilst you'd expect most abstractions handle both start and stops, I think the mbed abstraction does not expect it.

http://asf.atmel.com/docs/latest/samd21/html/group__asfdoc__sam0__sercom__i2c__group.html#ga6bddf9e717847b5fa8462596e7e1489c

Indicates that i2c_master_read_packet_wait must be used before you can use the i2c_master_send_stop API. The mbed driver does not observe this rule.

The best way to move forward is to subclass codal::mbed::Serial and override all write and read functions inherited from codal-core::I2C using mbed's multi-write/read API as I think that is the most tested path.

The I2C abstraction works best when you have full control over the chip by writing to registers – we're on an abstraction skyscraper right now 😄

It should be noted that all of the above is speculation, but it is speculation informed by previous observations...

As per usual, I'm tied up right now... I can see if I can free myself a little to patch this.

@pelikhan @mmoskal @finneyj

[jamesadevine]

@deanm1278 I'm in an airport right now, but I think I have pushed a fix. Would you be able to test?

The two commits you'll need are:

lancaster-university/codal-mbed@0677022

and

lancaster-university/codal-circuit-playground@a4574d8

You will need to replace the I2C instance attached to the CircuitPlayground object with CPlayI2C from the commit above. Hopefully it works, let me know if you have any problems. 😄

[deanm1278]

Hey @jamesadevine unfortunately this doesn't seem to fix :(
The steps I took to test were:

• in a working pxt-adafruit directory run pxt serve --local. This built a codal core.
• in the codal core that pxt built, (pxt-adafruit/libs/blocksprj/built/codal/), pull master of libraries/codal-circuit-playground and codal-mbed. Verified that your commits were there in git log.
• rebuild the codal core with python build.py
• delete the cached core in pxt-adafruit/built/hexcache
• re-run pxt serve --local to start server and create a new cached core
• use the following code to test in makecode:

forever(function () {
    light.graph(crickit.touch1.touchRead())
})

behavior seems to be the same as before. @pelikhan @mmoskal please let me know if I was supposed to do something else to patch the codal core pxt is looking at.

[jamesadevine]

Hey @deanm1278, I believe the I2C package will need to have its class reference changed, it's probably still using codal::mbed::i2c?

[deanm1278]

ok this seems to work better after the scope fix. I'm still getting crashes but they're different than before and I'll have to dig a bit deeper into what may be causing them.
Thanks!

[deanm1278]

@jamesadevine I do have one more question though. Why is this just a circuit playground specific issue and not a general issue for all SAMD21 boards?

[jamesadevine]

@deanm1278 good question!

I think my logic was that the circuit playground builds on mbed, and thus uses the mbed::I2C interface. Since the samd21 port on mbed is bad but other mbed boards are ok-enough, it makes sense to only change it for the cplay, hence why it lives in the "target" repo.

Other devices that may come along may use the HAL rather than depend on mbed, and those implementations would be placed in this repository.

[deanm1278]

ok also is this file still supposed to be using mbed::i2c? https://github.com/lancaster-university/codal-circuit-playground/blob/master/model/CircuitPlayground.h

[jamesadevine]

@deanm1278 done. Although I don't think in MakeCode our singleton model is used, @mmoskal ?

[deanm1278]

@jamesadevine thanks! Right I don't think it does use the singleton model. Where I'm a bit stuck now is how to update this file to override the default I2C object for just the CPlay without also changing it for all other SAMD21 boards: https://github.com/Microsoft/pxt-common-packages/blob/master/libs/core/platform.h
@mmoskal maybe you could shed some light on what to do next?

[mmoskal]

As discussed on Discord: All SAMD21 boards in MakeCode use the codal-circuit-playground codal target, so we should be good to change

[deanm1278]

Hm ok I'm still getting seemingly random i2c failures after implementing this. It will still chug along just fine like this for a while:

and then all of a sudden it will fail like this and permanently stop all further traffic on the bus:

The change did fix the extra byte at the end of each read however.

The slave does everything correctly and gives an ACK, but the master just fails to complete the transaction. This leads me to believe that there is still a problem with the master I2C driver. Since it fails after a random (but short) amount of time I think something could be getting interrupted somewhere and not be happy about it.
I will have to look further into it.