Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Received I2C data is mangled #8390

Open
Merlin83b opened this issue Sep 12, 2023 · 3 comments
Open

Received I2C data is mangled #8390

Merlin83b opened this issue Sep 12, 2023 · 3 comments

Comments

@Merlin83b
Copy link

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.

@Merlin83b Merlin83b added the bug label Sep 12, 2023
@jepler
Copy link
Member

jepler commented Sep 12, 2023

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
Copy link
Author

Merlin83b commented Sep 12, 2023

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.

@tannewt tannewt added this to the Support milestone Sep 12, 2023
@dhalbert
Copy link
Collaborator

dhalbert commented Feb 1, 2025

It would be worth retesting with 9.2.x, as a lot of the underlying I2C implementation has changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants