I2C data being lost between master and slave

Hi, I’m having a challenge sending and receiving data from sensors over I2C and would love some input to my code.

Project: I am running a n-scale train layout and are using multiple arduino nanos to control junctions (servos and relays) and signals (LEDs) via hall effect sensors. pictures of the project here: https://www.instagram.com/model.train.project/

Challenge: data from sensors and commands to and from the master nano work for the most part, however sensors fail to trigger and data is lost to and from the master regularly.

  • If a train passes over the sensor too quickly (only average speed), the sensor fails to trigger. Also data sent back from the master to multiple slaves on occasion fail to make all the servos/ relays operate as intended. the servos and relays are operated on seperate 5v from the nanos.
  • I think it may have something to do with how I have programed the cases (on both the slave and master) as it appears to happen mostly with cases further down the program. Could this be due to a timing issue with how data is sent recieved from the i2C?
    Slave code in the comments as I have reached the max char limit
// I2C Master Sensor Reader

// Reads data from each I2C slave device and returns command to slave



#include <Wire.h>
int addressI2C;
byte dataToI2C;
byte dataFromI2C;



void setup() {
  Wire.begin();
  Serial.begin(9600);
  Serial.print("ready:");

}

void loop() {
  // ----  COLLECT INCOMING SIGNALS
  // ------------------------------------  COLLECT INCOMING SIGNALS-----------------------------------------

  // I2C REQUEST
  ////from local 1 /// 2 sensors
  static unsigned long prevTime = 0;
  if (millis() - prevTime > 5) { ///was 25ms
    addressI2C = 1;
    Wire.requestFrom(addressI2C, 2);
    prevTime = millis();
    dataFromI2C = Wire.read();
    if (dataFromI2C != 0) Serial.println(dataFromI2C);
  }
  I2CRequest();

  ////form local 2

  if (millis() - prevTime > 5) {
    addressI2C = 2;
    Wire.requestFrom(addressI2C, 2);
    prevTime = millis();
    dataFromI2C = Wire.read();
    if (dataFromI2C != 0) Serial.println(dataFromI2C);
  }
  I2CRequest();

  ////form local 7 (30)

  if (millis() - prevTime > 5) {
    addressI2C = 30;
    Wire.requestFrom(addressI2C, 2);
    prevTime = millis();
    dataFromI2C = Wire.read();
    if (dataFromI2C != 0) Serial.println(dataFromI2C);
  }
  I2CRequest();

  ////from local 9

  if (millis() - prevTime > 5) { ///was 25ms
    addressI2C = 9;
    Wire.requestFrom(addressI2C, 2);
    prevTime = millis();
    dataFromI2C = Wire.read();
    if (dataFromI2C != 0) Serial.println(dataFromI2C);
  }
  I2CRequest();





}


void sendDataViaI2C() {
  Wire.beginTransmission(addressI2C);
  Wire.write(dataToI2C);
  Wire.endTransmission();
}

void I2CRequest() {
  ////////// request data from slaves and respond////////////////////////////////


  if (dataFromI2C != 0) {

    switch (dataFromI2C) {


      case 11: //// main line station




        //M/// //branch line bridge
        addressI2C = 1; dataToI2C = 31; sendDataViaI2C(); //no movement
        delay(20);
        addressI2C = 1; dataToI2C = 81; sendDataViaI2C(); //Blackline bridge
        addressI2C = 1; dataToI2C = 33; sendDataViaI2C(); //moves
        delay(20);
        addressI2C = 4; dataToI2C = 35; sendDataViaI2C();//moves
        addressI2C = 1; dataToI2C = 35; sendDataViaI2C();//moves

        addressI2C = 11; dataToI2C = 31; sendDataViaI2C(); //no movement
        //other
        addressI2C = 3; dataToI2C = 31; sendDataViaI2C(); //no movement
        addressI2C = 2; dataToI2C = 82; sendDataViaI2C(); //whiteline Transition
        addressI2C = 2; dataToI2C = 87; sendDataViaI2C(); //blackline goods on



        ////L open////
        // Serial.println(" l0z");
        addressI2C = 6; dataToI2C = 30; sendDataViaI2C();
        addressI2C = 8; dataToI2C = 34
                                    ; sendDataViaI2C();


        break;

      case 12: ////transition station 1
        addressI2C = 3; dataToI2C = 82; sendDataViaI2C(); //transition line stop

        // case 21: //main line bridge --old
        //p///// main line bridge
        addressI2C = 1; dataToI2C = 30; sendDataViaI2C();
        delay(20);
        addressI2C = 4; dataToI2C = 30; sendDataViaI2C();
        addressI2C = 11; dataToI2C = 30; sendDataViaI2C();
        //other switches
        delay(20);

        addressI2C = 1; dataToI2C = 80; sendDataViaI2C(); //whiteline bridge
        addressI2C = 2; dataToI2C = 83; sendDataViaI2C(); //Blackline Transition
        addressI2C = 2; dataToI2C = 84 ; sendDataViaI2C(); //whiteline main

        break;

      case 21: ///branch line station

        ///o/////transition line main
        addressI2C = 1; dataToI2C = 31; sendDataViaI2C();
        addressI2C = 4; dataToI2C = 31; sendDataViaI2C();
        addressI2C = 4; dataToI2C = 32; sendDataViaI2C();
        addressI2C = 1; dataToI2C = 32; sendDataViaI2C();
        addressI2C = 3; dataToI2C = 31; sendDataViaI2C();
        addressI2C = 11; dataToI2C = 30; sendDataViaI2C();
        //other switches
        // addressI2C = 1; dataToI2C = 34; sendDataViaI2C();
        addressI2C = 4; dataToI2C = 35; sendDataViaI2C();

        addressI2C = 1; dataToI2C = 80; sendDataViaI2C(); //whiteline bridge
        addressI2C = 2; dataToI2C = 82; sendDataViaI2C(); //Whiteline Transition
        addressI2C = 2; dataToI2C = 85 ; sendDataViaI2C(); //Whiteline Transition mainline off
        addressI2C = 3; dataToI2C = 83; sendDataViaI2C(); //transition line go
        break;

      case 71: //outer tunnel track Close
        ///L////outer tunnel track Close
        addressI2C = 6; dataToI2C = 31; sendDataViaI2C();
        addressI2C = 8; dataToI2C = 35; sendDataViaI2C();


        /////P////
        addressI2C = 1; dataToI2C = 30; sendDataViaI2C();
        addressI2C = 4; dataToI2C = 30; sendDataViaI2C();
        addressI2C = 11; dataToI2C = 30; sendDataViaI2C();
        //other switches
        delay(20);
        addressI2C = 1; dataToI2C = 80; sendDataViaI2C(); //whiteline bridge
        addressI2C = 2; dataToI2C = 83; sendDataViaI2C(); //Blackline Transition
        addressI2C = 2; dataToI2C = 84 ; sendDataViaI2C(); //whiteline main
        addressI2C = 3; dataToI2C = 82; sendDataViaI2C(); //transition line stop

        break;


      case 91: ////uphill main line to tunnel

        //outer tunnel track



        addressI2C = 1; dataToI2C = 31; sendDataViaI2C(); //no movement
        addressI2C = 1; dataToI2C = 33; sendDataViaI2C(); //moves
        addressI2C = 4; dataToI2C = 35; sendDataViaI2C();//moves
        addressI2C = 1; dataToI2C = 35; sendDataViaI2C();//moves
        addressI2C = 3; dataToI2C = 31; sendDataViaI2C(); //no movement
        addressI2C = 11; dataToI2C = 31; sendDataViaI2C(); //no movement
        addressI2C = 1; dataToI2C = 81; sendDataViaI2C(); //Blackline bridge
        addressI2C = 2; dataToI2C = 82; sendDataViaI2C(); //whiteline Transition
        addressI2C = 2; dataToI2C = 87; sendDataViaI2C(); //blackline goods on


        break;


    }
    dataFromI2C = 0;


  }
}

Slave #1 code

[code]Slave #1
//-------------------------------------//

#include <Wire.h>
#include <Servo.h>

// I/O PINS
#define JUNCTION_EN 4
Servo J1;
Servo J2;
Servo J3;

#define RELAY_A1 9  //Relay
#define RELAY_A2 10

#define SIGNAL_A1_GREEN 15 //Signal light
#define SIGNAL_A1_RED 16  //Signal light

#define SENSOR_A1 14//main station sensor

#define SENSOR_A2 17//tranistion station sensor 1

// VARIABLES

byte dataFromI2C;
byte dataToI2C;
unsigned long  millisJunction;
static unsigned long prevTime = 0;

bool SENSOR_A1_Trigger = false;
//  Open_block_A1 = true; //not implimented yet

bool SENSOR_A2_Trigger = false;
//  Open_block_A1 = true; //not implimented yet

// Variable for received data
int rd;


void setup() {

  // Initializing I2C
  Serial.begin(9600); // for debugging
  Wire.begin(1); // set address #1
  Wire.onReceive(receiveI2C);
  Wire.onRequest(requestI2C);
  Serial.print("Ready LOCAL 1: ");



  // ------------------------Initializing PINs----------------------------

  // Initializing PINs
  pinMode(JUNCTION_EN, OUTPUT);
  J1.attach(7);
  J2.attach(8);
  J3.attach(11);

  //Outputs

  pinMode(RELAY_A1, OUTPUT);
  pinMode(RELAY_A2, OUTPUT);

  //Inputs
  pinMode(SENSOR_A1, INPUT);
  pinMode(SENSOR_A2, INPUT);

  // Initializing others
  pinMode(SIGNAL_A1_RED, OUTPUT); 
  pinMode(SIGNAL_A1_GREEN, OUTPUT);


  // -----------------------SET DEFAULT POSITIONS --------------------

  J1.write(125);
  J2.write(62);
  J3.write(95);

  digitalWrite(JUNCTION_EN, HIGH);
  delay(600);
  digitalWrite(JUNCTION_EN, LOW);
  //millisJunction = millis();


  //relay state White line is high, black line is low
  digitalWrite(RELAY_A1, HIGH); //White line on
  digitalWrite(RELAY_A2, HIGH); // White line on


  digitalWrite(SIGNAL_A1_RED, HIGH);
  digitalWrite(SIGNAL_A1_GREEN, LOW);



}



void receiveEvent() {
  // read one character from the I2C
  rd = Wire.read();
  // Print value of incoming data
  Serial.println(rd);


}
void loop() {

  //sensor read
  sensorRead();


  // ---------------------------COMMAND PARSING
  if (dataFromI2C != 0) {
    switch (dataFromI2C) {

      //--------------------------- RESET
      case 99: resetFunc(); break;

      // -------------------------JUNCTION 1 (urb:2J1 )
      case 30: J1.write(120);

        digitalWrite(JUNCTION_EN, HIGH);
        millisJunction = millis();
        digitalWrite(SIGNAL_A1_RED, LOW);
        digitalWrite(SIGNAL_A1_GREEN, HIGH);
  

        break;

      case 31: J1.write(62);

        digitalWrite(JUNCTION_EN, HIGH);
        millisJunction = millis();

        digitalWrite(SIGNAL_A1_RED, HIGH);
        digitalWrite(SIGNAL_A1_GREEN, LOW);
 

        break;

      //URB 2J2
      case 32: J2.write(62);

        digitalWrite(JUNCTION_EN, HIGH);
        millisJunction = millis();
        digitalWrite(SIGNAL_A1_RED, LOW);
        digitalWrite(SIGNAL_A1_GREEN, HIGH);


        break;

      case 33: J2.write(100);

        digitalWrite(JUNCTION_EN, HIGH);
        millisJunction = millis();

        digitalWrite(SIGNAL_A1_RED, HIGH);
        digitalWrite(SIGNAL_A1_GREEN, LOW);
   

        break;

      //URB 2J3
      case 34: J3.write(55);
        //   delay(50);
        digitalWrite(JUNCTION_EN, HIGH);
        millisJunction = millis();
        digitalWrite(SIGNAL_A1_RED, LOW);
        digitalWrite(SIGNAL_A1_GREEN, HIGH);
  

        break;

      case 35: J3.write(125);
        //   delay(50);
        digitalWrite(JUNCTION_EN, HIGH);
        millisJunction = millis();

        digitalWrite(SIGNAL_A1_RED, HIGH);
        digitalWrite(SIGNAL_A1_GREEN, LOW);


        break;

      // TRANSITION BRIDGE
      case 80: //white line
        digitalWrite(RELAY_A1, HIGH);
        digitalWrite(RELAY_A2, HIGH);
        break;

      case 81:   //Black Line
        digitalWrite(RELAY_A1, LOW);
        digitalWrite(RELAY_A2, LOW);
        break;



    }

    dataFromI2C = 0;

  }

  if (millis() > (millisJunction + 600)) digitalWrite(JUNCTION_EN, LOW);
}

// ----------- FUNCTIONS ----------- //

void sensorRead() {

  SENSOR_A2_Trigger = digitalRead(SENSOR_A2);
  if (SENSOR_A2 _Trigger != 1) {
    dataToI2C = 12;


  }
  else dataToI2C = 0;

  SENSOR_A1_Trigger = digitalRead(SENSOR_A1 );
  if (SENSOR_A1_Trigger != 1) {
    dataToI2C = 11;
    //  Serial.println(dataToI2C);

  }
  else dataToI2C = 0;



}

void receiveI2C(int howMany) {
  while (Wire.available() > 0) {
    dataFromI2C = Wire.read();
    // read one character from the I2C

    // Print value of incoming data
    Serial.println(dataFromI2C);
  }
}


void requestI2C() {
  Wire.write(dataToI2C); // respond with message to master

}

[/code]

The I2C bus was designed by Philips to be used inside a television on the same pcb board. It was never meant to be used in a cable or with longer wires. That means that as soon you attach a cable to the I2C bus, then you can run into trouble very quickly.
The train and relays and junctions might cause sparks or voltage peaks or current peaks. It is not the ideal situation for a (weak) I2C bus.

I can not see your photos because I have no Instagram account.
Does you photos show what kind of cable you use ? What the value of the pullup resistors is ? How the grounding is ?

You should not use a "Serial" function inside an interrupt. The onReceive and onRequest handlers are called from an interrupt, so they are interrupt functions.

Add the keyword 'volatile' to a variable that is used both in a interrupt and in the loop(). The interrupt functions are the onReceive and onRequest handlers, so that is for the variables 'dataFromI2C' and 'dataToI2C'.
The keyword 'volatile' tells the compiler that the value can be changed in a interrupt, and that the compiler should not keep its value in a register or so, but actually read its value from its memory location.

Never add something to a millis value.
This could go wrong:

if (millis() > (millisJunction + 600))

Always use:

if( millis() - previousMillis >= 600)

[ADDED]
I took a better look at both skeches for the data-path.
In the Slave you use the same variable for two sensor triggers, and immediate make it zero if one of them is not triggered. You never know if that trigger has been passed on to the Master.
In the Master there are too many delays. It might take a while before reading data from the Slave. It's a miracle that you can see a trigger at all.

When a trigger occurs, then you have to be sure that the information will be read by the Master.

This is in addition to what Koepel already wrote.

If you look at the following routine:

void sensorRead() {

  SENSOR_A2_Trigger = digitalRead(SENSOR_A2);
  if (SENSOR_A2 _Trigger != 1) {
    dataToI2C = 12;


  }
  else dataToI2C = 0;

  SENSOR_A1_Trigger = digitalRead(SENSOR_A1 );
  if (SENSOR_A1_Trigger != 1) {
    dataToI2C = 11;
    //  Serial.println(dataToI2C);

  }
  else dataToI2C = 0;



}

Don't expect pin SENSOR_A2 ever to be read by the master. Even if the pin is pulled low, the variable dataToI2C will be set but just to be overwritten by the next few lines. The result in the variable will almost ever be the value of the SENSOR_AI pin. Only in the extremely seldom case that the I2C request is received exactly while the pin value of SENSOR_A1 is still being read, then the SENSOR_A2 value will be sent.

Thanks Koepel for the reply.
Yes I realize I am pushing the limits of what i2C considering what it was designed for, and this may cause issues. Most of the cables between the nanos are about 3cm, but a couple are 6cm. I have two 10k pull up resistors on the i2C. They are all grounded by a heavier gauge wire (standard 240v house wire)

I can not see your photos because I have no Instagram account.
Does you photos show what kind of cable you use ? What the value of the pullup resistors is ? How the grounding is ?
some directlinks to pictures (that should work without an account)

https://www.instagram.com/p/CFG-CYPHCFN/

https://www.instagram.com/p/CBgyFZwhmlo/

You should not use a "Serial" function inside an interrupt. The onReceive and onRequest handlers are called from an interrupt, so they are interrupt functions.
Add the keyword 'volatile' to a variable that is used both in a interrupt and in the loop(). The interrupt functions are the onReceive and onRequest handlers, so that is for the variables 'dataFromI2C' and 'dataToI2C'.
The keyword 'volatile' tells the compiler that the value can be changed in a interrupt, and that the compiler should not keep its value in a register or so, but actually read its value from its memory location.

Never add something to a millis value.
This could go wrong:

if (millis() > (millisJunction + 600))

Always use:

if( millis() - previousMillis >= 600)

That you for the insight on interrupt functions and millis. I was not aware of them.

[ADDED]
I took a better look at both skeches for the data-path.
In the Slave you use the same variable for two sensor triggers, and immediate make it zero if one of them is not triggered. You never know if that trigger has been passed on to the Master.

  • I earlier had serial.print the variable sent from the slave as well as that receive by the master to confirm that they have been sent. But I can see from your comment above I need to remove them.

In the Master there are too many delays. It might take a while before reading data from the Slave. It's a miracle that you can see a trigger at all.

Thanks. will remove them.

pylon:
This is in addition to what Koepel already wrote.

If you look at the following routine:

void sensorRead() {

SENSOR_A2_Trigger = digitalRead(SENSOR_A2);
 if (SENSOR_A2 _Trigger != 1) {
   dataToI2C = 12;

}
 else dataToI2C = 0;

SENSOR_A1_Trigger = digitalRead(SENSOR_A1 );
 if (SENSOR_A1_Trigger != 1) {
   dataToI2C = 11;
   //  Serial.println(dataToI2C);

}
 else dataToI2C = 0;

}




Don't expect pin SENSOR_A2 ever to be read by the master. Even if the pin is pulled low, the variable dataToI2C will be set but just to be overwritten by the next few lines. The result in the variable will almost ever be the value of the SENSOR_AI pin. Only in the extremely seldom case that the I2C request is received exactly while the pin value of SENSOR_A1 is still being read, then the SENSOR_A2 value will be sent.

Thanks for pointing this out. I posted this slave as it is the first where I have installed 2 sensors, and yes I am having issues with the sensors triggering correctly. The code seems to work fine with just one sensor. Can you suggest a better way of doing it?

You can use one byte, and each bit is for a sensor.
Arduino has the bitWrite() and bitRead() to deal with bits.
You could also use a byte per sensor, and use two bytes. They can be an array, but they can also be two separate variables.

Your I2C bus should work 100%. If it really works 100%, then you can assume that the Master has read the information when the onRequest handler runs.

For the data, you can use a variable with a boolean flag, or combine them in a single variable and make the value zero when there is no data.
When you set the data when a trigger is detected and clear it in the onRequest handler, then the Master clears it when reading it.

The code depends on what the Master wants to know:

  1. Only the event when the input became active, or only the event when the input became inactive.
  2. Both the events when the input changes.
  3. The state of that moment.
  4. A combination of the other options.

A 10k pullup resistor is a high value. The signals on the I2C bus will be weak.
Here are nice pictures for different pullup values: Gammon Forum : Electronics : Microprocessors : I2C - Two-Wire Peripheral Interface - for Arduino.
Are there sensor-modules connected to the I2C bus ? do those modules have pullup resistors ? how many Arduino boards ? A value of 2k7 to 4k7 might be better.