Am I doing something very wrong, can anyone else try this? I2C, Pi to uno

I’ve got an uno as an I2C slave to a Pi, whatever I do it keeps crashing. I can’t see why it shouldn’t work, I’ve tested with the code of both the uno and pi stripped right back to it’s essentials, no interrupts to mess with it. nothing that should go wrong. Infact the code is almost exactly what most online tutorials describe, did they never get it to work either? I am aware of the pi’s inability to clock stretch properly but don’t think this should happen when trying to address an uno at 100 or 400KHz i2c.

My problem is that about once in 30 writes from the Pi to the uno something happens which causes a NACK to be returned from the Pi’s I2C bus to the program running on it. And with about the same frequency I also get events where the Pi’s error checking says everything is fine but in reality the returning message has been corrupted and is all 255s, all 127s or sometiems all 0s. I know that because of the nature of the data I am sending upwards there is no way for the whole array on the arduino to be filled with 255s, 127s or 0s, so I kno this is I2C corruption not a problem on the uno. In either case all subsequent I2C transactions fail, coming through corrupt, until both the Pi’s I2C bus has been restarted and the uno has been reset by pulling the reset pin to ground. I know my pullups on the I2C lines are ok, I’ve got a 10K resistor on each side of a BSS138 MOSFET which acts as a level shifter on each line. And I’ve looked with an oscilloscope they are sufficiently high or low when they are supposed to be and the I2C pulses do not have too curved a starting rise or ending fall.

I’ve included below the key elements of the code for the Pi and for the uno, can anyone have a go at putting them together and see if they can get it working totally crash free? I am trying to do cycles of a write of 32 bytes from the Pi to the uno, then the Pi sends down a request for 32 bytes the othr way and then i expect 32 bytes passed from uno to pi. I want to run such a cycle of data down and back upward around 25 times a second.

I simply can’t work out how and why this is failing. Incidentally when it does fail neither the Pi or Uno hang completely, tasks which each is supposed to do keep running but the I2C does not.

Key elements of Pi code (usinf C and the BCM2835 library):

if (!bcm2835_i2c_begin()){
        printf("BCM  I2C begin fail. Are you sudo");
 }

 bcm2835_i2c_setSlaveAddress(SlaveI2CAddresses[SlaveID]);
 bcm2835_i2c_setClockDivider(BCM2835_I2C_CLOCK_DIVIDER_626);

uint8_t data = bcm2835_i2c_write(MessageToAtmega, 32);// data gives a 0,1,2 or 4 dependning on if I2C  write works or fails according to some condition, MessageToAtmega is a 32 byte array of data we want to send down
bcm2835_delayMicroseconds(250);
uint8_t data2 = bcm2835_i2c_read(AtmegasReply, 32);// data2 gives a 0,1,2 or 4 dependning on if I2C  read works or fails according to some condition, AtemagsReply is a 32 byte array which this function will fill with upcoming data bytes
 bcm2835_i2c_end();

//then run this code again a short while later and keep looping

If the I2C fails or gets corupt data upwards then, after the end of the section of code above, on the Pi I run

bcm2835_gpio_write(UnoResetPin, HIGH);
bcm2835_delay(5);
bcm2835_gpio_write(UnoResetPin, LOW);
bcm2835_delay(5);
bcm2835_i2c_end();
if (!bcm2835_i2c_begin()){
printf("BCM  I2C begin fail. Are you sudo");
}
bcm2835_i2c_setClockDivider(BCM2835_I2C_CLOCK_DIVIDER_626);
printf("!!atmega has been reset!!\n");
bcm2835_delay(35);

The uno runs code doing the following stuff, I’ve removed irrelevant functions from here so you can just see the I2C related stuff, if there are typos or mising semicolons below then they were just from copying key sections of code into this forum post and are not eros which exist in the actual code:

#include <Wire.h>
#define SLAVE_ADDRESS 0x13 
//this address does not conflict with anything else on the bus

#define Incoming_I2C_Length 32
#define Replying_I2C_Length 32

volatile byte IncomingI2C_RAW [Incoming_I2C_Length];//this version is used within the interrupt routine then converted to a non-volatile in an atomic block of code within the main loop
byte IncomingI2C [Incoming_I2C_Length];

byte ReplyingI2C_RAW [Replying_I2C_Length];
byte ReplyingI2C [Replying_I2C_Length]={0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0}; 

void receiveData(int byteCount){
  while(Wire.available()) {
    byte counter=0;
    while (counter < Incoming_I2C_Length){
      if(Wire.available()){ 
        IncomingI2C_RAW[counter] = Wire.read();
      }
      counter=counter+1;
    }
    
    //by this point we should have Incoming_I2C_length bytes worth of data in our IncomingI2C array
  }

}

// callback for sending data
void sendData(){
  Wire.write(ReplyingI2C_RAW,Replying_I2C_Length); //only one wire.write is allowed in a request callback, 32 bytes is the max that can be sent, the Replying_I2C_length here says how many bytes to send
}

void setup(){
  //various pins set up here, not A4 or A5 though
  
  //We prepare to use i2c as a slave
  Wire.begin(SLAVE_ADDRESS);
  
  //We define callbacks for i2c communication
  Wire.onReceive(receiveData);
  Wire.onRequest(sendData);
}

void loop(){
  //note that I've tried disabling evrything in here yet still get the same forms of crashes

  //we check the time
  t=micros();
  unsigned long TimeNow=micros(); //and if tried disabling all this timing but still get crashes

  uint8_t oldSREG = SREG;
      cli();
      for(byte i=0; i<Incoming_I2C_Length; i++){
        IncomingI2C[i]=IncomingI2C_RAW[i];
      }
   SREG=oldSREG;

  //do a load of pin manipulation and stuff, none of which should affect I2C and which I've tried disabling but had no effect

  uint8_t oldSREG = SREG;
  cli();
  for(byte i=0; i<Replying_I2C_Length; i++){
    ReplyingI2C_RAW[i]=ReplyingI2C[i];
  }
  SREG=oldSREG;

}

Thank for your help

This code

       for(byte i=0; i<Incoming_I2C_Length; i++){
        IncomingI2C=IncomingI2C_RAW;
       }

is NOT copying your data. These variables are arrays and you have left off the index '
A faster method of doing this would be to just use memcpy()

These variables are arrays and you have left off the index

Or were the indexes removed by the forum software because the code was improperly posted in quote tags instead of code tags?

See #7 of the how to use this forum-please read sticky to see how to post code in code tags.

Thanks for spotting that, I've checked and that error isn't in the full source code, just a typo made during copying into the forum and trying to make the variable names in the original source code into clearer ones for the post.

EDIT: actually yes it is the action of the forum software (BBcode?). I've now edited that post to use code tags properly.

What really has me wondering is whether there might be something wrong with my receiveData function, is it normal to have both two while loops and an if loop encasing the use of Wire.read()? If that isn't the problem can someone else have a go at this and see if their code is regularly crashing (every few secodns when used at the speeds I described) or not? I wonder if there is something very weird going on between pi and uno for multi-byte transaction, yet no-one else seems t have posted online about this specific issue.

It is possible the Uno is busy doing other things and you get a buffer overrun since your receive code only reads in 32 bytes, not necessarily everything that is there. Try this code (untested) since it always reads in all the receive transmission and throws away any excess bytes.

This code also signals when 32 bytes have been received so you know you have a complete message and it is time to process it.

#include <Wire.h>
#define SLAVE_ADDRESS 0x13
//this address does not conflict with anything else on the bus

const int Incoming_I2C_Length = 32;
const int Replying_I2C_Length = 32;

volatile byte IncomingI2C_RAW[Incoming_I2C_Length];//this version is used within the interrupt routine then converted to a non-volatile in an atomic block of code within the main loop
byte IncomingI2C[Incoming_I2C_Length];
volatile int IncoumingCount = 0;
volatile bool IncomingMessageReceived = false;

byte ReplyingI2C_RAW[Replying_I2C_Length];
byte ReplyingI2C[Replying_I2C_Length];

void receiveData(int byteCount) {
  while (Wire.available()) {
    byte c = Wire.read();
    if ( IncomingCount < Incmoing_I2C_Length )
      IncomingI2C_RAW[IncomingCount++] = c;
  }
  if ( IncomingCount == Incoming_I2C_Length ) IncomingMessageReceived = true;
  else IncomingMessageReceived = false;
}

// callback for sending data
void sendData() {
  Wire.write(ReplyingI2C_RAW, Replying_I2C_Length); //only one wire.write is allowed in a request callback, 32 bytes is the max that can be sent, the Replying_I2C_length here says how many bytes to send
}

void setup() {
  //various pins set up here, not A4 or A5 though

  //We prepare to use i2c as a slave
  Wire.begin(SLAVE_ADDRESS);

  //We define callbacks for i2c communication
  Wire.onReceive(receiveData);
  Wire.onRequest(sendData);
}

void loop() {
  //note that I've tried disabling evrything in here yet still get the same forms of crashes

  //we check the time
  t = micros();
  unsigned long TimeNow = micros(); //and if tried disabling all this timing but still get crashes

  uint8_t oldSREG = SREG;
  cli();
  if ( IncomingMessageReceived == true ) {
    for (byte i = 0; i < Incoming_I2C_Length; i++) {
      IncomingI2C[i] = IncomingI2C_RAW[i];
    IncomingCount = 0;  // message copied so reset count to allow next message
  }
  SREG = oldSREG;

  //do a load of pin manipulation and stuff, none of which should affect I2C and which I've tried disabling but had no effect

  uint8_t oldSREG = SREG;
  cli();
  for (byte i = 0; i < Replying_I2C_Length; i++) {
    ReplyingI2C_RAW[i] = ReplyingI2C[i];
  }
  SREG = oldSREG;

}