I2C/Wire behavior reversed when using both onRequest and onReceive callbacks

[stonehippo]

I think the Wire implementation is broken, with the onRequest and onReceive handlers working in reverse order when both are defined. Here's a simple example sketch:

#include <Wire.h>

volatile byte val = 0x00;

void setup() {
    Wire.begin(0x2c);
    Wire.onRequest(wireRequest);
    Wire.onReceive(wireReceive);
}

void loop() {
// nothing to do
}

void wireRequest() {
    Wire.write(val);
}

void wireReceive(int bytes) {
    val = Wire.read();
}

If this sketch is run on a device using the AVR core, the behavior is correct when doing a write then immediate read I2C operation. For example, running something like this (using Circuitpython) sets the value and reads back the value just set:

import busio, board
import adafruit_bus_device.i2c_device as i2c_device

i2c = busio.I2C(board.SCL, board.SDA)
dev = i2c_device.I2CDevice(i2c, 0x2c)
buf = bytearray(1)
dev.readinto(buf)
print(buf)               # bytearray(b'\x00') or 0, as expected
dev.write_then_readinto(bytes([127]), buf) # try to set the value then immediately read it
print(buf)              # bytearray(b'\x7f') or 127, as expected

This sort of write then immediate read is a common pattern for working with I2C registers (which is why the Adafruit Bus Device API has an explicit method for it).

In any case, if that same sketch is run on a device using ArduinoCore-samd, such as a MKR ZERO or Adafruit Feather M0, the result will not be correct. When the write_then_readinto is executed the first time, the value reported will be 0, not 127. The next readinto or write_then_readinto will return a 127, though. Subsequent write and immediate reads will always return the previous value.

I added some Serial outputs (I know this isn't ideal, but it seems fine in this case), and in doing so, I can see that the onRequest() callback is processed before the onReceive(). For example, this modified version of the above

#include <Wire.h>

volatile byte val = 0x00;

void setup() {
    Serial.begin(115200);
    Wire.begin(0x2c);
    Wire.onRequest(wireRequest);
    Wire.onReceive(wireReceive);
}

void loop() {
// nothing to do
}

void wireRequest() {
    Serial.println("sending to master!");
    Wire.write(val);
}

void wireReceive(int bytes) {
    Serial.println("receiving from master!");
    val = Wire.read();
}

The output when running on an AVR core device:

receiving from master!
sending to master!

is this on a samd core device:

sending to master!
receiving from master!

Definitely seems like the callback order is reversed from what's expected on a write and immediate read. While this is the case, writing I2C peripherals using SAMD-based devices is kind of broken. While it is possible to write an application to do discrete read then writes (basically with a stop between them) and get the correct behavior, write then immediate read is very common.

[stonehippo]

Hmm, I didn't see #590 until after I opened this, but I believe it would resolve the issue, since repeated start is how write then immediate read is implemented. I tried it and it doesn't fully resolve the issue, plus there are some additional problems that remain.

[stonehippo]

I’ve submitted #605 to fix this issue, as well as correct some issues when handling writes from the controller to the peripheral (master to slave).

[stonehippo]

I've closed #605 and opened #669, since the previous PR had stalled and gotten bit stale. Fix is the same, but the new PR is applied against the current master head.