I2C Hangs on Master when slave uses interrupts?

Hi All,

I have a project that requires 8 rotary encoders (Incremental quadrature type) for human/hand input. I liked how they feel better with 2 interrupts per encoder (rather than polling in loop) but 8 encoders means 16 interrupt pins and I don’t have that many pins. To keep pin requirements down, permit the use of 2 interrupts per encoder, and overcome some unrelated issues with processor time, I decided to try offloading the reading of each encoder to an ATtiny85 - where it would read states on interrupt, calculate rotation, and store the data for retrieval - and letting a “main” processor poll them at it’s leisure. Without getting into the details this main processor will have LOTS of other stuff to do.

I’m having good luck reading the encoder, calculating “clicks” and storing useful data. For the most part the master is able to pull this data every 10ms or so (roughly how often the actual project will be able to do so) and the ATtiny85 program can even accrue multiple clicks when I turn the encoder quickly and the master gets the accumulated values as expected.

The problem is that I get random crashes in the master code - typically but not always when I’m pushing it on encoder rotation speed but without a pattern as to how many data retrievals. Sometimes it runs for 10 clicks, sometimes for 500. The program hangs up for some reason and I have confirmed that it truly stops by placing Serial.prints in the loop so I can see when it halts. There are other threads on I2C hangups but they seem inconclusive and modifying the libraries is beyond my comfort zone.

So, the prototype of the encoder reader/ I2C slave:

#include <TinyWireS.h>
//#include <usiTwiSlave.h>

#define I2C_SLAVE_ADDRESS 0x4 // Address of the slave

static byte AB;
static byte old_AB;
static int8_t state;
static int8_t count;
static byte data;

static int8_t enc_states[] = {0, -1, 1, 0, 1, 0, 0, -1, -1, 0, 0, 1, 0, 1, -1, 0};

void setup() {
  // Interrupts
  GIMSK = 0b00100000;    // turns on pin change interrupts
  PCMSK = 0b00011000;    // turn on interrupts on pins 3 and 4
  sei();      //Enable interrupts


  //Pins
  DDRB = 0b00011000;    
  PORTB = 0b00011000;    //Pins 3&4 INPUT_PULLUP

  TinyWireS.begin(I2C_SLAVE_ADDRESS); // join i2c network
  TinyWireS.onRequest(requestEvent);

}

void loop() {

}

void requestEvent()
{
  data = count / 4;  //I only want to increment a "click" every 4 pulses
  count %= 4;  // Don't lose the remainder if I2C is requested mid-click

  TinyWireS.send(data);
  data = 0;
}


ISR(PCINT0_vect)
{
  old_AB <<= 2;
  AB = (PINB & 0b00011000) >> 3; //Read pins 3 and 4, shift over
  old_AB |= ( AB & 0x03 );
  state = enc_states[( old_AB & 0x0f )];
  count += state;
}

The ISR runs some code adapted from this blog > https://www.circuitsathome.com/mcu/reading-rotary-encoder-on-arduino

And the master:

//Code for the Arduino Mega<
#include <Wire.h>

int8_t i;

void setup()
{
  Wire.begin(); // join i2c bus (address optional for master)
  Serial.begin(9600); // start serial for output
}

void loop()
{
  
  Wire.requestFrom(4, 1); // request 1 byte from slave device address 4

  //while (Wire.available()) // slave may send less than requested
  {
    i = Wire.read(); // receive a byte as character
  }

  if (i) //check if i is non-zero
  {
    Serial.println(i); // print the character
  }
  //else
  {
    //Serial.println("none"); // print the character
  }

  delay(10);
}

The master requests a byte every loop but only prints it when it receives a non-zero value. Typical values are 1 or -1, but I can get them up to +/-5 with really fast turns. Never high enough to exceed the limits of int8_t variables at least.

I’ve tried moving the ISR routine code to the loop (and disabling interrupts) but I still get the hangups on the master end, so I don’t think excess interrupt activity is the cause. I’m also under the impression form Nick Gammon’s page the I2C function disable interrupts any way.

I’d appreciate some extra perspectives,
Brian

How long are the wires of the I2C bus ? I mean the total length of every piece of wire for SDA and SCL.
Do you use some kind of cable for the I2C bus ? can you make a photo of it ?

The Wire library is 'blocking'. When there is big trouble on the I2C bus, the Master could just halt. That is a big disadvantage of course, and we are all waiting for a timeout to be added to the Wire library.

I noticed a few things that can be improved.

The Wire.requestFrom() can check if the byte was received. Either with Wire.available() or with the return value of Wire.requestFrom().

// option 1
int n = Wire.requestFrom(4, 1);
if ( n == 1 )
{
  i = Wire.read();
}

// option 2
Wire.requestFrom(4, 1);
if ( Wire.available() == 1 )
{
  i = Wire.read();
}

Every global variable that is used in an interrupt handler must be 'volatile'. That means all the global variables in requestEvent() and ISR().
The requestEvent() is also an interrupt handler, it is called from an interrupt handler inside the Wire library.

volatile byte AB;
volatile byte old_AB;
volatile int8_t state;
volatile int8_t count;
volatile byte data;

The 'enc_state' is an array of constants. It is possible to declare it as 'const'. The compiler will have more possibilities for optimizations.

const int8_t enc_states[] = {0, -1, 1, 0, 1, 0, 0, -1, -1, 0, 0, 1, 0, 1, -1, 0};

How long are the wires of the I2C bus ? I mean the total length of every piece of wire for SDA and SCL.
Do you use some kind of cable for the I2C bus ? can you make a photo of it ?

For now everything is on a breadboard. I2C bus is two 6" pololu jumper wires.

I'll look at those other suggestions ASAP. Thanks.

Brian

Those short wires should be no problem.
I suppose everything runs at 5V ? So you have a 5V I2C bus and no 3.3V sensors are attached to that I2C bus ?

The Wire library has been updated with version 1.6.6 to avoid a timing/collision problem.
Do you use the newest Arduino IDE 1.6.7 ?

You have commented out the while(Wire.available()). Are you sure that something like that is not in your sketch ?

Those short wires should be no problem.
I suppose everything runs at 5V ? So you have a 5V I2C bus and no 3.3V sensors are attached to that I2C bus ?

The Wire library has been updated with version 1.6.6 to avoid a timing/collision problem.
Do you use the newest Arduino IDE 1.6.7 ?

You have commented out the while(Wire.available()). Are you sure that something like that is not in your sketch ?

Yup, all 5v. I’ve done lot’s of projects with I2C and never had this happen. I should have been more specific about my hardware - the ATtiny85 is actually an Adafruit Trinket and the main arduino is a Teensy 2.0++.

I was using 1.6.1 but I upgraded and recompiled. Still having the crashes.

I have now changed my variable calls. Globals are volatile and the ones used in the interrupt only are declared inside of the function as static:

#include <TinyWireS.h>
//#include <usiTwiSlave.h>

#define I2C_SLAVE_ADDRESS 0x4 // Address of the slave

volatile int8_t state;
volatile signed long count;
volatile byte data;

bool flag;

const int8_t enc_states[] = {0, -1, 1, 0, 1, 0, 0, -1, -1, 0, 0, 1, 0, 1, -1, 0};

void setup() {
  // Interrupts
  GIMSK = 0b00100000;    // turns on pin change interrupts
  PCMSK = 0b00011000;    // turn on interrupts on pins 3 and 4
  sei();      //Enable interrupts


  //Pins
  DDRB = 0b00011000;
  PORTB = 0b00011000;    //Pins 3&4 INPUT_PULLUP

  TinyWireS.begin(I2C_SLAVE_ADDRESS); // join i2c network
  TinyWireS.onRequest(requestEvent);

}

void loop() {
}

void requestEvent()
{
  data = count / 4;  //I only want to increment every 4 pulses
  count %= 4;  // Don't lose the remainder if I2C is requested mid-click

  TinyWireS.send(data);
  data = 0;
}


ISR(PCINT0_vect)
{
  static byte AB;
  static byte old_AB;

  old_AB <<= 2;
  AB = (PINB & 0b00011000) >> 3; //Read pins 3 and 4, shift over
  old_AB |= ( AB & 0x03 );
  state = enc_states[( old_AB & 0x0f )];
  count += state;
}

I think it should not crash with ATmega328P Arduino board as Master and as Slave. There might be a bug somewhere, or they are somehow not compatible.
The Trinket is running at 8MHz ? and the Teensy++ 2.0 at 16MHz ?
You could try a short delay between the different I2C transmissions, so every Slave knows that the bus is idle.
Are you very sure that you don't wait forever on Wire.available() ? You should check how many bytes are received.

I've been leaving out the while statement (and experimenting with substituting in an if) because I always know how many bytes will be received from this group of slaves. Removing it or replacing it isn't having any affect on the hang ups, sadly.

I've also added some 10k pullups. The processors have internal pullup but the extra parallel resistance is also not doing anything.

Currently there is only one slave in this setup and there is a 10ms delay between reads.

...And to make things more perplexing, when I Serial.println(i) in the master program I get 1 or -1 as expected. But when I Serial.println(i, BIN) I get 32 bytes (all 1's when the value is negative 1)... but as an int8_t it should only be 8 bits long. If I change i to a byte I get 8 bits so I don't know what I'm seeing here... int16_t and int32_t show on 8 bits, presumably due to leading 0's. If I print sizof(i) both byte and int8_t versions return as a single byte. I tested this in version 1.6.1 and 1.6.6.

:confused:

The Serial.print with binary is extended to unsinged long or so. I think it was for a reason.

You have to check the number of actual received bytes. The Arduino Slaves use clock pulse stretching. Perhaps that might go bad once in a while.

The 10k pullup combined with the internal pullup resistors might still be high. Perhaps 4k7 is better, but reading your posts, that probably will not help. It seems to be some kind of incompatibility, and I have no idea what.