Servicing an I2C device from an ISR...

I was happily writing an interrupt service routine to service an NXP UART that is communicated with via I2C and of course it didn't work. Here is why:

http://www.arduino.cc/cgi-bin/yabb2/YaBB.pl?num=1251326763

Since I2C uses interrupts you can't use it in an ISR because interrupts are down. So I set a flag in the ISR and poll the flag in my main application, and service the UART when the flag is set. Code below.

My question is, what's the point of the ISR? If I have to set a flag and poll it, why not just poll the device directly?

Is there a better way? Maybe put interrupts back up in the ISR but declare it atomic?

Thanks...

#include <Wire.h>
#include <MultiSerial.h>

#define DATA_LED 20

// Create a MultiSerial instance to communicate with port 0 on shield 0x4D
MultiSerial msInstance = MultiSerial(0x4D, 1);

volatile bool isrTriggered = false;

// The ISR in all its glory
ISR(PCINT1_vect)
{
  isrTriggered = true;
}

void setup() 
{
  // establish interrupt processing / ISR
  PCICR |= (1<<PCIE1);
  PCMSK1 |= (1<<PCINT10);
  MCUCR = (1<<ISC10);

  pinMode(2, INPUT);
  
  Serial.begin(9600);
  msInstance.begin(4800);

  // Turn on the shield's receive interrupt
  msInstance.enableInterrupt(INT_RX);

  // Read some registers to confirm a few things
  
  unsigned char tmp = msInstance.msReadRegister(IER);
  Serial.print("IER: ");
  Serial.println(tmp, HEX);
  
  tmp = msInstance.msReadRegister(FCR);
  Serial.print("FCR: ");
  Serial.println(tmp, HEX);
  
  tmp = msInstance.msReadRegister(IIR);
  Serial.print("IIR: ");
  Serial.println(tmp, HEX);
}


void loop() 
{
  if(isrTriggered)
  {
    isrTriggered = false;
    Serial.print((char)msInstance.read());
  }
}

My question is, what's the point of the ISR? If I have to set a flag and poll it, why not just poll the device directly?

Well a ISR might also increment a variable as well as set a flag. the main program would test the flag and if set then read the variables value and then reset the flag. ISRs do work and can ensure that the main program does not miss external events if structured properly. ISR routine should be as short and fast as possible, especially if your program is using other functions that rely on interrupts (millis(), Serial(), etc.)

Without using ISR and only polling in the main program, the loop time may be too long and slow to capture external events. Interrupts are very useful and powerful, but do have to be used properly and carefully.

Lefty

As Lefty said, with interrupts you won't miss a short event, with polling you can easily miss them.


Rob

Guys, I appreciate the replies, but I do understand the purpose of an ISR. Normally an ISR can service what its doing, like extracting the data from a nearly full UART and moving it to a buffer, right away so that the buffer does not overflow and result in lost data.

If I just set a flag in the ISR, then I have to poll for that flag and then do what needs to be done after I see the flag set. That means the task is not going to get done right away, the FIFO in the UART is going to overflow, and data is lost. I can as easily just poll the UART as I can a flag set by an ISR.

There must be a hierarchy of interrupts and a better way to do this... Off to the data sheet I guess...

Sorry, didn't mean to teach you how to suck eggs. That's true, if the ISR is for a UART for example then yes you should get and save the character as you say.


Rob

Graynomad:
Sorry, didn't mean to teach you how to suck eggs. That's true, if the ISR is for a UART for example then yes you should get and save the character as you say.

Right, so the trick is, how do you talk to an I2C UART from an ISR when you can't use I2C inside the ISR?

I am thinking I can declare the code inside the ISR atomic and put interrupts back up inside the atomic block. Just a theory. I expect I am not the first guy to run across this issue.

skyjumper:
If I just set a flag in the ISR, then I have to poll for that flag and then do what needs to be done after I see the flag set. That means the task is not going to get done right away, the FIFO in the UART is going to overflow, and data is lost. I can as easily just poll the UART as I can a flag set by an ISR.

I looked at I2C devices that generated interrupts here:

The point of the ISR is that you can be doing other useful work, without having to poll very frequently.

Can you give the part number of the UART in question? What rate of data do you expect to process (ie. how fast do you need to react)? What else, if anything, needs to be done in the main loop?

Interrupts have their good and bad points. In another thread recently we established that using interrupts to handle SPI would not keep up with very high speed incoming data. However, the only thing that worked was a very tight CPU loop (polling). So, you wouldn't get anything else done.

But in the application of keypresses (like in the thread I posted) where the interrupts only need to be serviced quite slowly (eg. human reaction time) then it greatly simplifies the rest of the code that you don't be polling all the time.

I am thinking I can declare the code inside the ISR atomic and put interrupts back up inside the atomic block.

You could conceivably enable interrupts in the ISR (after disabling this particular interrupt hook) and then press on and get the data from within the ISR. But I would be cautious. For example, you may be changing variables which the main loop is looking at. But if the main loop isn't doing much, then setting a flag and checking the flag is probably just as simple.

Another approach is to "manually" do the I2C stuff - that is write your own code that doesn't rely on interrupts. After all you have access to the I2C hardware on the CPU.

But really, I2C isn't that fast a protocol (like, 10,000 bytes a second). Setting a flag, and testing it in the main loop, is probably plenty fast enough.

skyjumper:
Right, so the trick is, how do you talk to an I2C UART from an ISR when you can't use I2C inside the ISR?

You can use I2C inside an ISR. You probably can't use the Wire library in its unmodified state. After all, the Wire library accesses I2C from inside interrupts.

Thanks Nick, comments below...

[quote author=Nick Gammon link=topic=71250.msg531462#msg531462 date=1315088392]

skyjumper:
Can you give the part number of the UART in question? What rate of data do you expect to process (ie. how fast do you need to react)? What else, if anything, needs to be done in the main loop?

The UART is an NXP SC16IS752. Here is the data sheet:

This is a dual UART with 64 byte FIFOs for each channel (send and receive). There is one IRQ pin for the entire chip. When the IRQ is triggered, the code needs to read the registers and see what triggered the IRQ. It could be the FIFO filling on either channel, it could be either channel ready to accept more data to send, it could be an overrun condition...

The data coming in is not lightning fast, it will vary from 4,800 baud to 19,200 depending upon user configuration. As for what's done int he main loop, lots and lots and lots of stuff.

Ultimately I'll need to process up to 4 incoming data streams, process the data in real time to generate summary data derrived fromt he read data, and then even spit out a few data streams based upon the data I read.

Interrupts have their good and bad points. In another thread recently we established that using interrupts to handle SPI would not keep up with very high speed incoming data. However, the only thing that worked was a very tight CPU loop (polling). So, you wouldn't get anything else done.

Sometimes an Arduino is not the right tool for the hob I guess. However, with careful design and a good implementation of a state machine you can accomplish an amazing amount of processing.

But in the application of keypresses (like in the thread I posted) where the interrupts only need to be serviced quite slowly (eg. human reaction time) then it greatly simplifies the rest of the code that you don't be polling all the time.

Human reaction time is amazingly slow. The need I have is very similar to the Arduino's HardwareSerial.cpp file, which services the internal UARTs of the MPU. It just takes data from the UART and adds it to a nice ring buffer. The bigger the buffer, the more flexibility the main loop has in getting around to it. Yes I realize big buffers cause other issues and don't always solve that problem.

Another approach is to "manually" do the I2C stuff - that is write your own code that doesn't rely on interrupts. After all you have access to the I2C hardware on the CPU.

This is the conclusion I am arriving at. Do you know if anyone has done that on Arduino platform?

But really, I2C isn't that fast a protocol (like, 10,000 bytes a second). Setting a flag, and testing it in the main loop, is probably plenty fast enough.

I tried that, no joy.

This is the conclusion I am arriving at. Do you know if anyone has done that on Arduino platform?

No I don't. They may have done, I haven't tried Googling it.

If I were to attempt it, I would look at twi.c and the functions twi_writeTo and twi_readFrom. Then remove the dependence on interrupts. In particular things like this:

// wait until twi is ready, become master receiver
  while(TWI_READY != twi_state){
    continue;
  }

and:

// wait for read operation to complete
  while(TWI_MRX == twi_state){
    continue;
  }

Instead of testing twi_state (which is set in the interrupt) simply test the appropriate hardware register (ie. poll it).

Turning interrupts back on in the ISR might work - I'll try that and see what happens.

Well, somewhat surprisingly perhaps, that seems to work. :wink:

Master (this gets the interrupt and requests data via I2C during the interrupt routine):

// Written by Nick Gammon
// September 2011

#include <Wire.h>

const byte SLAVE_ADDRESS = 22;

#define GET_COUNT 0x42
#define GET_DATA 0x43

volatile byte buf [256];
volatile byte count;
volatile boolean got_data;

// ISR
void data_ready ()
{

  // don't let us fire again for now
  detachInterrupt(0);  
  
  // allow I2C to work
  interrupts ();

  // ask slave how much data it has
  Wire.beginTransmission (SLAVE_ADDRESS);
  Wire.send (GET_COUNT);
  Wire.endTransmission ();
  Wire.requestFrom (SLAVE_ADDRESS, (byte) 1);
  count = Wire.receive();

  // now get that data
  Wire.beginTransmission (SLAVE_ADDRESS);
  Wire.send (GET_DATA);
  Wire.endTransmission ();
  Wire.requestFrom (SLAVE_ADDRESS, count);

  // put into buf
  for (byte i = 0; i < count; i++)
  {
    if (!Wire.available ())
      break;
    buf [i] = Wire.receive (); 
  }

  // flag main loop to look inside buf
  got_data = true;

}  // end of data_ready

void setup () 
{
  Wire.begin ();
  Serial.begin (115200);
  attachInterrupt(0, data_ready, FALLING);
} // end of setup

void loop ()
{

  // blah blah do stuff here
  
  // aha! data has arrived from I2C ...
  if (got_data)
    {
    Serial.print ("Got: ");
    Serial.write ((byte *) buf, count);
    Serial.println ();
    
    // clear flag
    got_data = false;
    
    // re-enable interrupts from device
    attachInterrupt(0, data_ready, FALLING);
    }

}  // end of loop

Slave (periodically raises the "interrupt" line). Responds to I2C requests at any time:

// Written by Nick Gammon
// September 2011

#include <Wire.h>

const byte SLAVE_ADDRESS = 22;

#define GET_COUNT 0x42
#define GET_DATA 0x43

#define INTERRUPT_PIN 5

volatile byte command;

char buf [] = "But does it get blood out?";

void setup() 
{
  Wire.begin (SLAVE_ADDRESS);
  Wire.onReceive (receiveEvent);
  Wire.onRequest (requestEvent);  
  pinMode (INTERRUPT_PIN, OUTPUT);
  digitalWrite (INTERRUPT_PIN, HIGH);
} // end of setup

void loop() 
{
  delay (5000);
  
  // generate an interrupt at the other end
  digitalWrite (INTERRUPT_PIN, LOW);   
  delay (1);
  digitalWrite (INTERRUPT_PIN, HIGH);   
  
}  // end of loop

void receiveEvent (int howMany)
 {
  while (Wire.available () > 0)
    command = Wire.receive ();
}  // end of receiveEvent

void requestEvent ()
{
 switch (command)
   {
   case GET_COUNT:  Wire.send ((byte) sizeof buf); break;  
   case GET_DATA:   Wire.send ((byte *) buf, sizeof buf); break;  

   }  // end of switch
  
}  // end of requestEvent

This screenshot shows that it responded to the interrupt within 19 uS (T1 - T2):

Whatcha got for an analyzer there Nick?

Nick, thank you very much!

So in my code, I don't use the attatchInterrupt() / detatchInterrupt() calls but I can still disable the ISR. If I do that, I wonder what will happen if the UART decided to fire the IRQ to alert about another condition, like TX buffer empty. When it fires the IRQ to indicate that the FIFO needs attention, that IRQ is cleared when I read the data. So maybe if I clear that condition, another condition will leave the IRQ pin asserted, and then cause the MPU to detect that interrupt when I put the handler back on. if so, my ISR will be reentered before I exited it.

And then of course if I am reading say 56 bytes from a 64 byte FIFO, the IRQ is probably cleared as soon as I read the first one, so if the FIFO fills again while I am reading it then another IRQ is triggered.

Also, leaving interrupts on means another one might come along and take away from my ISR.

But let me try what you did. ATOMIC_BLOCK() is not going to work in this case because all that does is put global interrupts down, so I can check the value of a variable that is modified inside an ISR without havign to worry that the ISR will change it while I am checking it.

I think the best approach is to be able to talk to I2C without needs interrupts on, but let me give this a try, thank you very much!

Whatcha got for an analyzer there Nick?

That's the Saleae Logic, same as mine.

I don't know how people debug this sort of thing without one :slight_smile:


Rob

You guys have the 8 channel or the 16? I'm gonna order one.

I've got the 8 ch.

I'm gonna order one.

Best $150 you'll ever spend.

The 16ch wasn't available when I got mine, I haven't had a good look at it but if you have the $ maybe that would be better. Although I've yet to need more than about 5-6 channels IIRC.


Rob

skyjumper:
So in my code, I don't use the attatchInterrupt() / detatchInterrupt() calls but I can still disable the ISR.

How? Why do that?

skyjumper:
If I do that, I wonder what will happen if the UART decided to fire the IRQ to alert about another condition, like TX buffer empty. When it fires the IRQ to indicate that the FIFO needs attention, that IRQ is cleared when I read the data. So maybe if I clear that condition, another condition will leave the IRQ pin asserted, and then cause the MPU to detect that interrupt when I put the handler back on. if so, my ISR will be reentered before I exited it.

That's why I did the attachInterrupt in the main loop, so I have in fact left the ISR by that time. However it might be wiser to do that as the very last thing in the ISR. Like:

...
  noInterrupts ();
  attachInterrupt(0, data_ready, FALLING);
}  // end of data_ready

Now it can't be re-entered because we disabled interrupts temporarily (but they are still queued).

Also, leaving interrupts on means another one might come along and take away from my ISR.

Not with the detachInterrupt it won't.

... that IRQ is cleared when I read the data ...

Not necessarily. Browing the datasheet it looks more likely that it is cleared when you read the interrupt status register, which you can choose to do at any time, but don't quote me on that.

I've gotten by with just serial prints & 2-channel USB scope for a year, so 8 channel probably enough.
I might get 16 just to have them if needed, is not that much more. Then can watch data on an 8-bit bus and still have some control signals left.

CrossRoads:
You guys have the 8 channel or the 16? I'm gonna order one.

I have both, but the 8 channel one is the one I use all the time. The 16 channel one has more channels, obviously, and can run faster, but since the 8 channel one works fine for everything I need right now I still use that. Like, for the screenshot above.

And indeed, without it you are largely guessing when debugging interrupts, I2C, SPI, serial etc.

With one you can "think outside the box" a bit. For example, use the normal SDA/SCL lines (with its inbuilt analyzer) but then hook up another pin for detecting interrupts (like I did above). Or connect a pin to D13 to see exactly when the LED lights up. That sort of thing.

The analyzer helps narrow down whether the problem is the sending end, the receiving end, the wiring, the timing etc.

Extremely useful.