Receiving and writing data over I2C does not work

bally94 · December 19, 2021, 12:33pm

I have tried to send and receive some data over I2C. It works perfectly, if I send data inside the loop and receive the data on the other side with Wire.onReceive() and send some new data inside the loop.
But now, I have tried to send the received data directly back to the master instead of write inside the loop a new sending command. Why this does not work? With the attached code it sends and receives nothing anymore!

Master sender:

#include <Wire.h>

const byte serialBuffer = 10;
char receivedData[serialBuffer];
char sentData[serialBuffer];

boolean newData = false;

byte DPangle = 13;

byte A = 0;
byte B = 0;
byte C = 0;
byte D = 0;
byte BUT = 0;

void setup() {
    Wire.begin(10);
    //Wire.setClock(300000);
    Wire.onReceive(readCommand);

    Serial.begin(38400);
}

void loop() {
    //readCommand(Wire);

    //command('G',122, 9);
    //command('E',2, 9);
    command('#',DPangle, 9);
    //command('F',189, 9);
    //command('H',51, 9);
}


void command(const char command, const byte value, const byte address){
  sprintf(sentData, "\n%c%d\r", command,value); //generates a formatted string output with the format <character|decimal> from the char command and value with start- and end-marker and stores it to buffer
  const size_t characters {strlen(sentData)};
  //Serial.print("length: "); Serial.println(characters);
  Wire.beginTransmission(address);
  Wire.write(sentData, characters);
  Wire.endTransmission();
  delay(12);
}


void readCommand(int bytes){
  readIncomingData();
  storeReceivedData();
}


void readIncomingData(){
  static boolean InProgress = false;
  static byte i = 0;
  char startMarker = '\n';
  char endMarker = '\r';
  char rc;
 
  while (Wire.available() > 0 && newData == false){
    rc = Wire.read();
      if (InProgress == true){
        if (rc != endMarker){
          receivedData[i] = rc;
          i++;
          if (i >= serialBuffer){
            i = serialBuffer - 1;
          }
        } 
        else {
          receivedData[i] = '\0'; // terminate the string
          InProgress = false;
          i = 0;
          newData = true;
        }
      }
      else if (rc == startMarker){
        InProgress = true;
      }
  }
}


void storeReceivedData(){
  char var;
  int y = 0;
  
  if (newData == true){
    var = receivedData[0];
    y = atoi(&receivedData[1]);  //making the integer part
        
      for(byte i=0; i < serialBuffer; i++){
        receivedData[i] = '\0';
      }

    switch(var){
      case 'A' : A = y;
                 break;
      case 'B' : B = y;
                 break;
      case 'C' : C = y;
                 break;
      case 'D' : D = y;
                 break;
      case '@' : BUT = y;
                 break;
    }

    Serial.print("A = ");
    Serial.println(A);
    Serial.print("B = ");
    Serial.println(B);
    Serial.print("C = ");
    Serial.println(C);
    Serial.print("D = ");
    Serial.println(D);
    Serial.print("BUT = ");
    Serial.println(BUT);
    
    newData = false;
  }
}

Slave receiver:

#include <Wire.h>

const byte serialBuffer = 10;
char receivedData[serialBuffer];
char sentData[serialBuffer];

boolean newData = false;

int E = 0;
int F = 0;
int G = 0;
int H = 0;
int BUT = 0;

void setup() {
    Wire.begin(9);
    //Wire.setClock(300000);
    Wire.onReceive(readCommand);
    
    Serial.begin(38400);
}

void loop() {
    //readCommand(Wire);

    //command('@',255, 10);
    //command('C',122, 10);
    //command('A',05, 10);
    //command('D',242, 10);
}


void command(const char command, const byte value, const byte address){
  sprintf(sentData, "\n%c%d\r", command,value); //generates a formatted string output with the format <character|decimal> from the char command and value with start- and end-marker and stores it to buffer
  const size_t characters {strlen(sentData)};
  //Serial.print("length: "); Serial.println(characters);
  Wire.beginTransmission(address);
  Wire.write(sentData, characters);
  Wire.endTransmission();
  delay(12);
}


void readCommand(int bytes){
  readIncomingData();
  storeReceivedData();
}


void readIncomingData(){
  static boolean InProgress = false;
  static byte i = 0;
  char startMarker = '\n';
  char endMarker = '\r';
  char rc;
 
  while (Wire.available() > 0 && newData == false){
    rc = Wire.read();
      if (InProgress == true){
        if (rc != endMarker){
          receivedData[i] = rc;
          i++;
          if (i >= serialBuffer){
            i = serialBuffer - 1;
          }
        } 
        else {
          receivedData[i] = '\0'; // terminate the string
          InProgress = false;
          i = 0;
          newData = true;
        }
      }
      else if (rc == startMarker){
        InProgress = true;
      }
  }
}


void storeReceivedData(){
  char var;
  int y = 0;
  
  if (newData == true){
    var = receivedData[0];
    y = atoi(&receivedData[1]);  //making the integer part

      for(byte i=0; i < serialBuffer; i++){
        receivedData[i] = '\0';
      }

    switch(var){
      case 'E' : E = y;
                 break;
      case 'F' : F = y;
                 break;
      case 'G' : G = y;
                 break;
      case 'H' : H = y;
                 break;
      case '#' : BUT = y;
                 command('A',BUT,10);
                 break;
    }

    Serial.print("E = ");
    Serial.println(E);
    Serial.print("F = ");
    Serial.println(F);
    Serial.print("G = ");
    Serial.println(G);
    Serial.print("H = ");
    Serial.println(H);
    Serial.print("BUT = ");
    Serial.println(BUT);
    
    newData = false;
  }
}

StefanL38 · December 20, 2021, 7:46am

if you define variables inside a function.
This variable is only accessible inside this function.
To make a variable accessible from everywhere you have to declare the variable outside of the function.

This is called a global declared variable

best regards Stefan

Koepel · December 20, 2021, 8:17am

We don't send readable ASCII over the I2C bus, we use the variables themself (binary data).
That means that all your atoi() and sprintf() are not needed. All the code to read the string, to detect the end of the string, adding a zero-terminator, the while-loops, all that code is not needed.
The I2C bus is not a serial stream of data. It is packages of data.

The onReceive and onRequest handlers are called from a interrupt. Treat them as interrupts routines. Keep them very short and very fast. Avoid using any Serial functions. It is not possible to do a I2C session while in the onReceive or onRequest handler. That does not work as you already noticed.

When two Arduino boards are able to send data, then they both can be Master on the I2C bus. That will go wrong some day. It is better to keep one Arduino board as Master and everything else as Slave.

bally94 · December 20, 2021, 11:37am

Where I have variables that are used global? Could you show it to me please?
The function command() inside readIncomingData() uses only the global declared variable BUT. The other parameters are given directly as numbers/letters.

bally94 · December 20, 2021, 11:41am

The problem was, that I have received sometimes not all the data or wrong values. For that reason I have put the start- and end-marker. Now I had not any faults during transmission.
Can you show me what is not needed or could be deleted to make it shorter?
The serial output is only during the test routines to see what is sent and received.

Do you mean something like that?

#include <Wire.h>

int E = 0;
int F = 0;
int G = 0;
int H = 0;
int BUT = 0;

void setup() {
    Wire.begin(9);
    //Wire.setClock(300000);
    Wire.onReceive(readCommand);
    
    Serial.begin(38400);
}

void loop() {
    
    Serial.print("E = ");
    Serial.println(E);
    Serial.print("F = ");
    Serial.println(F);
    Serial.print("G = ");
    Serial.println(G);
    Serial.print("H = ");
    Serial.println(H);
    Serial.print("BUT = ");
    Serial.println(BUT);

    //command('@',255, 10);
    //command('C',122, 10);
    //command('A',05, 10);
    //command('D',242, 10);
}


void command(const char command, const byte value, const byte address){
  Wire.beginTransmission(address);
  Wire.write(command);
  Wire.write(value);
  Wire.write('\0');
  Wire.endTransmission();
  delay(12);
}


void readCommand(int bytes){
  char var;
  
    var = Wire.read();

    switch(var){
      case 'E' : E = var;
                 break;
      case 'F' : F = var;
                 break;
      case 'G' : G = var;
                 break;
      case 'H' : H = var;
                 break;
      case '#' : BUT = var;
                 command('A',BUT,10);
                 break;
    }

}

Koepel · December 20, 2021, 1:50pm

The I2C bus has a START and STOP condition, there is not need to add extra start end stop bytes. The I2C bus is also not a fault tolerant bus, you should make it reliable.
Do you use wires or cables (what cables, how long, and so on) ? Which Arduino boards do you use ? Do you use pullup resistors ? Can you show of photo of the wiring ?

Yes, like that. There is no need to send a "\0". The I2C bus uses binary packages of data.

I have a working Master and Slave sketch for you.
I made a few fixes:

Sometimes you use an indent of 2 spaces and sometimes 4. I use 2 spaces.
A baudrate is 115200 is often used, although in many examples the very slow baudrate of 9600 is used.
I'm not naming the variables A, B, C, D, BUT. I name them A, B, C, D, E, and then I don't even use that, because I put them in an array.
I didn't like to request a byte with a command when there are only 5 bytes. So I request 5 bytes.

Master sketch

#include <Wire.h>

byte DPangle = 13;
byte receivedData[5];

void setup() 
{
  Serial.begin(115200);        // 9600 is old slow default, 115200 is often used today
  Serial.println("The Master (Controller) sketch has started");

  Wire.begin();                // Master mode only
}

void loop() 
{
  int x = random( 0, 5);
  if( x == 0)
    command('G',122, 9);
  else if( x == 1)
    command('E',2, 9);
  else if( x == 2)
    command('#',DPangle, 9);
  else if( x == 3)
    command('F',189, 9);
  else
    command('H',51, 9);

  requestData( receivedData, sizeof(receivedData), 9);     // request data from I2C address 9
  for( int i=0; i< (int)sizeof( receivedData); i++)
  {
    Serial.print( (char)('A' + i));
    Serial.print( " = ");
    Serial.print( receivedData[i]);
    Serial.print( ", ");
  }
  Serial.println();

  delay( 250);
}


// sending two bytes
bool command( char command, byte value, byte address)
{
  Wire.beginTransmission(address);
  Wire.write(command);
  Wire.write(value);
  int error = Wire.endTransmission();

  return( error == 0);
}


// request all the bytes, 5 bytes at this moment
bool requestData( byte *pData, byte bytes, byte address)
{
  bool success = false;

  int n = Wire.requestFrom( address, bytes);
  if( n == bytes)
  {
    Wire.readBytes( pData, bytes);
    success = true;
  }
  return( success);
}

Slave sketch

#include <Wire.h>

volatile byte dataReceived[2];
volatile bool newData;

byte dataToSend[5] = { 12, 114, 46, 8, 220};

void setup() 
{
  Serial.begin( 115200);
  Serial.println("The Slave (Target) sketch has started");

  Wire.begin(9);

  Wire.onReceive( receiveEvent);
  Wire.onRequest( requestEvent);
}

void loop()
{
  if(newData)
  {
    Serial.print( "Command = ");
    Serial.print( dataReceived[0]);
    Serial.print( ", value = ");
    Serial.print( dataReceived[1]);
    Serial.println();
    newData = false;
  }
}

void receiveEvent(int howMany)
{
  if(howMany == 2)          // expecting two bytes
  {
    dataReceived[0] = (byte) Wire.read();  // read first byte
    dataReceived[1] = (byte) Wire.read();  // read second byte
    newData = true;
  }
}

void requestEvent()
{
  // return the 5 bytes
  Wire.write( dataToSend, sizeof( dataToSend));
}

Perhaps I should have send 5 bytes instead of 2 bytes. It is only 5 bytes. The buffer inside the Wire library is 32 bytes, so up to 32 bytes can be send or received with basic Arduino boards, such as the Arduino Uno.

bally94 · December 20, 2021, 7:23pm

Thanks a lot! That is a very simple method, but does not receive it correctly. On the serial monitor I get this output:

Command = 72, value = 189
Command = 35, value = 51
Command = 70, value = 13
Command = 72, value = 122
Command = 69, value = 51
Command = 72, value = 13
Command = 35, value = 51
Command = 71, value = 2
Command = 69, value = 122
Command = 35, value = 2
Command = 72, value = 122
Command = 71, value = 51
Command = 71, value = 122
Command = 70, value = 122
Command = 72, value = 51
Command = 71, value = 51
Command = 72, value = 2
Command = 71, value = 2
Command = 72, value = 13
Command = 69, value = 13
Command = 72, value = 2
Command = 72, value = 122
Command = 71, value = 122
Command = 35, value = 2
Command = 71, value = 122
Command = 69, value = 122
Command = 70, value = 51
Command = 35, value = 122

For example, the sommand 72 should be every time the same value, but it is not!
Is it really that easy to send and receive data over I2C? My sketch works without issues and is not slower I think, is not it?!
What I do not understand is the return(error == 0);, what does it make? Only when error is equal to 0 it returns error?

Koepel · December 21, 2021, 12:03am

I run the sketches in Tinkercad on two Arduino Uno boards and there it is alright.
Since the values are always a value that belongs to a other number, it is not noise on the I2C bus. It must be something in software or with a timing mismatch.

Have you changed something ? For example removing the 'volatile' keyword or removing the delay(250) in the Master ? Both can be fixed. Do you use something else than a Arduino Uno ?

A if-statement has a condition that is either true or false. For example: if(error == 0) then the variable error is compared to zero. It is possible to grab that condition and use it in other situations.
So this is the condition on its own, it is true or false: (error == 0)
That means it can be used to translate a variable into a condition and have that condition be true or false. This returns true if error is zero or else false: return(error == 0)

This is the same:

// The function command returns true if it was successful
bool command( char command, byte value, byte address)
{
  Wire.beginTransmission(address);
  Wire.write(command);
  Wire.write(value);
  int error = Wire.endTransmission();
 
  if( error == 0)
    return( true);
  else
    return( false);
}

By using the return value in a if-statement, you can do this:

if( command('G',122, 9))
{
  Serial.println( "Command was successfully sent");
}
else
{
  Serial.println( "O no, could not deliver the data");
}

bally94 · December 21, 2021, 6:28pm

Year! Thanks a lot! I have forgotten the dataReceived[] array to store the data...
And I think for starting with my project, I can send only in one direction. That is totally enough!

Here are my codes with error messages if it is not successfull.

Master:

#include <Wire.h>

//bool newData;
byte error;

int A = 0;
int B = 0;
int C = 0;
int D = 0;
int BUT = 0;

void setup() {
    Wire.begin(10);
    //Wire.setClock(300000);
    //Wire.onReceive(readCommand);
    
    Serial.begin(115200);
}

void loop() {
  /*if(newData == true){
    Serial.print("A = ");
    Serial.println(A);
    Serial.print("B = ");
    Serial.println(B);
    Serial.print("C = ");
    Serial.println(C);
    Serial.print("D = ");
    Serial.println(D);
    Serial.print("BUT = ");
    Serial.println(BUT);
    newData = false;
  }*/
  
    command('#',34, 9);
    command('H',185, 9);
    command('E',1, 9);
    command('G',212, 9);

  if(error != 0){
    if(error == 1){
      Serial.print("Command error: ");
      Serial.print(error);
      Serial.println(" Data too long to fit in transmit buffer!");
    }
    else if(error == 2){
      Serial.print("Command error: ");
      Serial.print(error);
      Serial.println(" Received NACK on transmit of address!");
    }
    else if(error == 3){
      Serial.print("Command error: ");
      Serial.print(error);
      Serial.println(" Received NACK on transmit of data!");
    }
    else if(error == 4){
      Serial.print("Command error: ");
      Serial.print(error);
      Serial.println(" Other error!");
    }
  }
  
}


void command(const char command, const byte value, const byte address){
  Wire.beginTransmission(address);
  Wire.write(command);
  Wire.write(value);
  error = Wire.endTransmission();
  
}


/*void readCommand(int bytes){
  char var;

  if(bytes == 2){
    var = Wire.read();

    switch(var){
      case 'A' : A = var;
                 break;
      case 'B' : B = var;
                 break;
      case 'C' : C = var;
                 break;
      case 'D' : D = var;
                 break;
      case '@' : BUT = var;
                 break;
    }
    newData = true;
  }
}*/

Slave:

#include <Wire.h>

volatile byte dataReceived[2];
bool newData;

int E = 0;
int F = 0;
int G = 0;
int H = 0;
int BUT = 0;

void setup() {
    Wire.begin(9);
    //Wire.setClock(300000);
    Wire.onReceive(readCommand);
    
    Serial.begin(115200);
}

void loop() {
  if(newData == true){
    Serial.print("E = ");
    Serial.println(E);
    Serial.print("F = ");
    Serial.println(F);
    Serial.print("G = ");
    Serial.println(G);
    Serial.print("H = ");
    Serial.println(H);
    Serial.print("BUT = ");
    Serial.println(BUT);
    newData = false;
  }
  
    /*command('@',255, 10);
    command('C',122, 10);
    command('A',05, 10);
    command('D',242, 10);*/
}


/*bool command(const char command, const byte value, const byte address){
  Wire.beginTransmission(address);
  Wire.write(command);
  Wire.write(value);
  int error = Wire.endTransmission();
  return(error);
}*/


void readCommand(int bytes){

  if(bytes == 2){
    dataReceived[0] = Wire.read();
    dataReceived[1] = Wire.read();

    switch(dataReceived[0]){
      case 'E' : E = dataReceived[1];
                 break;
      case 'F' : F = dataReceived[1];
                 break;
      case 'G' : G = dataReceived[1];
                 break;
      case 'H' : H = dataReceived[1];
                 break;
      case '#' : BUT = dataReceived[1];
                 break;
    }
    newData = true;
  }
}

What I did not understand is the volatile for the array of dataReceived. Why to declare a volatile byte and not only a byte?

Koepel · December 21, 2021, 7:41pm

Have you told which Arduino boards you use ?

When a compiler uses a global variable in the Arduino loop(), then it might optimize it in a way that it keeps the variable in a register and does not use the original variable in its memory location.
A interrupt can happen at any time, and the interrupt routine could change a global variable.
By using the 'volatile' keyword, the compiler will always use the original memory location of the variable.

Since the loop() runs over and over again, and dataReceived is an array, using the 'volatile' keyword is more theoretical. You need specific code and very bad luck for the 'volatile' to make a difference.

bally94 · December 21, 2021, 8:06pm

Oh sorry, I have forgotten it! An Arduino Uno and/or Mega. Does this make a difference?

Okay, that sounds interesting. I will let it be a volatile now, but how to know which variable should be better a volatile or not?

cattledog · December 21, 2021, 8:40pm

Any variable changed within an interrupt service routine and accessed in the main program should be declared volatile.

Wire.onReceive() is an interrupt.

bally94 · December 21, 2021, 9:02pm

All right! Now I have understood it correctly I think. Thanks a lot for that claryfication!

Topic		Replies	Views
A theoretical I2C Question General Guidance	21	317	December 8, 2025
Code for I2C Slave Peripherals Networking, Protocols, and Devices	58	909	April 28, 2024
Use I2C for communication between Arduinos Showcase	20	22491	September 6, 2020
How to set up a command-based communication using I2C Networking, Protocols, and Devices	32	5765	November 26, 2021
i2C missing some data Programming	17	5217	February 23, 2016

Receiving and writing data over I2C does not work

Related topics