adafruit / circuitpython

CircuitPython - a Python implementation for teaching coding with microcontrollers
https://circuitpython.org
Other
4.04k stars 1.2k forks source link

Received I2C data is mangled #8390

Open Merlin83b opened 1 year ago

Merlin83b commented 1 year ago

CircuitPython version

Adafruit CircuitPython 8.2.4 on 2023-08-22; TinyS3 with ESP32S3

Code/REPL

import time
import board
from adafruit_bus_device.i2c_device import I2CDevice

def read_i2c(send="123"):
    ret = "nothing"
    try:
        with board.I2C() as i2c:
            device = I2CDevice(i2c, 0x08)
            br = bytearray('yyyyyy', 'ascii')
            with device:
                print(f"sending {send}")
                device.write_then_readinto(send, br)
            ret = br
    except Exception as e:
        print(f"{type(e)}: {e}")

    return ret

send = bytearray(1)
send = "123"

while True:
    print("I2Cing in 1 second...")
    time.sleep(1)
    r = read_i2c(send)
    print(f"Sent {send}, got {r}")

    time.sleep(5)

Behavior

Posting as an issue as suggested at https://forums.adafruit.com/viewtopic.php?p=985515#p985515

The printed output should be Sent 123, got bytearray(b'3\x00\x00\x00\x00\x00') But instead is bytearray(b'"\x00\x00\x00\x00\x00')

Description

No response

Additional information

I am sending some code over I2C to an ATTiny85 which is configured with a sketch to read it all in, then send back 6 bytes, the first of which is the final byte from the sent code. The idea was just to confirm that I could get I2C to work between the two chips, but CircuitPython seems to mangling the received the code.

The code running on the ATTiny is:

#include <Wire.h>

char rcvd = 0;

void setup() {
  Wire.begin(8);                // join i2c bus with address #8
  Wire.onReceive(receiveEvent); // register event
  Wire.onRequest(requestEvent); // register event
}

void loop() {
  delay(100);
}

void receiveEvent(int howMany) {
  while (1 < Wire.available()) { // loop through all but the last
    char c = Wire.read(); // receive byte as a character
  }
  rcvd = Wire.read();    // receive byte as an integer
}

void requestEvent() {
  char buf[6] = "xxxxxx";
  sprintf(buf, "%c      ", rcvd);
  Wire.write(buf); // respond with message of 6 bytes
}

Running Arduino on the TinyS3 with a sketch to do similar to the CircuitPython behaves correctly. Here is that sketch:

#include <Wire.h>

void setup() {
  Serial.begin(9600);
  Wire.begin(7); // join i2c bus (address optional for master)
  Wire.onReceive(receiveEvent);
}

byte x = 0;

void loop() {
  Serial.println("I2Cing in 1 second...");
  delay(1000);
  Wire.beginTransmission(8); // transmit to device #8
  Wire.write("123");        // sends five bytes
  Wire.endTransmission(false);    // stop transmitting

  Wire.requestFrom(8, 6);    // request 6 bytes from slave device #8

  Serial.print("Sent 123, received ");

  while (Wire.available()) { // slave may send less than requested
    char c = Wire.read(); // receive a byte as character
    Serial.print("\\x");
    Serial.print(c, HEX);         // print the character
  }

  Wire.endTransmission();

  Serial.print("\n");

  delay(5000);
}

void receiveEvent(int howMany) {
  while (1 < Wire.available()) { // loop through all but the last
    char c = Wire.read(); // receive byte as a character
    Serial.print(c);         // print the character
  }
  int x = Wire.read();    // receive byte as an integer
  Serial.println(x);         // print the integer
}

Which gives the expected output:

Sent 123, received \x33\x20\x20\x20\x20\x20

Further, using a login analyzer on the I2C pins shows the correct data on the wire.

jepler commented 1 year ago

Thanks for the report.

I looked over your example programs for any problems and I did notice some things that are strange but I don't think they are probably the cause of the behavior you're seeing.

The first thing I noticed is apparent incorrect buffer use in your attiny firmware:

  char buf[6] = "xxxxxx";
  sprintf(buf, "%c      ", rcvd);

Handling strings & buffers in C is .. hard. I think code is incorrect, because C strings always include a trailing "NUL" byte. This means a char buf[6] can only hold a 5-byte string plus the trailing "NUL" byte. But the sprintf() is producing (I think?) 7 bytes plus the trailing "NUL" byte, so would need char buf[8]. This could also affect the length of the buffer that Wire.write() is trying to write; changing how the buffer is filled and using the Wire.write(data, length) call style might be better.

send = bytearray(1)
send = "123"

Here, the first line has no net effect because the variable send is immediately re-assigned the value "123". To change the elements of a bytearray you'd write something like send[0] = ord('1'). (And technically the sent value should be a bytes or bytearray, not str, but this problem is apparently not detected by CircuitPython)

br = bytearray('yyyyyy', 'ascii')

because the initial value isn't being used, br = bytearray(6) is a more idiomatic way to write this. This creates a bytearray of length 6, filled with NUL bytes. I see that your forum code used the latter form, so I guess this is just an artifact of trying various things to see how the behavior is affected.

There also seems to be some confusion between whether the padding bytes are expected to be b'\0' NUL bytes or spaces b'\x20'.

The scope trace from the forum thread -- is this CircuitPython or Arduino as the I2C controller?: image

Merlin83b commented 1 year ago

It's quite some years since I really coded in C, so yes I've been lazy.

The first buffer size mismatch is a fair cop - 7 bytes (rcvd, 5 * space, null terminator). But that said, it's sending 6 bytes over the wire, so whether there's a seventh \0 byte or not isn't the issue.

The second double assignment of the send buffer was down to me simplifying things to try and narrow the problem down and not bothering to remove the previous line.

'yyyyyy' in the buffer was putting something in so at one point I could see if it was being filled in at all, as you guessed. Same for the 'ascii' encoding.

Padding bytes should be showing spaces (b'\x20') as that's what the ATTiny is sending. That is, as you see from the logic analyzer, what is one the wire. Apologies that yes, I didn't update the expected behaviour correctly to be Sent 123, got bytearray(b'3 ')

The scope trace is from CircuitPython as the master, but it looks identical when the Arduino code is running as master on the TinyS3.