i2C missing some data

Hello everyone.

I think questions about i2C was ask several times. I've looking for a solution here and on google but I don't find it or there is something I miss understand.

Classical situation :
I get 2 Arduino
Linked together on i2C
I need to send integer data from slave to master with RequestFrom method.

code on MASTER :

void SLAVE_GET( int type, int way ){
  SLAVE_SET(type*10+way);              // sub function to set slave on desired data
  delay(100);

  Wire.requestFrom(8, 2);                   // request 2 bytes
  Serial.println(Wire.available());          // show how much bytes availables
  while( Wire.available() )
  {
    Serial.println(Wire.read());              // show every bytes
  }
}

code on SLAVE

void SEND(){
  Serial.println( "i2C === SEND" );         // show that Master request something
  int val = 720;                                 // just a test value
  Serial.println( val );                         // show what are going to be send
  Wire.write( val );                            // send val.
  }

results :

instead of 720, master show 208 and 255.

720 = 0000001011010000
208 = 0000000011010000
255 = 0000000011111111

that meaning master only get ONE of the 2 bytes and the second (or the first, what is the order ?) the second is "put" on 255 value

I don't understand why.

Please show us the complete sketch for both the Master and the Slave and tell on which Arduino boards this was used. Did you connect the GND pins ?

The onReceive() and onRequest() are interrupt handlers. Don't use any delay() or Serial function in it. I know that is in the examples, but it is not okay.

The Wire.requestFrom() returns the number of received data. If you request two, it is possible to check for two, and that is the only way that the data is valid.

  int n = Wire.requestFrom(8, 2);                   // request 2 bytes
  if ( n == 2 )                       // two bytes received ?
  {
    Serial.println(Wire.read());
    Serial.println(Wire.read());
  }
  else
  {
    Serial.println("Bad I2C");
  }

SLAVE_GET is just a function, not related to any interrup
The retrieving process rolls in two times :

  • SLAVE_SET to define a value on slave arduino to set the desired value
    (some delay to be sure that desired value has been take into account.
  • RequestFrom to get desired value.

MASTER

void setup() {
  Serial.begin(9600);
  Wire.begin(0x3F);        // i2C LCD
  lcd.begin(16,2);          // LCD 16x2
  lcd.backlight();           // LCD on
}

void SLAVE_SET( unsigned int val ){
  Wire.beginTransmission(8);           // transmit to device #8
  Wire.write( val );                        // sends val to slave
  Wire.endTransmission();               // stop transmitting
}

void SLAVE_GET( int type, int way ){
  SLAVE_SET(type*10+way);
  delay(100);
  Wire.requestFrom(8, 2);
  Serial.println(Wire.available());
  while( Wire.available() )
  {
    Serial.println(Wire.read());
  }
}

SLAVE

void setup() {
  Wire.begin(8);                    // join i2c bus with address #8
  SPI.begin();                       // SPI for the use of MCP
  Wire.onReceive(GET);          // réception ordre
  Wire.onRequest(SEND);        // envoi information
  Serial.begin(9600);              // start serial for output
}

void GET(int howMany)
{
  Serial.println( "i2C === GET" );
  int data = Wire.read();           // receive byte as an integer
  if( data < 100 )                     // all data received below 100 is to define what value to send next
  {
    Serial.print( " : REQUEST : " );   Serial.println(data);    // show what was defined
    dataRequest = data;
  }
  else                                    // all data more than 100 is to define behaviour function
  {
    int way = int(data/1000);
    int val = data - way*1000;
    Serial.print( " : DEFINE  : way:" ); Serial.print(way); Serial.print( " v:" ); Serial.println(val);
    FAN_SET_SPEED( way, val );
  }
}

void SEND(){
  Serial.println( "i2C === SEND" );            // show that slave is going to send value
  int val = 720;                                    // just a test value
  Serial.println( val );                            // show what are going to be sent
  Wire.write( val );                               // send test value.
}

Now, there is the complete code (without third part, because my worked about 2 months)

Do you think Serial.println on SEND function could skip some byte ?
It should be better to put Serial.print at the end ? AFTER Wire.write ?

Master:

  • The Master is set as Slave at 0x3F. Is there another Master ? When does that other Master become active ? What kind of protocol is used to avoid collision on the I2C bus ? What do you do when there is a collision ?
  • Why is there a delay of 100ms in SLAVE_GET ?
  • You do not check how many bytes are received. If you request 2 bytes, please check if you received 2 bytes.

Slave:

  • The most used names for the interrupt handlers are: "requestEvent" and "receiveEvent". I prefer it if those names are used, it makes it easier to read.
  • It is not a complete sketch, I want to know the code of FAN_SET_SPEED.
  • The GET() and SEND() use the Serial library. Please remove calls to the Serial library completely. The GET() and SEND() are called as (part of) an interrupt handler. The Serial library uses interrupts itself and it can delay for a long time if the transmit buffer is full. It will never be reliable as long as a function of the Serial library is called.
  • It is not a complete sketch, I want to know if 'dataRequest' is volatile.

Hardware:

  • Are the GND of the Arduinos connected ?
  • Are both Arduino boards running at 5V, or is there a mix of 5V and 3.3V ?
  • What else is connected to the I2C bus ?
  • What is the total pullup resistor value ? Many modules have onboard pullup resistors. If the total pullup resistor value (all the pullup resistors parallel) is too low, then I2C is no longer reliable. The maximum current to pull a line low is 3mA. Thus the lowest value for a 5V I2C bus is 1k6.

Not at all, 0x3F is LCD i2C address.

  • I never know that there could be some collision. when there are TWO master ?
  • I understand why. But there are only ONE master, an Arduino Uno.
  • Delay of 100ms is put in place to be sure that slave had take into account the desired value by SLAVE_SET
  • I check number of byte and I get 2 bytes available. but the second is 255

-> is there a difference between
int NbBytesReceived = RequestFrom(8, 2 );
int NBBytesReceived = Wire.available();

dataRequest is an integer.

Take care about complete code
Slave has 300 lines, a quiet short
Master has more than 1000 lines....

All arduino is linked to GND and Vcc.
i2C LCD is linked on 5V via a 78L05 + capacitor

-> analogic 4 and 5 had to be set to OUTPUT with pinmode ?
-> may I have to put a pullup resistor ?

BayBus-V1.7-master.zip (10.1 KB)

Baybus-V1.7-slave.zip (3.11 KB)

This is not going in the right direction. Now I think the schematic is wrong as well, and I still don't understand how many Arduino boards you have connected to the I2C bus.

I can't read all the labels in the schematic.
The 7805 has no capacitors at input and output ? That makes it an oscillator instead of a voltage regulator.
Why is diode D4 used ? It does not protect something, it makes it less reliable.
I hope that the capacitor C2 and C3 are there for a good reason.
I can't find how transistor Q9 is connected.

In the schematic I see two Arduino Nano boards. They are both I2C Slaves ? There is another Arduino Uno connected to the I2C bus that is a Master ? And the Arduino Nano for the display is Master for the other Arduino Nano ? Then there are two Masters, and there will be collisions. If you don't know how to deal with collisions, you should avoid having two Masters.

If a global variable is written in an interrupt routine, that variable must be 'volatile'.
A missing 'volatile' could be the problem.

volatile int dataRequest;

The return value of Wire.requestFrom() is the same Wire.available(). But after the first Wire.read(), the Wire.available() is decremented. You can therefor do this as well:

  Wire.requestfrom( 8, 2);
  if( Wire.available() == 2)
  {
    int first = Wire.read();
    int second = Wire.read();
  }
  else
  {
    Serial.println("I2C error");
  }

The delay of 100ms is not needed. In the past a short delay of 1ms (or less) was needed, but that should be fixed.

If you call Wire.begin(), then the Wire library sets the pins for I2C. They should not be set as output.

The best way for pullup resistors is 4k7 pullup resistors at the Master. If there are no wires and very short connections, then 10k is okay. If there wires are used for SDA or SCL then perhaps 3k3 or 2k2 pullup resistors are needed. Wires longer than 50cm can be a problem. The SDA and SCL may never be next to each other in a flat ribbon cable.
I'm talking about the total pullup value. That means all the pullup resistors parallel.

You still use Serial library in the interrupt handles for onReceive and onRequest. You have to stop doing that.

I will no longer answer this topic if I have to repeat over and over again how to fix it.

If everything is fixed, then we have to take a look what is transmitted and what is received. The Slave Wire.write( 'an integer value' ) returns a single byte to the master. The Wire library requires a that just one Wire.write() in the onRequest handler is used. That means you have to use a pointer to the integer, or make a buffer and fill it with highByte() and lowByte(). The master can glue them together with the same thing, also with a pointer to an integer and using Wire.readBytes() or do the opposite of highByte() and lowByte() which is word().
I use a 'struct' to transfer data via I2C.
Another option is I2C_Anything : Gammon Forum : Electronics : Microprocessors : I2C - Two-Wire Peripheral Interface - for Arduino

I'll going to check all pullup resistor.
I'm doing no change to script because I'm haven't hardware part beside me.
I'll put "volatile" on variable related to interrupt function.
I've not put any capacitor at the input of 7805. I'll try to put one.
C2-C3 is to filter re-bounce on rotative selector
Q9 is just a transistor to drive an strong output
all arduino provide 5V. The diodes is to protect all each other.

concerning master/slave
-1 Arduino master
-1 Arduino slave
-1 i2C LCD slave
-> I'll put pullup resistor
-> I'll remove Serial.print

Hitsugaya81:
-1 Arduino master
-1 Arduino slave
-1 i2C LCD slave

The third one requests data from the second one ? Then the third one is also a Master. That will cause a lot of problems on the bus.

never.
Arduino Master is the ONLY master. LCD DONT get any order from slave ardino.

OK. My problem is fixed now.
Pull-up resistor has made no change at all.
I hope it increase the quality of signals.
Serial.print has no impact on the transmission.
BUT... I remove it now to have a clear code (and a light serial output).

Serial.print is a good solution but a temporary way to debug. But at the end : remove all of them.

To increase more : I put "volatile" on all variable related to interrupt functions.
Solution was the use of buffer, as you suggest it.

MASTER :

void SLAVE_SET( unsigned int val ){
  Wire.beginTransmission( SLAVE_ADDRESS );
  I2C_writeAnything ( val );                                     // I'm going to replace it by a buffer.
  Wire.endTransmission();
}
void SLAVE_GET( int type, int way ){
  SLAVE_SET(type*10+way);
  int nb = Wire.requestFrom(8, 2);
  uint16_t value = Wire.read() | Wire.read()<<8 ; // uint16_t to suit 2 bytes.
  Serial.println(value);                                      // at the end, should be remove.
}

SLAVE :

void requestEvent(){
  int value = DATA[i_request][j_request];    // previously define by "SLAVE_SET" on master
  uint8_t buffer[2];                                    // buffer of 2 of 8 bites, integer. BUT limited to 65535
                                                               // buffer[4] = 4 x 8 bites = 4 octects limited to 4 294 967 296
  buffer[0] = value & 0xff;
  buffer[1] = value >> 8;                           // binary shift by 8 to get 2nd byte.
//buffer[2] = value >> 16;                         // binary shift by 16 to get 3th byte.
//buffer[3] = value >> 24;                         // binary shift by 24 to get 4th byte.

  Wire.write(buffer,2);                                // 2 replaced by 4 if value > 65535
  Serial.println("request 720");                    // at the end, should be remove.
}

anyway, I made a misstake. It's the reason why you has been confused :
on Master Setup :

Setup(){
  Serial.begin(9600);
  Wire.begin(0x3F);
  lcd.begin(16,2);
  lcd.backlight();
}

0x3F is the adresse of i2C LCD interface.
So the right "initialisation" in view to communicate with i2C LCD interface is

Wire.begin();

This way, Master IS a master ( Wire.begin(byte _ADDRESS); define a SLAVE ).

Every Slave can be a Master, just call Wire.beginTransmission(), Wire.write() and Wire.endTransmission().

A Master can be a slave, if the address if used a parameter with Wire.begin().

I'm still confused which is one is the Master and which Arduino is doing what and what is the I2C LCD interface.


This is an LCD screen with and I2C interface to drive it.

Every Slave can be a Master
A Master can be a slave

The power of the i2C protocol.

On my schem, Nano_m is the Master, Nano_S is the Slave

But there are only ONE master, an Arduino Uno.
Nano_m is the Master,
So how many Masters do you have ?

hummm...
no... list of i2C devices :
Nano_M : Arduino Nano, the master
Nano_S : Arduino Nano, setting as slave at 0x08 address
i2C_LCD : i2C Interface driving an LCD 16x2, slave at 0x3F address

You really have written in Reply #4 : But there are only ONE master, an Arduino Uno.

So you gave the Master the same slave address as the I2C display. I don't even know what that will do.

Creating the LiquidCrystal_I2C object is done in AFFICHEUR.h for the Master sketch.
That is not nice.
And the AFFICHEUR.h has no defines to use it just once.
I prefer that an object is created in the main *.ino file.
I also prefer that variables are declared in the main *.ino file. They are now in Variables.h

Yes, it's was a mistake. I've fix it by replacing "Wire.begin(0x3F)" by "Wire.begin()"

Okay, okay, you are on the right track now.
Do you want to transfer other variables ? The I2C_Anything can be useful. I don't use it, I use a 'struct'. If you only transfer an integer, you don't have to use either of them.
Be careful that the Master and Slave use exactly the same data. When I add something to my 'struct' in the Slave and forget to do that in the Master, then the data is a big mess.

If global data is changed in the onReceive or onRequest, it must be volatile.
If that global data is used in the loop(), the interrupts must be turned off if the data is longer than a byte. If for example the loop() uses an integer, an interrupt could occur right in the middle when retrieving the two bytes of the integer from memory. That means that one byte was old, and the other byte was changed by onReceive, and the loop() uses an integer with non-matching bytes.

'volatile' means that the compiler really writes the data to the memory location. That is not normal, because the compiler can do many optimizations and keep variables in registers.