Interrupt Service Routine (ISR) and Serial Communication Conflicts

Hi Everyone,

I’ve been trying to use an UNO to sample power draw on a one second interval. In order to do this, I setup an ISR interrupt with timer1, which works perfectly.

I then have a python script on a PC requesting the UNO to pass power usage every 10secs (I’d like to have this request made for every 1hr, but for testing purposes it is 10secs). Most of the time requests go through and data is sent back as expected, but there are rare occasions data fails to be sent back to the PC.

This puzzled me for some time, until I realized when the python script sends a request for data, the serial communication occasionally happens when the UNO is dealing with an ISR interrupt. It seems as though request information is written on the serial line, but the UNO ignores processing the request after completing the interrupt. The python code then hangs expecting to receive something back from the UNO.

My main question is, why does the UNO ignore the data sent on the serial interface? Is there no input buffer holding the information until the UNO is ready to process it?

For reference, below is the code found on the UNO and the python script. I have also noticed the UNO seems to be in the ISR interrupt for about 250ms.

Thanks for any help provided.

#include <Wire.h>
#include "EmonLib.h"

#define VOLTAGE_RMS 122.8
#define CT_SENSOR_PIN 1
#define LED_PIN 13

EnergyMonitor emon;

double Wrms;
uint32_t wattSecWhole = 0;
uint32_t wattSecFract = 0;
double prevSmpl = 0.0;
String cmd;

void setup() {
  Serial.begin(115200);
  emon.current(CT_SENSOR_PIN, 13.3);
  pinMode(LED_PIN, OUTPUT);

  //Sets sample rate of power using timer1 interrupt at 1Hz (1sec)
  TCCR1A = 0; // set entire TCCR1A register to 0
  TCCR1B = 0; // same for TCCR1B
  TCNT1  = 0; //initialize counter value to 0
  TCCR1B |= (1 << WGM12); //Sets CTC mode
  TCCR1B |= (1 << CS12); //Sets 256 prescaler
  OCR1A = 62499; //Set Compare match register for 1Hz
  TIMSK1 |= (1 << OCIE1A); //Enable timer compare interrupt
}

void loop() {
  cmd = "";

  //Reads command on serial line, if any
  while (Serial.available()) {
    delay(1);
    cmd += (char)Serial.read();
  }

  //Sends power usage sample in WattHour
  if (cmd == "smpl") {
    TIMSK1 &= (0 << OCIE1A); //Disable timer
    TCNT1  = 0;              //Reset timer counter
    
    prevSmpl = wattSecWhole / 3600.0 * 1000000.0;
    Serial.println(prevSmpl, 0);
    wattSecWhole = 0;
    wattSecFract = 0;
    
    TIMSK1 |= (1 << OCIE1A); //Enable timer
  }
  //Recall last sample if needed
  else if (cmd == "recall") {
    Serial.println(prevSmpl, 0);
  }
}

//Timer interrupt used to sample power usage
ISR(TIMER1_COMPA_vect) {
  digitalWrite(13, HIGH);
  
  Wrms = emon.calcIrms(1480) * VOLTAGE_RMS;
  //Wrms = emon.calcIrms(1480) * VOLTAGE_RMS;
  //Wrms = emon.calcIrms(1480) * VOLTAGE_RMS;
  //Wrms = emon.calcIrms(1480) * VOLTAGE_RMS;

  wattSecWhole += (uint32_t)Wrms;
  wattSecFract += (uint32_t)((Wrms - (double)wattSecWhole) * 10000000.0);

  if (wattSecFract > 10000000) {
    wattSecWhole += 1;
    wattSecFract -= 10000000;
  }
  
  digitalWrite(13,LOW);
}
import time
import serial

ser = serial.Serial('COM4', 115200) #,timeout=3)
powerUsage = 0

while 1:
  #Wait 10sec before next sample
  for i in range(10):
    print("\rSmpl in %isec... " % (10 - i), end="")
    time.sleep(1)
  
  #Request next sample from arduino
  print ("\rGetting sample...", end=" ")
  ser.write(b'smpl')
  while ser.in_waiting == 0: None
  
  print (ser.in_waiting, end= " ")
  
  try:
    powerUsage = int(ser.readline().decode("utf-8").replace("\r\n", ""))
    print ("(%i)" % powerUsage, end=" ")
  except ValueError:
    print ("Error...")
    print (ser.in_waiting)
    exit()
    
  print ("DONE")

ISR should be as short as possible - do you need to do the calculations

  wattSecWhole += (uint32_t)Wrms;
  wattSecFract += (uint32_t)((Wrms - (double)wattSecWhole) * 10000000.0);

  if (wattSecFract > 10000000) {
    wattSecWhole += 1;
    wattSecFract -= 10000000;
  }

in the ISR?
make Wrms volatile and do the calculations in loop()?

Have a look at the examples in Serial Input Basics - simple reliable ways to receive data. I don’t believe the code will be affected by your Timer ISR.

And I would go further than @horace. I would reduce the ISR to

ISR(TIMER1_COMPA_vect) {
  digitalWrite(13, HIGH);
 
  Wrms = emon.calcIrms(1480);
  newISRval = true;
}

I don’t know what the call to emon.calcIrms() does, but if it takes more than a few microsecs I would not do that inside the ISR either.

…R

Wrms = emon.calcIrms(1480);

Given that you use the EmonLib from the OpenEnergyMonitor project (https://openenergymonitor.org/), that calcIrms() method needs much more than a few microseconds:

double EnergyMonitor::calcIrms(unsigned int Number_of_Samples)
{

  #if defined emonTxV3
    int SupplyVoltage=3300;
  #else
    int SupplyVoltage = readVcc();
  #endif


  for (unsigned int n = 0; n < Number_of_Samples; n++)
  {
    sampleI = analogRead(inPinI);

    // Digital low pass filter extracts the 2.5 V or 1.65 V dc offset,
    //  then subtract this - signal is now centered on 0 counts.
    offsetI = (offsetI + (sampleI-offsetI)/1024);
    filteredI = sampleI - offsetI;

    // Root-mean-square method current
    // 1) square current values
    sqI = filteredI * filteredI;
    // 2) sum
    sumI += sqI;
  }

  double I_RATIO = ICAL *((SupplyVoltage/1000.0) / (ADC_COUNTS));
  Irms = I_RATIO * sqrt(sumI / Number_of_Samples);

  //Reset accumulators
  sumI = 0;
  //--------------------------------------------------------------------------------------

  return Irms;
}

It calls analogRead() and does some double (!) calculations almost 1500 times. That will need more than 100ms which is far outside the time frame an ISR is allowed to consume.

Anyway: To call a routine once a second you definitely don’t need a timer interrupt. Take a look at the BlinkWithoutDelay example of the IDE, it shows you how to call such routines at the right time.

Thanks for the kind comments everyone,

horace:
ISR should be as short as possible - do you need to do the calculations in the ISR?

I don’t think I really need to do the calculations within the ISR. I should be able to figure out a method to move it within the main loop(). In fact, Robin and pylon both gave me a great idea how to execute this, as seen in the code block at the end of this post.

Robin2:
Have a look at the examples in Serial Input Basics - simple reliable ways to receive data. I don’t believe the code will be affected by your Timer ISR.

And I would go further than @horace. I would reduce the ISR to
[SNIP]

I don’t know what the call to emon.calcIrms() does, but if it takes more than a few microsecs I would not do that inside the ISR either.

…R

Your example code was great to get a better understanding as to how the Serial communication works with an Arduino (best practices of sending and receiving). Although, I am left with one question. When data is being sent to the Arduino, is there some sort of interrupt being called in order to have the arriving bytes stored within the input buffer before being processed and stored into a variable?

ie. I’m sending 4 bytes ‘smpl’ to the Arduino with the understanding that it should be held within the 64byte input buffer even if the Arduino is busy doing other tasks, but it seems like the data is lost if the Arduino is busy within an interrupt. If I remember correctly, the Atmega chip can not execute an interrupt while inside another interrupt.

The “newISRval” flag seems like a great idea, and I think I have found a way to implement it and remove everything out from the ISR. My new implementation in obtaining a sample reading is shown in the code block at the end of this post.

pylon:
It calls analogRead() and does some double (!) calculations almost 1500 times. That will need more than 100ms which is far outside the time frame an ISR is allowed to consume.

Anyway: To call a routine once a second you definitely don’t need a timer interrupt. Take a look at the BlinkWithoutDelay example of the IDE, it shows you how to call such routines at the right time.

You are 100% correct, the emon.calcIrms(1480) is the cause of all the time spent within the ISR. I didn’t realize how much time until toggling an LED to visually see what time was spent within the ISR.

I know with interrupts, it is suggested to spend as little time as possible in it. I was under the assumption that even though around 250ms is spent inside the ISR, there should still be plenty of time to work on other tasks as long as these other tasks were not interrupts as well. I’m starting to believe there is some sort of hidden interrupt being called whenever Serial data arrives in order to have the Arduino store data within the input buffer. Though, I can’t seem to find anything within the datasheet to back this accusation up.

Implementing something similar to the BlinkWithoutDelay example would probably be the best bet. I think I came up with a better idea which basically mixes everyone’s suggestions together. Please see the code below.

#include <Wire.h>
#include "EmonLib.h"

#define VOLTAGE_RMS 122.8
#define CT_SENSOR_PIN 1
#define LED_PIN 13

EnergyMonitor emon;

double Wrms;
uint32_t wattSecWhole = 0;
uint32_t wattSecFract = 0;
double prevSmpl = 0.0;
bool getNextSmpl = false; //NEW CODE
String cmd;

void setup() {
  Serial.begin(115200);
  emon.current(CT_SENSOR_PIN, 13.3);
  pinMode(LED_PIN, OUTPUT);

  //Sets sample rate of power using timer1 interrupt at 1Hz (1sec)
  TCCR1A = 0; // set entire TCCR1A register to 0
  TCCR1B = 0; // same for TCCR1B
  TCNT1  = 0; //initialize counter value to 0
  TCCR1B |= (1 << WGM12); //Sets CTC mode
  TCCR1B |= (1 << CS12); //Sets 256 prescaler
  OCR1A = 62499; //Set Compare match register for 1Hz
  TIMSK1 |= (1 << OCIE1A); //Enable timer compare interrupt
}

void loop() {
  cmd = "";

  //Reads command on serial line, if any
  while (Serial.available()) {
    delay(1);
    cmd += (char)Serial.read();
  }

  //Sends power usage sample in WattHour
  if (cmd == "smpl") {
    TIMSK1 &= (0 << OCIE1A); //Disable timer
    TCNT1  = 0;              //Reset timer counter
    
    prevSmpl = wattSecWhole / 3600.0 * 1000000.0;
    Serial.println(prevSmpl, 0);
    wattSecWhole = 0;
    wattSecFract = 0;
    
    TIMSK1 |= (1 << OCIE1A); //Enable timer
  }
  //Recall last sample if needed
  else if (cmd == "recall") {
    Serial.println(prevSmpl, 0);
  }
  
  //Obtain sample reading and store data (NEW CODE)
  if (getNextSmpl) {
    Wrms = emon.calcIrms(1480) * VOLTAGE_RMS;
    
    wattSecWhole += (uint32_t)Wrms;
    wattSecFract += (uint32_t)((Wrms - (double)wattSecWhole) * 10000000.0);
    
    if (wattSecFract > 10000000) {
      wattSecWhole += 1;
      wattSecFract -= 10000000;
    }
    
    getNextSmpl = false;
  }
}

//Timer interrupt used to sample power usage
ISR(TIMER1_COMPA_vect) {
  digitalWrite(LED_PIN, HIGH);
  
  getNextSmpl = true; //NEW CODE
  
  digitalWrite(LED_PIN,LOW);
}

The Arduino definitely spends less time within the ISR now, while obtaining a close 1sec sample rate. I haven’t seen any freeze-ups too, so I’d say my issue seems to be fixed. I’m still curious to figure out if there is some sort of hidden interrupt I am not aware of though.

GaM3r2Xtreme:
Although, I am left with one question. When data is being sent to the Arduino, is there some sort of interrupt being called in order to have the arriving bytes stored within the input buffer before being processed and stored into a variable?

The Arduino software includes an interrupt routine that moves each received byte from the USART to the 64 byte Serial Input Buffer until the buffer is full. If the buffer is full additional bytes will be lost.

...R

Don’t use an interrupt when you don’t have to. Use millis(), start with the BlinkWithoutDelay.

It is many times better to use millis(). With millis() you can extend it as far as you want. When you need to do 50 different things with 50 different intervals, then it can be done with millis().

If I remember correctly, the Atmega chip can not execute an interrupt while inside another interrupt.

It can but by default it doesn't and that's better because you might get even bigger problems if you try to use nested interrupts. If you know exactly what you're doing, you can allow interrupts to trigger while you're handling one.