I2C Issues

So I was trying to use on of my Arduinos to send a single byte to my other 2 Arduinos based on which they would modify their PWM values.

I checked all the hardware twice, it is working fine on its own. But my problem is that when I put the system together the slave Ardunios only read the first byte sent from the master the next one is not read at all.

Please help me, I have no clue what is going wrong!

I have attached a picture as well if it helps.

My code is as follows:
Master Code

// Wire Master Writer

// Writes data to an I2C/TWI slave device
#include <Wire.h>

void setup()
{
  Wire.begin(); // join i2c bus (address optional for master)
}



void loop()
{
  Wire.beginTransmission(5); // transmit to device #5
  Wire.write(1);              // sends one byte  
  Wire.endTransmission();    // stop transmitting

  Wire.beginTransmission(4); // transmit to device #5
  Wire.write(1);              // sends one byte  
  Wire.endTransmission();    // stop transmitting

   delay(1000);
  
  Wire.beginTransmission(5); // transmit to device #5
  Wire.write(2);              // sends one byte  
  Wire.endTransmission();    // stop transmitting
  
  Wire.beginTransmission(4); // transmit to device #5
  Wire.write(2);              // sends one byte  
  Wire.endTransmission();    // stop transmitting

}

Slave code

// Wire Slave Receiver

// Receives data as an I2C/TWI slave device

#include <Wire.h>
int i = 0;
int led = 13;
int data;
void setup()
{ pinMode(3, OUTPUT);
  pinMode(6, OUTPUT);
  pinMode(9, OUTPUT);
  pinMode(11, OUTPUT);
  pinMode(led, OUTPUT);
  Wire.begin(4);                // join i2c bus with address #4
  Wire.onReceive(receiveEvent); // register event
}

void loop()
{
}

// function that executes whenever data is received from master
// this function is registered as an event, see setup()
void receiveEvent(int howMany)
{  
     data =  Wire.read(); // receive byte as a character
    if(data == 1)
    {//Switch all motors on
         analogWrite(3, 255);
         analogWrite(6, 255);
         analogWrite(9, 255);
         analogWrite(11, 255);     
       
    }
    else if (data == 2)
    {// Switch all motors off
         analogWrite(3, 0);
         analogWrite(6, 0);
         analogWrite(9, 0);
         analogWrite(11, 0); 
    }
    else if (data == 3)
    {//Switch on motora at half speed
         analogWrite(3, 125);
         analogWrite(6, 125);
         analogWrite(9, 125);
         analogWrite(11, 125);     
       
    }  
    
   else if(data  == 4)
   { //Speed up one motor very slowly
    for (int i=0; i<=255; i++)
      {
      analogWrite(11, i);
      //analogWrite(motorPin2, i);
      delay(10);
      }
  
    delay(500); //Hold it!
  
    //Decrease Motor Speed from 255 -> 0
    for(int i=255; i>=0; i--)
      {
      analogWrite(11, i);
     // analogWrite(motorPin2, i);
      delay(10);
      }
  
    delay(500); //Hold it!
   }    
   
    else
    {// Blink LED for unidentified character
      digitalWrite(led, HIGH);   // turn the LED on (HIGH is the voltage level)
      delay(1000);               // wait for a second
      digitalWrite(led, LOW);    // turn the LED off by making the voltage LOW
      delay(1000);               // wait for a second
    }
}

If the Wire library has received data, and the I2C transfer is closed, an function is called, and that function can read the buffer of the Wire library.

That function is receiveEvent() and it is called from within an interrupt routine.
You can not use delay() in an interrupt routine. Also Serial.write() or other library functions will not always work.

Create a buffer or global variables.
Write the received data in those variables.
In the loop() function you can check those variables and do what needs to be done.
I would use a boolean flag. It would be set in the receiveEvent() to signal that something is received. The loop() function should test that flag, and clear it if the new data is processed.

You have already a global 'data' variable. You can use that for data and as a flag.
For example 0 or -1 to indicate that nothing is received.

Ummm I'm sorry im quite new to I2C and Arduino, I didnt really understand your answer, sorry could you explain:

a) What is a buffer? How do i create one?

b)What is a boolean flag?

c) Basically I need to update my PWM values very quickly, almost as soon as the data is received, will this be possible with the changes you have suggested?

Thanks for your reply though, I am trying to google the terms now!

  Wire.beginTransmission(4); // transmit to device #5

Wire.write(2);              // sends one byte 
  Wire.endTransmission();    // stop transmitting

Get the comments right, or omit them. Two out of three are wrong. In fact the third is wrong too, because the Wire.endTransmission() does the transmitting, not stops it.

  Wire.write(2);              // sends one byte

This comment is right, isn't it? It sends one byte, whose value is 2. Doesn't it?

Sorry about the comments, as I said I am new, I would gladly correct them if I knew otherwise.

nishant_91:
a) What is a buffer? How do i create one?
b)What is a boolean flag?
c) Basically I need to update my PWM values very quickly, almost as soon as the data is received, will this be possible with the changes you have suggested?

a) A buffer is inside the Serial library. The received data from the I2C bus is stored in that buffer (an array in ram). So you don't have to do anything with it. It is being done for you.

b) A boolean is a variable that is either 'true' or 'false'.

boolean myFlag;

myFlag = true;
if( myFlag)
  myFlag = false;

c) Yes, if the loop() function is doing nothing else, it wil be just as fast.

Erdin, thanks a lot!

One last question, when I store data in the buffer how do I read it, do I have to use string compare?

Also since you mentioned Serial library functions will not always work, will I be able to use the buffer?

Since you transmit only one data byte, you don't need a buffer.
I was only explaning how the Wire library works with the receiveEvent() function.

If you want to store a complete string, you can start thinking about a buffer.

Could you move the code for the motors into the loop() function.
And test if 'data' has new data.
Upload the new sketch, so we can take a look at it.

Sending (debug) message to the serial monitor should also be done in the setup() and loop() function, but not in the receiveEvent() function.

About buffers:
A buffer is an array, to store a bunch of data.

char buffer[100];    // a 'buffer' to store 100 characters.
int buffer2[50];  // a 'buffer' to store 50 integers.

for (int i=0; i<10; i++)
  buffer[i] = 'A';      // fill buffer with ten 'A'

Buffers are used a lot together with interrupt routines.
I think the Wire library has 5 internal buffers for the I2C to work smooth. The Serial library has also buffers. Those buffers are created in the library. You don't have to know about it, they only make things run faster and smooth.

Thanks for the explanation Erdin, I do not have access to my lab for the next few hours, I will get back to you as soon as I am able to try out your suggestions!

Sorry one more question,I know all the Arduinos need to have a common ground, do the 5V pins also need to be connected together in order for I2C to be successful?

No, if every Arduino is powered (with a power supply or via usb) you only have to connect all the GND and SDA and SCL.

So I changed the slave code to the following:

#include <Wire.h>
int i = 0;
int led = 13;
boolean datain;
int data;
void setup()
{ pinMode(3, OUTPUT);         
  pinMode(6, OUTPUT);
  pinMode(9, OUTPUT);
  pinMode(11, OUTPUT);
  pinMode(led, OUTPUT);
  Wire.begin(4);                // join i2c bus with address #4
  Wire.onReceive(receiveEvent); // register event
}

void loop()
{
  if (datain)
   { datain = false; 
     switch (data)
     {
       case 1:              //Switch all motors on
       analogWrite(3, 255);
       analogWrite(6, 255);
       analogWrite(9, 255);
       analogWrite(11, 255); 
       break;
       
       case 2:              // Switch all motors off
       analogWrite(3, 0);
       analogWrite(6, 0);
       analogWrite(9, 0);
       analogWrite(11, 0);
       break;
       
       case 3:              // Switch all motors off
       analogWrite(3, 125);
       analogWrite(6, 125);
       analogWrite(9, 125);
       analogWrite(11, 125);
       break;
       
       case 4:
        for (int i=0; i<=255; i++)
        {
          analogWrite(11, i);
          delay(10);
        }
        delay(500); //Hold it!
          //Decrease Motor Speed from 255 -> 0
        for(int i=255; i>=0; i--)
        {
          analogWrite(11, i);
          delay(10);
        }
        delay(500); //Hold it!
        break;
        
        default:
        // Blink LED for unidentified character
        digitalWrite(led, HIGH);   // turn the LED on (HIGH is the voltage level)
        delay(1000);               // wait for a second
        digitalWrite(led, LOW);    // turn the LED off by making the voltage LOW
        delay(1000);               // wait for a second
        break;
     }
  }
}
void receiveEvent(int howMany)
{    datain = true;
     data =  Wire.read(); // receive byte
}

But there is still no improvement, it still only detects the first transmission and starts the motors at full speed!

If I switch the first byte in the Master code to stop all the motors and the one after the delay to start the motors then all the motors remain stationary.

I can only conclude that for some reason only the first byte transmitted to both the slaves is being read and the second is ignored, any idea what I should try/do next?

It looks good at first glance.

Can you add 'volatile' in front of "boolean datain;" and "int data;".
Every variable that is used in an interrupt routine and in the normal code should be 'volatile'. It tells the compiler that the variable can change in the interrupt routine.

You can test in the Master if the I2C transfer was succesful.
If the Wire.endTransmission() returns a value other than 0, you could send a message to the serial monitor that the I2C transfer failed.

All right so I changed the Master code to:

// Wire Master Writer

// Writes data to an I2C/TWI slave device
#include <Wire.h>

int x =0;
void setup()
{
  Serial.begin(9600);
  Wire.begin(); // join i2c bus (address optional for master)
}



void loop()
{
  Wire.beginTransmission(5); //Initialize I2C bus
  Wire.write(1);              // Configure internal variables  
  x = Wire.endTransmission();    //Transmit Byte
  Serial.print( " Sent 1 to 5 , result= ");
  Serial.println(x);

  Wire.beginTransmission(4); //Initialize I2C bus
  Wire.write(1);              // Configure internal variables  
  x = Wire.endTransmission();     //Transmit Byte
   Serial.print( " Sent 1 to 4 , result= ");
   Serial.println(x);
   delay(1000);
  
  Wire.beginTransmission(5); //Initialize I2C bus
  Wire.write(2);              // Configure internal variables  
   x = Wire.endTransmission();     //Transmit Byte
   Serial.print( " Sent 2 to 5 , result= ");
   Serial.println(x);
   
  Wire.beginTransmission(4); //Initialize I2C bus
  Wire.write(2);              // Configure internal variables 
  x = Wire.endTransmission();     //Transmit Byte
   Serial.print( " Sent 2 to 4 , result= ");
   Serial.println(x);
}

And the output from the serial monitor is attached, It seems the transmission is working, but no change in the response

I must have missed something.
Perhaps the voltage drops if the slaves activate something.
Perhaps the pull-up resistors for the I2C bus are missing.
Perhaps the ground (GND) connections between the Arduinos is not good.
Can you confirm that it is all done well ?

All GND connections are good,

Im using 4.7 kilo ohm resistors for pull ups connected to the 5V pin.

I dont know about voltage drops, so I removed all the motors and left only one to check.....

And I think the circuitry is ok, because when I change the first byte that is transmitted the motor changes its speed accordingly