Need help troubleshooting I2C issues

Hi Everyone,

I'm working on a project, that will have 1 master talking to 16 slaves. The idea is that I have 16 targets where one will randomly turn RED and another one BLUE. Airsoft player must shoot the target that correspond to their team's color. Once a target is hit, it will be turned OFF, and another of the same color will be turned ON, ready to be shot.

it somehow works, but for whatever reason, when a target is hit, it will sometime record 2 or even 3 point for that single hit ...

The slaves are "hit sensor" for an Airsoft target system. The target will illuminate red or blue. corresponding team need to shoot at the target. The principle of operation of the target sensor is as follow:

The slave are Pro Mini. They each have 2 red LEDs, 2 blue LEDs and a piezo element as the sensing element. The master will send either a '1' (Red Target) or a '2' (Blue Team).

The piezo is connected to the pin that has Interrupt 0. When a '1' or a '2' is received, the Interrupt is enabled. When the piezo senses a hit, a feedback value of 99 is set (arbitrary value) and the interrupt is disabled. LEDs are all turned OFF. The Arduino will then wait for a request from the master. Once that request comes in, Feedback is sent and the value is brought back to 0.

I have stripped down and modified my Master program, in order to simply send a '1' or a '2' to a certain station. Setup switch will allow to change that station, with the use of the "INC" and "DEC" button.

The "Enter" and "Next" will send a '1' or a '2' to that station.

I have connected a protocol sniffer: the first time I send, let's say a '2', the commend goes thru, the Blue LEDs turn ON. If I hit the sensor, the Master will read back '99' indicating it got it. On the protocol analyzer, you can see that there is a delay of about 200,000 uSeconds between the time the LED is turned ON and the time the Hit is recorded. So far so good.

(See "First Capture.JPG" & "Second Capture.JPG")

If I send another '2', the slave gets it, but reply right away with a returned value of '99'. That does not make as the sensor did not get hit, and its "feedback" value was set to '0'

(see "Third Capture.JPG")

Here is the code of the Master:

#include <LiquidCrystal.h>

#include <I2C.h>

// Initialize I/O used
int Setup = 2;
int Enter = 3;
int Next = 4;
int Inc = 5;
int Dec = 6;
int RS = 7;
int EN = 8;
int D4 = 9;
int D5 = 10;
int D6 = 11;
int D7 = 12;
int Start = 13;
int Stop = 14;
int ReceivedHit = 0;
int Station = 1;
boolean Started = 0;
int c = 0;

String Line1 = "";
String Line2 = "";


LiquidCrystal lcd(RS, EN, D4, D5, D6, D7);

void setup() {
  pinMode(Setup,INPUT);
  pinMode(Enter,INPUT);
  pinMode(Next,INPUT);
  pinMode(Inc,INPUT);
  pinMode(Dec,INPUT);
  pinMode(EN,OUTPUT);  
  pinMode(RS,OUTPUT);  
  pinMode(D4,OUTPUT);  
  pinMode(D5,OUTPUT);  
  pinMode(D6,OUTPUT);  
  pinMode(D7,OUTPUT);  
  pinMode(Start,INPUT);
  pinMode(Stop,INPUT);  
  
  lcd.begin(16, 2); 
  
  //Serial.begin(9600); 
  I2c.begin();
  I2c.setSpeed(0);
  I2c.timeOut(1000);

  Line1 = "Station:" + String(Station);
  Line2 = " ";
  LCD_Routine();  
}

void loop() 
{
  if (digitalRead(Setup) == 1)
  {
    Line1 = "Select station";
    Line2 = "Station:"+ String(Station);
    LCD_Routine();
    ReceivedHit = 0;
    ChooseStation();
    delay(25);
  }
    
 if (Started == 1)
    {
    if (digitalRead(Stop) == 1)
        {
        Started = 0;
        }
    else
        {
        Running();
        }
    }
if (Started == 0)
    {
    if (digitalRead(Start) == 1) 
        {
        Started = 1;
        }
    }
}

void Running()
{
delay(25);
ReadStation();
if (digitalRead(Next) == 1)
  {
  I2c.write(Station,1);  // value of 1 is for Red LED
  }
  while(digitalRead(Next) ==1)
    {
    }
if (digitalRead(Enter) == 1)
  {
  I2c.write(Station,2);    // value of 2 is for Blue LED
  }
  while(digitalRead(Enter) ==1)
    {
    }  
}

void ChooseStation()
{
    if(digitalRead(Inc) ==1)
        {
        Station = Station +1;
        if (Station > 8)
          {
          Station = 1; 
          }
        Line1 = "Station:"+ String(Station);
        LCD_Routine();
        while (digitalRead(Inc) == 1)
          {
          }
        }
    if(digitalRead(Dec) ==1)
        {
        Station = Station -1;
        if (Station <1 )
          {
          Station = 8; 
          }
        Line1 = "Station:"+ String(Station);
        LCD_Routine();
        while (digitalRead(Dec) == 1)
          {
          }
        }
}


void ReadStation()
{
      I2c.read(Station,1);    // request 1 byte from Station
      delay(50);
      while(I2c.available())  
        { 
          c = I2c.receive(); 
          if (c == 99) // if equal 99, means that this station recorded a hit
            {             
              ReceivedHit = ReceivedHit +1;
              Line1 = "";
              Line2 = "Received Hits: " + String(ReceivedHit);
              LCD_Routine;
              c = 0;
              while(I2c.available())
                {
                int c = I2c.receive();
                 }
            }
        }
}



void LCD_Routine()
{
  lcd.clear();
  lcd.setCursor(0, 0); 
  lcd.print(Line1);
  lcd.setCursor(0, 1); 
  lcd.print(Line2);
}

and the Slave's code:

#include <Wire.h>

#define I2C_SLAVE_ADDRESS 0x3
// Address of the slave

int HitSensor = 2;    // the pin that the Piezo is attached to
int BlueLED1 = 3;       // the pin that the Blue LED is attached to
int RedLED1 = 4;       // the pin that the Red LED is attached to
int BlueLED2 = 10;       // the pin that the Blue LED is attached to
int RedLED2 = 11;      // the pin that the Red LED is attached to

byte  ByteRcvd = 0;
int NewDataFlag = 0;
byte Feedback = 0;

void setup() 
{
  // initialize the button pin as a input:
  pinMode(HitSensor, INPUT);
  // initialize the LED as an output:
  pinMode(BlueLED1, OUTPUT);
  pinMode(RedLED1, OUTPUT);
  pinMode(BlueLED2, OUTPUT);
  pinMode(RedLED2, OUTPUT);
  Wire.begin(I2C_SLAVE_ADDRESS);                
  Wire.onReceive(receiveEvent); // register event
  Wire.onRequest(requestEvent); // register event
}

void loop() 
  {
    if (Feedback ==99)
    {
      digitalWrite(BlueLED1, LOW);
      digitalWrite(RedLED1, LOW);
      digitalWrite(BlueLED2, LOW);
      digitalWrite(RedLED2, LOW);
    }
  }
  
void receiveEvent(int howMany)
{
  while(Wire.available()) // loop through all but the last
  {
    int x = Wire.read();    // receive byte as an integer
    ByteRcvd = x;
    NewDataFlag = 1;
  }
    delay(50);
if (NewDataFlag ==1)
{
  if (digitalRead(HitSensor) ==0)
  {
  switch (ByteRcvd) 
    {
      case 3:
        Feedback = 0; 
        detachInterrupt(0);
        digitalWrite(BlueLED1, LOW);
        digitalWrite(RedLED1, LOW);
        digitalWrite(BlueLED2, LOW);
        digitalWrite(RedLED2, LOW);
        ByteRcvd = 0;
        NewDataFlag = 0;
        break; 
      case 1:
        Feedback = 0;
        attachInterrupt(0,OnInterrupt, RISING); 
        digitalWrite(BlueLED1, LOW);
        digitalWrite(RedLED1, HIGH);
        digitalWrite(BlueLED2, LOW);
        digitalWrite(RedLED2, HIGH);
        ByteRcvd = 0;
        NewDataFlag = 0;
         
        break;           
      case 2:
        Feedback = 0; 
        attachInterrupt(0,OnInterrupt, RISING); 
        digitalWrite(BlueLED1, HIGH);
        digitalWrite(RedLED1, LOW);
        digitalWrite(BlueLED2, HIGH);
        digitalWrite(RedLED2, LOW);
        ByteRcvd = 0;
        NewDataFlag = 0;
        
        break;  
     default: 
        break ;
      }
    } 
  }
}

void requestEvent()
{
  Wire.write(Feedback); 
  Feedback = 0;
}

void OnInterrupt()
{
      detachInterrupt(0);
      Feedback = 99;
}

Anyone has a solution for me ??? I'm totally puzzled !

should you need more info, let me know.

First capture.jpg

Second capture.jpg

Third capture.jpg

void receiveEvent(int howMany)
{
...
    delay(50);

Don't do delays in an ISR.

Why do you use that library, instead of the Arduino Wire library ? Can you give a link to that I2C library ?

If you add the length of all the wires of SDA and SCL, how long is the total length ?

You took some code from the examples, but that code is not very good. It can be simpler.
I don't know how many bytes you transmit. I assume just one byte.

void receiveEvent(int howMany)
{
  if(howMany == 1)   // check if 1 byte was received.
  {
    ByteRcvd = Wire.read();    // receive byte as an integer
    NewDataFlag = 1;
  }
  ...

Where did you find code with the attachInterrupt and detachInterrupt ?
You can do the detection in the loop() without using interrupts.

The byte Feedback should be 'volatile' since it is used in an interrupt and in normal code.

to Nick:

Good point about the delay....

Since you seem quite familiar with I2C, I have 2 questions:

Whenever I'm sending an I2C command, I'm always worried about execution time of the command vs scantime of the uCPU. That's why I have a tendency to put a delay after a write or a read. I'm assuming let's say for a read, that the I2C read will take some time to be executed, and if later in my program, I need to refer to the result of that read, if not enough time has past, I will not get the expected result...

Am I right to add some delay ? if so, what do you suggest ?

second question: looking at the captured trace, I noticed that when the master request to read something, it gets it right away . I would have expected that the master make the request, then a few uSeconds, the slave would reply with the requested data... there is no delay ?

to Peter_n:

I have been struggling On & Off with that problem for a while. during that period, I read somewhere that the I2C library was more efficient than the Wire library. That's why I used that one. It did not improve anything, but was left there. It might be a good idea to revert back to the more standard, Wire library.

the length of all my wiring is made of 8 x 2 Meters telephone cable (they have RJ-11 jacks at both end). Each cable starts from a central PCB to a sensor. Kind of a start topology. Right now, I'm conducting my tests with 8 sensors, but the final version will have 16. I have not noticed (yet) any drop of packet, so I'm somehow confident that the wiring is not what's causing my issue. Traces looks good of the scope.

I will look at the modified code you posted and play around it.

Regarding the AttachInterrupt & DetachInterrupt, I don't remember where it's coming from....Might be from me after reading the reference manual...Again, as I said, I have been trying to solve that issue for a while and I have made many modification over time...some are trials that stayed in the code.... But is there anything suspicious about my usage of Attach/Detach ?

I have never used the "Volatile" qualifier...will read about it and implement it ..thanks !

jasmino:
to Nick:

Good point about the delay....

Since you seem quite familiar with I2C, I have 2 questions:

Whenever I'm sending an I2C command, I'm always worried about execution time of the command vs scantime of the uCPU. That's why I have a tendency to put a delay after a write or a read.

I am not familiar with the I2C library as I usually use the Wire library that comes with the IDE. My notes: Gammon Forum : Electronics : Microprocessors : I2C - Two-Wire Peripheral Interface - for Arduino

From that page:

Wire.requestFrom does not return until the data is available (or if it fails to become available). Thus it is not necessary to do a loop waiting for Wire.available after doing Wire.requestFrom.

I can't speak for the I2C library but I presume it is similar.

I noticed that when the master request to read something, it gets it right away . I would have expected that the master make the request, then a few uSeconds, the slave would reply with the requested data... there is no delay ?

I would expect a slight delay, yes. Can you post this trace? You can attach images to posts.

When you do a Wire.write(), it is written to a buffer.
The transmission itself is started and finished with Wire.endTransmission().
When you do a Wire.read(), it is read from a buffer.
The transmission is started and finished with Wire.requestFrom().
The OnReceive handler is an interrupt handler after all the bytes have been received in a buffer.
The OnRequest is an interrupt handler in which the buffer should be written.

So you don't need a delay.

The Arduino Wire library has teeth-grinding issues (blocking, no collision detect), but other libraries are often worse.

The I2C bus can do 50cm. With some cables it can do 2 meters. That's about it. You are now at 16 meters ? I didn't know that was even possible.

I don't know about runtime attach and detach form an interrupt routine. I never used it like that, and I won't try it. My feeling tells me its just not right to do.

Gentlemen,

first of all, thank you for your replies: you have raised a couple of question in my mind, resulting in a complete re-write of the slave program; I am not using interrupt anymore and got rid of delays in any interrupt.

Works like a charm !!!

The next step is to revert back to the Wire library in the master program.

Nick: I have attached a screen capture of the report from my protocol analyzer,

Peter_n: right now, I'm using 8 x 2M....as soon as I have made my test with 16 x 2M cable, I'll let you know of the results !

32m of twisted-pair seems like a lot of capacitance. I2C is very sensitive to capacative coupling of the two wires. I would not expect it to work but more surprising things have happened.

jasmino:
Nick: I have attached a screen capture of the report from my protocol analyzer,

The data at the top of that picture is just reading one byte. I don't expect a big delay there, quite possibly that byte has been put in the I2C buffer in advance.

I note that there is a somewhat bigger delay (55118 of whatever units) between the write further down the page and the read straight after it. I'm guessing that is 55 µS which sounds about right.

MorganS: anything that can be done to lower the overall capacitance, in case I end-up with issues ?

You can get I2C-extenders.

It is possible to lower the I2C clock with Wire.setClock(). The default is Wire.setClock(100000UL); (that is 100kHz, the 'UL' indicates that it is an unsigned long number). I don't know what the lowest value may be.

For a longer cable, you can lower the impedance of the I2C bus.
Do you know what the total pullup-resistor is for SDA and SCL ?
Every module (every AVR chip) has internal about 50k, and the master should have pullup resistors. The maximum current is 3mA. So you could try 5V and 2.5mA.
16 Slave modules + 1 Master, each 50k (17*0.1mA) = 1.7mA.
0.8mA left. That means you can add an extra pullup resistor at the Master of 6250 ohm. That can be 6k8.
Is the Master an Arduino Mega 2560 board ? That changes the calculation, it has an extra 10k pullup resistor.
Or is the Master an Raspberry Pi ?

Is it a cable with a few single-core wires, or a flat cable with multi-core wires ? I think the first one has a lower capacitance.