I2C: writes work but reads return `undefined`

[nodebotanist]

I have been working with the SparkFun Qwiic Twist and have run into an issue where I2C writes work but reads return undefined. This has been replicated on a Moddable Two and an M5Stack.

Source code:

import I2C from 'pins/i2c'
import Timer from 'timer'

const registers = {
	VERSION: 0x02,
	TWIST_COUNT: 0x05,
	STATUS: 0x01,
	RED: 0x0D,
  GREEN: 0x0E,
	BLUE: 0x0F,
	CONNECT_RED: 0x10,
  CONNECT_GREEN: 0x12,
  CONNECT_BLUE: 0x14
}

const qwiic_status = {
	BUTTON_PRESSED: 1
}

class Qwiic_RGB_Rotary {
	constructor(dictionary) {
    this.I2C = new I2C({address: 0x3F})
	}

	getVersion() {
    this.I2C.write(0x02)
		let version = this.I2C.read(2)
		Timer.delay(2)	
    version = (version[0] << 8) | version[1]
		trace(`Version: ${version}\n\n`)
	}

	qwiic_twist.write(registers.RED, 0x66)
	qwiic_twist.write(registers.GREEN, 0x33,)
	qwiic_twist.write(registers.BLUE, 0x99)

	qwiic_twist.write(registers.CONNECT_BLUE, 0x00)
	qwiic_twist.write(registers.CONNECT_BLUE, 0x10)
	qwiic_twist.write(registers.CONNECT_GREEN, 0x00)
	qwiic_twist.write(registers.CONNECT_GREEN, -10 & 0xFF)
	qwiic_twist.write(registers.CONNECT_RED, 0x00)
	qwiic_twist.write(registers.CONNECT_RED, 0x05)
}

export default Qwiic_RGB_Rotary

main.js

import Timer from 'timer'
import Qwiic_RGB_Rotary from 'qwiic-rgb-rotary'

export default function () {
	let rotary = new Qwiic_RGB_Rotary()
	const version = rotary.getVersion()
	trace(`Version: ${version}`)
}

I have tried with and without the Timer.delay() and no matter what I get:
C:\Users\Nodebotanist\Projects\moddable\examples\drivers\qwiic-twist\qwiic-rgb-rotary.js (43) # Break: Qwiic_RGB_Rotary.prototype.getVersion: cannot coerce undefined to object!

I2C writes on the same bus work as expected (rotary device lights up correctly), and I checked and double-checked the registers from the source material

[phoddie]

Hey Kas, let's try to sort this out. Since the I2C write works the wiring is correct. On the ESP32, the native I2C implementation is here. I2C devices can be quirky with some of the details. Some hacking might get it working. If so, then we can work on a clean solution.

The main reason I2C.prototype.read returns undefined on ESP32 is if the call to i2c_master_cmd_begin in modI2CRead fails. (It can fail if modI2CActivate fails too, but if writes are working for you that isn't the case). Presumably i2c_master_cmd_begin times out, but it would be good to know. You can add two lines to trace the result to xsbug:

ret = i2c_master_cmd_begin(I2C_NUM_1, cmd, 1000 / portTICK_RATE_MS);
modLogI("i2c_master_cmd_begin ret=");
modLogInt(ret);

There is an option to send the "stop" at the end of the transaction. The default (true) is usually fine, but not with some components. At the top of modI2CRead force it to false:

sendStop = 0;

Please try that. If it doesn't help, there may some other things to try.

Also - since your code doesn't specify the I2C bus speed, it is using the default of 100000. That's slow enough that it should be fine, but I don't know if it is within spec for the component.

FWIW - I looked around for an example app for the Qwiic Twist using the ESP IDF, but didn't find one. If you know of one, it should provide guidance on the how to use their i2c_master_* API with this part.

[nodebotanist]

Yeah they don't have one yet-- I am basically converting this from Sparkfun's Arduino Library. I'll take a look at these debugging steps tonight.

[nodebotanist]

So I tried these to no avail. The C code here:

modLogI("i2c_master_cmd_begin ret=");
modLogInt(ret);

Causes the build to error and I can find no mention of it in the source code.

setStop = 0

Didn't error but didn't help, and neither did setting the I2C bus to 400000.

[phoddie]

OK, we learned that the bus speed and stop don't seem to matter. That's something.

I had a typo in the C code. Apologies. It should be

modLog("i2c_master_cmd_begin ret=");
modLogInt(ret);

[nodebotanist]

With that C code added I get:

i2c_master_cmd_begin ret=
-1

[phoddie]

Alas, that's the most generic Espressif error code. -1 is ESP_FAIL which doesn't give a lot of detail. Reading their code in i2c.c / i2c_master_cmd_begin there may be more than one path that generates the error. I'm not enough of a I2C authority to try to dig deeper than this. Perhaps Andy Carle will know. I'll ping him.

[andycarle]

Hi, Kas — good to hear from you.

I'm reading through the Sparkfun Arduino library for this part, and there's nothing especially unusual about it. I would expect your code above to work based on the Sparkfun reference code, but (as Peter notes) i2c implementations can be a little quirky.

A couple of things that stand out to me from the Arduino library and the Qwiic Twist's schematic:

• The Sparkfun library does send a STOP bit between writing the register address and reading the data. So, while a reasonable guess, the suggestion to change setStop to 0 is probably not correct.

• The Qwiic Twist board has internal pull-up resistors on the SDA and SCL lines. It is plausible that having those resistors combined with the ESP32's internal pull-ups on those lines is too much for the i2c controller on the Qwiic Twist to overcome.

This would be more likely if you have a flakey connection to ground on the Qwiic Twist board for some reason—it would then have more trouble pulling those lines active low. That would reasonably explain why writes are working but reads are not; the ESP32 is able to pull the line low, but the Qwiic Twist can't.

This is certainly just speculation, but you might check that your connection to ground is solid. And it may be worth cutting the i2c pull-up jumpers on the Qwiic Twist board, as described in the Jumpers section here.

I've gone ahead and ordered a couple of the Qwiic Twist parts for the office. If we can't sort this out in the next couple of days, I'll have something I can hook up to the logic analyzer later this week.

Cheers!
 - Andy

[nodebotanist]

I will grab my oscilloscope and check on that this evening if I can, might be tomorrow evening.

[nodebotanist]

Before I make doubly sure I clipped the jumpers, when I run:

export default async() => {
	let qwiic_twist = new I2C({address: 0x3F})
	let version = 0 

	qwiic_twist.write(registers.VERSION, true)
	version = qwiic_twist.read(2)
	trace(`Version: ${version}\n\n`)
}

I get

i2c_master_cmd_begin ret=
263

but version remains undefined. This is both with stopBit set to true and false.

Still investigating...

[nodebotanist]

Innnnnnteresting. Not getting any data on the read back, but it doesn't look like a stop bit was sent either. Gonna keep digging

[andycarle]

My SparkFun box arrived this evening and I've been playing around with the Qwiic Twist. I was able to reproduce the issue you're seeing on the Moddable Two, got it working with the Sparkfun Arduino sketch, and found a fix for making the part work with the Moddable SDK.

The issue was a bit less exotic than I had guessed. Basically, the Twist is unusually slow to respond when setting up a read. That is, it stretches the I2C clock way out before ACKing read requests. You can see here (in a working example) that it takes more than 0.2ms to ACK the read setup:

The Arduino I2C implementation has a longer default timeout than the ESP IDF does, so it was working via Arduino and failing via the Moddable SDK (which uses the ESP IDF for I2C).

We don't currently have a way to specify the I2C timeout from script. I'll work on patching one in. But for now, you can just make a small change in $MODDABLE/modules/pins/i2c/esp32/modI2C.c to get the Qwiic Twist working.

At the very bottom of the file is the modI2CActivate function. Just before the return at the bottom of it, add: i2c_set_timeout(I2C_NUM_1, 20000); So, the end of the function should look like:

	gHz = conf.master.clk_speed;
	gSda = conf.sda_io_num;
	gScl = conf.scl_io_num;
	i2c_set_timeout(I2C_NUM_1, 20000);

	return 0;
}

That should get the part working for you until we can add in a proper way to specify the timeout.

(Incidentally, I did cut the pull-up jumper traces on my Twist before I found the right answer. I don't think that was necessary, but it does work fine with the ESP32 with those traces cut.)

Cheers!
 - Andy

[nodebotanist]

Yup, this cleared it up!

Should I file a clean ticket to allow setting an I2C read timeout then? I actually wouldn't mind taking a crack at that PR myself.

[andycarle]

@nodebotanist That sounds good to me—feel free to give the I2C read timeout implementation a go. Let me know if I can be of assistance in sorting it out.