I2c between arduino (slave) and raspberry pi (master)

I am writing a code that performs a PID loop on an arduino. A raspberry pi is connected to the arduino, communicating over i2c. The Pi can change things such as the setpoint and pid values, and has a user interface. In the interface, I plot the current position that is being sent over from the arduino. The pi is requesting this information every 100 ms. I send over two bytes (LSB,MSB) using Wire.write(sData,2); and receive on the Pi using i2ctest = wiringPiI2CReadReg16(fd,0x00);.

It seems that most of the time, the eighth bit on the LSB gets switched from 1 to 0 more often than not, though sometimes it isnt switched. If I print anything to serial within the interrupt then this problem goes away. I will post the relevant code below.

void setup() {
  Wire.begin(SLAVE_ADDRESS);
  Wire.setClock(1000000);
  Serial.begin(115200);
  Wire.onReceive(receiveData);
  Wire.onRequest(sendData);
  myDAC.begin(9);
  pinMode(2,INPUT);
  zeroPoint = analogRead(disppin);
  myDAC.analogWrite(512*4); //here we send out 512 because that should correspond to 0 current
}

void loop() {
  disp = analogRead(disppin);
  tosend=disp;
  
  current = analogRead(currentpin);
  //Serial.println(disp);
  //Serial.println(current);
  if(current<=100 || current>=830){ //if the current is outside threshold, reset system
    myDAC.analogWrite(512*4);
    //exit(0);
  }
  else{
  currentTime = millis();
  elapsedTime = currentTime-previousTime;
  val = disp-zeroPoint;
  
  error = (setPoint) - val;
  //Serial.println(error);
  cumError += error*elapsedTime;
  rateError = (error-lastError)/elapsedTime;
  Output = Kp*error+Ki*cumError+Kd*rateError;
  lastError = error;
  previousTime = currentTime;
  Output = max(100.0,min(900.0,Output)); //here we can make sure that we don't overload the servo valve
  //Serial.print("Output:");
  //Serial.println(Output+512);
  
  myDAC.analogWrite(4*(Output));
  }
}

void sendData(){
  //Serial.println(data);
  //Serial.println("Sent");
  //disp = analogRead(disppin);
  //tosend=1023;
  sData[1] = (tosend >>8) & 0xFF;
  sData[0] = tosend & 0xFF;
  //Wire.beginTransmission();
  //Serial.print(sData[1],BIN);
  Serial.println(sData[0],BIN); //printing this fixes the issue.
  Wire.write(sData,2);
  //Wire.endTransmission();
}

and then on the Pi
i2ctest = wiringPiI2CReadReg16(fd,0x00);

Please let me know if anyone is aware of how to fix this, or why printing something fixes the issue. While printing seems to work, it is slow and I would rather not have to print anything to serial as it will slow down my PID loop.

I doubt somehow that your code compiles; did you post the complete code? I can't find the declarations of e.g. tosend and disp.

Your topic has been moved to a more suitable location on the forum. Installation and Troubleshooting is not for problems with (nor for advise on) your project :wink: See About the Installation & Troubleshooting category.

Thanks for the response and for moving my post! I am new here as I'm sure you can see. I didn't post my full code because I thought the declarations of variables were extraneous, and the receiveData code was not too relevant either. That being said, I guess variable type could be important. I am not with the pi right now so I cannot post the exact code, but the variables used in the sendData function are declared as follows:
unsigned int tosend;
byte sData[2];

unsigned int disp;

If the Arduino board is connected to the Raspberry Pi via a USB cable, then you can use the Serial/UART communication. I think that is better.

If you want to continue with I2C, then you have to know about each and every of these subjects:

  1. Voltage levels on the I2C bus. The Raspberry Pi is not 5V tolerant and a 5V Arduino board has a 5V I2C bus. Which Arduino board do you use ?
  2. Does the Raspberry Pi support "clock pulse stretching" ? The Arduino in Slave mode uses that to give itself more time (for example to run the onRequest handler).
  3. Did you count the zeros ? Please remove the Wire.setClock() function. It has no meaning in Slave mode anyway.
  4. What about the wires or the cable ? The I2C bus is not supposed to go through a cable. If you use a cable, then SDA may not be next to SCL. The I2C bus has three wires: SDA, SCL, GND.
  5. A "unsigned int" can be 16 bit or 32 bit, depending on the Arduino board. Use the uint16_t or uint32_t types.
  6. I prefer that a variable is made 'volatile' when it is used both in a interrupt routine and in the loop().
  7. Do you have pullup resistors ? Which value and to which voltage ?

It feels like a timing problem, perhaps it might have to do with the clock pulse stretching. My second bet is that your I2C bus is so bad, that anything is possible.

Thanks so much for the response, I’ll try to reference each point.

  1. I am actually using the adafruit metro 328 which is adafruit’s arduino clone. This has the advantage of being able to set the pinout high to 3.3 volts
  2. It does not appear that the rpi supports clock stretching, but why would the clock stretch when printing?
  3. I was trying to set the clock to high speed mode and while I didn’t think it would help, when I changed this, I got more consistently correct values. Otherwise I seem to get noise or incorrect values
  4. I am connecting the three wires
  5. I will change this, though I am focused on 10 bits as data is set through an analog read
  6. I was unaware of the volatile declaration. I will add this as it may help
  7. The rpi has internal pull-up resistors to 3.3v

Thanks again for helping me through this. If I can’t fix it, I will switch to serial, though I just wanted to keep the pid loop and the communication as separate as possible. Though I don’t know that there would be a huge difference as the serial communication would also be implemented with interrupts.

  1. Adafruit METRO 328 : https://www.adafruit.com/product/2488.
    It has a jumper to run it at 3.3V or 5V.
    It is not allowed to run a ATmega328P with 16MHz at 3.3V according to the datasheet, but it will work.

  2. The SCL is kept low, all the time that is spent on the onRequest handler. That could influence the Raspberry Pi.

  3. Very unlikely that the Wire.onClock() has influence. The value will work for a Master from 50000 to 400000 (50k to 400k). If you want to try something, use such a normal value. So did you count your zeros :wink: Wire.setClock(10000000000000000

  4. What kind of wires ? Cheap jumper wires ? Is there a cheap breadboard somewhere nearby ?

  5. Use indeed 16 bit everywhere. The Raspberry Pi is 64-bit, running a 32-bit operating system and the Arduino is 8-bit but the compiler defaults to 16-bit.

  6. Show the sketch.

  7. Does every Raspberry Pi board have 1k8 pullup resistors for the I2C bus ? That is a strong pullup.

After some extra inspection, I found that this issue is only with the first bit being sent over. If I send only 8 bits, like 255, it oscillates between 255 and 127. If I send 10, then the first bit in the lsb is switching and this is the bit that is sent first. I have inverted the sending order, so that the first byte sent is the msb as I don’t use the most significant bit in that byte. Doing this also fixes the problem.
I believe the problem actually has to do with onrecieve and onrequest working in tandem. When a request is sent, onrecieve is still activated and I have it just read the incoming signal. Does it send a confirmation that the signal was read? This could be what is causing the flipping bit. If yes, is there a way I can exit onrecieve without reading the request? Or a way to only activate onrecieve if more than one byte is sent?

In the first reply, user sterretje asked for the complete code. There is a very good reason for that.

On this forum, we often say that the problem is in the part that someone is not showing.
Or someone might start to talk about looking into a "crystal ball" to try to see the things that you are not telling. The "crystal ball" is often not well polished and a old one from a flea market, so it is not very clear.

Now you ask questions about the code in the onReceive handler. Do you really want me to grab my crystal ball ? I have it somewhere in a box upstairs.

Nevertheless, you might be on to something.

First of all, do you use the Arduino IDE with the Arduino Uno selected ? If you use the Adafruit environment, then maybe you have an old version of the Wire library which had a bug.
If possible, you could try to wait 1ms in the Raspberry Pi, between sending data and requesting data.

When the Raspberry Pi requests data, then the Arduino makes SDA low for a ACK after the I2C address is detected. Immediately after that, the Raspberry Pi gives a clock pulse and reads SDA. The Raspberry Pi gives the ACK signal (a low SDA) after all the data bytes.

It seems as if the SDA does not turn high fast enough after the Arduino made the SDA low for a ACK.

The I2C bus is not fault tolerant, it should work 100% all the time.
I understand that you are trying to make workaround. In my opinion this is a serious bug, something is terribly wrong and that I2C bus totally unreliable.

I definitely agree, and I couldn't upload the code until just now as I did not have access to the Pi. Here is the full code.

#include <Wire.h>
#include <Adafruit_MCP4725.h>
#include "MCP_DAC.h"

#define SLAVE_ADDRESS 0x08
MCP4921 myDAC;

byte data_to_echo = 0;
byte dataArray[2];
unsigned int data = 0.0;
unsigned int value = 0.0;
unsigned int command;
byte sData[2];
unsigned int test = 10;
int currentpin = A2;
int disppin = A1;
int disp;
int current;
int j;
double val =0;
double currentTime;
double previousTime = 0.0;
double elapsedTime;
double error;
double cumError;
double lastError = 0.0;
double rateError;
double Output;
double Kp=1;
double Ki=0;
double Kd=0;
double setPoint=512;
double zeroPoint=512;
double offset = 43.0;
double pmax = 5;
double imax = .001;
double dmax = 5;
double span = 5.5;
double maxspan = 5.5;
unsigned int tosend;

void setup() {
  // put your setup code here, to run once:
  Wire.begin(SLAVE_ADDRESS);
  Wire.setClock(1000000);
  Serial.begin(115200);
  Wire.onReceive(receiveData);
  Wire.onRequest(sendData);
  myDAC.begin(9);
  pinMode(2,INPUT);
  //zeroPoint = analogRead(disppin);
  myDAC.analogWrite(512*4); //here we send out 512 because that should correspond to 0 current
}

void loop() {
  disp = analogRead(disppin);
  tosend=disp;
  
  current = analogRead(currentpin);
  //Serial.println(disp);
  //Serial.println(current);
  if(current<=100 || current>=830){ //if the current is outside threshold, reset system
    myDAC.analogWrite(512*4);
    //exit(0);
  }
  else{
  currentTime = micros();
  elapsedTime = currentTime-previousTime;
  val = disp-zeroPoint;
  
  error = (setPoint) - val;
  //Serial.println(error);
  cumError += error;
  rateError = (error-lastError)/elapsedTime;
  Output = Kp*error-Ki*cumError+Kd*rateError;
  lastError = error;
  previousTime = currentTime;
  Output = max(100.0,min(900.0,Output)); //here we can make sure that we don't overload the servo valve
  //Serial.print("Output:");
  
  Serial.println(Output);
  myDAC.analogWrite(4*(Output));
  }
}

void receiveData(int bytecount){
  //Serial.println(bytecount);
  if (bytecount>1){
    for(int i = 0;i<bytecount;i++){
      dataArray[i] = Wire.read();
    }
    
  
    data = combineBytes((char) dataArray[2],(char) dataArray[1]);
    command = data >> 10;
    value = data<<6;
    value = value>>6;
    if (command == 0){
      setPoint = value;
      Serial.println(value);
    }
    else if(command ==1){
      Kp = value*pmax/1023;
      Serial.println(Kp);
    }
    else if(command ==2){
      Ki = value * imax/1023;
      Serial.println(Ki);
    }
    else if (command == 3){
      Kd = value*dmax/1023;
      Serial.println(Kd);
    }
    else if (command ==4){
      span = (value*2*maxspan/1023);
      Serial.println(span);
    }
    else if (command ==5){
      zeroPoint = value*2*span/1023;
      Serial.println(zeroPoint);
    }
    
    }
  else{
    Wire.read();
  }
  
}

void sendData(){
  //Serial.println(data);
  //Serial.println("Sent");
  //disp = analogRead(disppin);
  //tosend=1023;
  sData[0] = (tosend >>8) & 0xFF;
  sData[1] = tosend & 0xFF;
  //Wire.beginTransmission();
  //Serial.print(sData[1],BIN);
  //Serial.println(sData[0],BIN);
  Wire.write(sData,2);
  //Wire.endTransmission();
}

int combineBytes(unsigned char x_high, unsigned char x_low){
  int combined;
  combined = x_high;
  combined = x_high<<8;
  combined |= x_low;
  return combined;
}

Hopefully it helps find the issue

Count your zeros.

Can you show a photo of the project ? Can you give a link to the MCP module ? Variables that are used both in the interrupt and in the loop() should be 'volatile'. If you receive 3 bytes instead of 2, then the array is written beyond its limits. I don't like the use of Serial.println() in the interrupt, can you move that to the loop() ? Do you know the Arduino word() ? I don't see how the loop() runs predictable. The micros() is 4µs accurate and is calculated every time the loop() runs. You are relying on the delay caused by running the code. If the compiler optimizes a lot, then the accuracy is less.

Thank you for your help! I know that there seem to be too many zeros on the clock function, but I have to insist that it runs worse when I remove the setclock or even remove a zero to bring it down to 100 kHz. I will add the volatile to the variables, as I forgot to do that. I will also move the print statements, and can in fact remove them entirely as they were just to check that the correct command was being read. Do you recommend using word() instead of combining the bytes manually as I do? And I am not sure I understand what you are saying about the loop, It seems to take approximately 750 micros to run, and I am only using that information for the rate constant of the pid loop which is often just set to 0.

There is also a lot going on in my system, are you particularly interested in the i2c portion like the cables, or would you like the entire circuitry?

There are many more aspects to everything I wrote, and there are questions unanswered and many subjects not mentioned yet (ground currents, noisy power supply, things that are different that you think, two Masters on the bus will never work reliable, and so on).

If the MCP4725 was connected to the same I2C bus all the time, then I should read this topic from the start and see what I have missed, which is super vague because I don't know if that MCP4725 is a module with perhaps voltage regulator, pullup resistors, level shifter, and so on.

If you don't have the equipment or knowledge to get closer to the cause of the problem, then it is hard for me to help online.


Let's pick one problem, the zeros :face_with_raised_eyebrow:
First of all, you have not told which build environment you use. Suppose you have Arduino IDE 1.8.16 and have the official Arduino Uno selected.
Then this is the Wire.setClock function, which calls the setFrequency function. There you see:

void twi_setFrequency(uint32_t frequency)
{
  TWBR = ((F_CPU / frequency) - 16) / 2;
  
  /* twi bit rate formula from atmega128 manual pg 204
  SCL Frequency = CPU Clock Frequency / (16 + (2 * TWBR))
  note: TWBR should be 10 or higher for master mode
  It is 72 for a 16mhz Wiring board with 100kHz TWI */
}

There is no boundary checking, it is just the formula.

The CPU frequency is 16MHz and with 100kHz, the result is 72. That is a good value.
With all your zeros 1000000, the result is 0. That is not allowed, that will fail.
As far as I know, the frequency is not used in Slave mode.
This means that you could be fixing a bug with another bug.
You really have to remove all your zeros to be able to find the real cause.


Your zeros is just one thing. There are at least 20 more possible serious problems. Up to now, you won't even fix bug of all the zeros. I'm afraid that I can't help any further, I'm bailing out of this topic.