How to properly use interruptions with twi (I2C)

Question

I would like to reprogram Wire.h, or a part, to use interruptions, in order to not waste time to wait. I'm using an Arduino Mega with an Adafruit motor shield v2.3.

In the datasheet, page 262, 24.9.2, bit 0, I can read that I have to set the TWIE flag to one to be able to catch the interruption.

So, I started to write this code :

//https://www.arduino.cc/en/uploads/Main/arduino-mega-schematic.pdf
#include <avr/interrupt.h>
#define ADDRSHIELD 0xC0//adresse sur 8 bits
#define TWI_FREQ 100000L
#define cbi(sfr, bit) (_SFR_BYTE(sfr) &= ~_BV(bit))
#define sbi(sfr, bit) (_SFR_BYTE(sfr) |= _BV(bit))
#define START 0x08 //TWI start
#define MT_SLA_ACK 0x18 //slave ACK has been received
#define MT_DATA_ACK 0x28 //master ACK has been received
#define LED0_ON_L 0x6
#define TWI_STATUS (TWSR & 0xF8)
int etat = 0;
uint8_t currentstep = 0;
uint8_t _addr;
uint8_t _data;
ISR(TWI_vect)
{
 //I remove the TWINT flag
 TWCR |= (1<<TWINT);
 switch(etat)
 {
 case 0:
 //start has been send, let's check if it's ok
 if(TWI_STATUS == START)
 {
 //on charge l'adresse dans TWDR et on clear le bit TWINT dans TWCR pour demarrer la transmission
 TWDR = _addr;
 TWCR = _BV(TWIE) | _BV(TWEN);
 etat = 1;
 //Serial.println(etat);
 }
 else
 {
 etat = -1;
 }
 break;
 //addr has been send
 case 1:
 if(TWI_STATUS == MT_SLA_ACK)
 {
 TWDR = _data;
 TWCR = _BV(TWIE) | _BV(TWEN);
 etat = 2;
 //Serial.println(etat);
 }
 else
 {
 etat = -2;
 }
 break;
 //data have been send
 case 2:
 if(TWI_STATUS == MT_DATA_ACK)
 {
 // disable twi module, acks, and twi interrupt
 TWCR = (1<<TWIE)|(1<<TWEN)|(1<<TWSTO);
 etat = 3;
 //Serial.println(etat);
 }
 else
 {
 etat = -3;
 }
 break;
 //stop condition
 case 3:
 // desactivate internal pullups for twi.
 digitalWrite(SDA, 0);
 digitalWrite(SCL, 0);
 etat = 0;
 //Serial.println(etat);
 //Serial.println("Next");
 break;
 }
}
void sendData(uint8_t addr, uint8_t data)
{
 _addr = addr;
 _data = data;
 i2cSetup();
 //start condition
 TWCR = (1<<TWSTA)|(1<<TWEN)|(1<<TWIE);
}
void i2cSetup(void)
{
 // activate internal pull-ups for twi
 sbi(PORTD, 0);
 sbi(PORTD, 1);
 // initialize twi prescaler and bit rate
 cbi(TWSR, TWPS0);
 cbi(TWSR, TWPS1);
 TWBR = ((F_CPU / 100000) - 16) / 2;
}
void setup() 
{
 Serial.begin(9600);
}
void loop() 
{
 if(etat == 0)
 {
 sendData(0xC0, 0b1010101);
 }
 if(etat == 3)
 {
 Serial.println("Next");
 } 
 if(etat == -1)
 {
 Serial.println("Error from start");
 delay(100);
 exit(-1);
 } 
 if(etat == -2)
 {
 Serial.println("NACK");
 delay(100);
 exit(-1);
 } 
 if(etat == -3)
 {
 Serial.println("DATA NACK");
 delay(100);
 exit(-1);
 } 
}

My problem is, once I reset TWIE to one on my interruption, the code looks like to freeze(in ISR(), case 0, TWCR =...). In my mind, the interruption is call only when TWIE is set to one and when the twi module says (when he sets TWINT to one I guess) that he finished the job. I don't understand how to properly use this interruption.

EDIT : Thanks to @jfpoilpret, know I clearly know the problem, the slave doesn't reply by an acknowledge (in my switch, case 0, the status isn't equal to START (0x08)). In the datasheet, p246, they clearly explain how to use registers, but I'm not sure how to use the flag TWIE. I don't know if I have to set them both to one, or if i have to completely leave TWINT (I tried both, but nothing worked...).

Thanks for your help, julien

• jfpoilpret: First off, when using interruptions, you must make your ISR code as quick as possible. In particular, NEVER USE Serial.print... from an ISR.
• jfpoilpret: Great! Can you please post, as an answer, the modifications you made so that this can help other people with the same problem in the future?
• jfpoilpret: Perfect. Here is an additional hint to improve your program: make volatile your global variables when they are shared between your ISR and the rest of your code (i.e. etat, _addr and _data) otherwise the compiler might optimize access to these, thinking they never change.
• jfpoilpret: One last point I think you should fix (I did not pay attention to it before, sorry): etat is currently an int (2 bytes), reading or writing it requires several assembly instructions and hence may be interrupted. You should rather declare it as int8_t (one byte only, access not interruptible) as it still fits values you need.
• jfpoilpret: Well I believe there must be other general answers on this site about how to properly write ISR functions. I just wanted to quickly point you about some important hints for your code. One last point: you declared etat as uint8_t instead of int8_t: they are one byte each, but first one is unsigned, not suitable for negative values you assigned it, although it will probably work -maybe some compiler warnings- as long as you don't try if (etat < 0) which won't work.

Answer 1

After some tests, it's working ! First off all, I used Serial.println in my ISR, that's made it freeze apparently. Second, apparently, I can't send more than 6 bits, else I'll have an error (data NACK), I thought it was 8.

This is my code working :

//https://www.arduino.cc/en/uploads/Main/arduino-mega-schematic.pdf
#include <avr/interrupt.h>
#define ADDRSHIELD 0xC0//adresse sur 8 bits
#define TWI_FREQ 100000L
#define cbi(sfr, bit) (_SFR_BYTE(sfr) &= ~_BV(bit))
#define sbi(sfr, bit) (_SFR_BYTE(sfr) |= _BV(bit))
#define START 0x08 //TWI start
#define MT_SLA_ACK 0x18 //slave ACK has been received
#define MT_DATA_ACK 0x28 //master ACK has been received
#define LED0_ON_L 0x6
#define TWI_STATUS (TWSR & 0xF8)
volatile int8_t etat = 0;
volatile uint8_t _addr;
volatile uint8_t _data;
ISR(TWI_vect)
{
 switch(etat)
 {
 case 0:
 //start has been send, let's check if it's ok
 if(TWI_STATUS == START)
 {
 //on charge l'adresse dans TWDR et on clear le bit TWINT dans TWCR pour demarrer la transmission
 TWDR = _addr;
 TWCR = _BV(TWIE) | _BV(TWEN);
 etat = 1;
 }
 else
 etat = -1;
 break;
 //addr has been send
 case 1:
 if(TWI_STATUS == MT_SLA_ACK)
 {
 TWDR = _data;
 TWCR = _BV(TWIE) | _BV(TWEN);
 etat = 2;
 }
 else
 etat = -2;
 break;
 //data have been send
 case 2:
 if(TWI_STATUS == MT_DATA_ACK)
 {
 // disable twi module, acks, and twi interrupt
 TWCR = (1<<TWIE)|(1<<TWEN)|(1<<TWSTO);
 etat = 3;
 }
 else
 etat = -3;
 break;
 //stop condition
 case 3:
 // desactivate internal pullups for twi.
 digitalWrite(SDA, 0);
 digitalWrite(SCL, 0);
 etat = 0;
 break;
 }
 //I remove the TWINT flag
 TWCR |= (1<<TWINT);
}
void sendData(uint8_t addr, uint8_t data)
{
 _addr = addr;
 _data = data;
 i2cSetup();
 //start condition
 TWCR = (1<<TWSTA)|(1<<TWEN)|(1<<TWIE)|(1<<TWINT);
}
void i2cSetup(void)
{
 // activate internal pull-ups for twi
 sbi(PORTD, 0);
 sbi(PORTD, 1);
 // initialize twi prescaler and bit rate
 cbi(TWSR, TWPS0);
 cbi(TWSR, TWPS1);
 TWBR = ((F_CPU / 100000) - 16) / 2;
}
void setup() 
{
 Serial.begin(9600);
}
void loop() 
{
 if(etat == 0)
 {
 sendData(0xC0, 0b0010101);
 }
 if(etat == 3)
 {
 Serial.println("Next");
 }
 if(etat == -1)
 {
 Serial.println("Error from start");
 delay(100);
 exit(-1);
 } 
 if(etat == -2)
 {
 Serial.println("NACK");
 delay(100);
 exit(-1);
 } 
 if(etat == -3)
 {
 Serial.println("DATA NACK");
 delay(100);
 exit(-1);
 } 
}

Thanks a lot to jfpoilpret !

• Bence Kaulics: Note that you can accept your own answer after some days by clicking on the tick beside it.

Answer 2

Two observations.

1. The datasheetss does a fairly good job of Laying out the states and required conditions to move forward. Your code simply needs to mimic that. A short cut is to take a look at lpc210x datasheetss as it has fairly simple c code for the state machine implementation in the Isr.

2. What I typically do for slow multi byte transmission, i2c or UART, is to point a pointer to a string to be transmission, load up the datasheetss register and enable the interrupt. In the Isr, test if all the data has been transmission. If yes, turn off the Isr. Otherwise, load up thee next char. The data received can be processed via a callback function.

This approach minimizes user involvement. On a SPI with buffer, it can transmit without interruption.

Answer 3

Why ?

The Arduino Mega 2560 is not very fast. The gain by using interrupts is not a lot.

The Wire.endTransmission and Wire.requestFrom wait until the I2C transaction has finished. That might seem a waste of time, but for the slow ATmega microcontrollers, not a lot of time is wasted.

The overhead by more code might even slow it down. A I2C library that is completely in software might even be faster than the original Arduino Wire library. Although the Arduino Wire library uses interrupts, the overhead by the Wire library creates a few extra delays during the I2C transaction.

It took years (it really did) to have a normal working Wire library for ATmega microcontrollers and it still is not very good. For example a hardware problem on the I2C bus could halt a sketch. I have seen a number of custom I2C libraries that run into problems one way or the other.

You can waste less time with:

1. Make a well designed sketch, using millis instead of delay.
2. Write a good interface to the sensor. I see a lot bad usage of the Wire library with unnecessary I2C transactions.
3. Increase the I2C speed to 200kHz or 400kHz with Wire.setClock.
4. Try to use the SPI bus if possible. For a 3.3V sensor with a SPI interface, a 3.3V Arduino board is the best choice. For example the Arduino Due with ARM M4 or a small board with ARM M0+ processor.
5. When using the SPI bus with a Ethernet shield, try setting the SPI bus faster.
6. When using a SD card, try full speed, instead of half speed.
7. Use the newest Arduino IDE, because the 'lto' optimization was enabled which helps a lot.

Really serious work for better I2C functions has been done for the Teensy. It is still compatible with the Arduino Wire functions, but extra functions have been added that do not wait.

• julien2313: I need it because I use interruptions (every 1ms) to call functions to read sensors, etc etc, and to send data in I2C. If I used wire.h, it tooks 3.6ms to send my data. Even if I wanted to use it in 400kHz, and if it would took 0.9ms, it's always too loog, because I need additional time in the loop() to manage others things. So, with this, I can manage I2C with others things.
• julien2313: I only use delay() to let the time to Serial.println to print the error, before the exit(). I need I2C because my shield work with that.
• Jot: Thank you for answering my "Why"-question. jfpoilpret has been a very good help (for example with the volatile single bytes). You use a complete I2C transaction for just one data byte for the motor shield every millisecond. Is that really needed ? For a software PWM signal ? I wonder what the other code is. Doing so many things every millisecond with a ATmega chip might be pushing the limit. You could try the MultiSpeed I2C scanner to test the I2C bus: forum.arduino.cc/index.php?topic=197360
• julien2313: Not only 1 byte, little bite more. Yes, it's for a PWM signal. The other code is to manage a Lora connection, logs, and maybe others futur things. I honeslty don't know if I need to send everytime the same data, I just mimic this library : github.com/adafruit/Adafruit_Motor_Shield_V2_Library Why would I push to the limit the ATmega ? If it isn't in my interruption, it would be in my loop(). Thank you for the scanner, I'll try it !
• Jot: The PCA9685 on the motor shield creates the PWM signal. I tried to read the Adafruit library to understand why you need software PWM, but I don't understand it. Using software PWM via I2C at 1000 times per second can't be right. To set individual pins via I2C for a stepper motor is a bad idea, then you need a better motor shield. Either use the Arduino Stepper or the Mikem AccelStepper directly with Arduino pins, or a shield that can do that on its own. Perhaps someone else knows that Motor Shield, or you could try to ask on the Adafruit forum.