Solved: reading PWM signal: the "noInterrupts()" function crashes my code!

Update 9 Feb. 2014:
In order to enhance the accuracy and precision of my code below, here's my timer to replace "micros()." It has a precision of 0.5us, so your PWM reads will be much more precise & accurate now. Simply replace micros() with my "get_T2_count()" function, then after taking the time difference, divide by 2 to convert from "counts" (with units of 0.5us per count) to microseconds (us).
ElectricRCAircraftGuy.com--RC, Arduino, Programming, & Electronics: Arduino micros() function with 0.5us precision - using my Timer2_Counter Library


I am using an attachInterrupt method to read in a PWM (pulse width modulation) signal from a standard hobby RC receiver. My code below is functioning properly, but I don't understand why placing the "noInterrupts" command where I originally had it will completely crash the code and make it not work! I have commented it out for the code to work. You can see the commented out line denoted by the following statement: "//turn off interrupts during data processing ////////////FOR SOME WEIRD REASON, ENABLING THIS LINE BREAKS THE ENTIRE CODE!!!"

The new location of "noInterrupts" has been moved to within my "interrupt_triggered()" function. To duplicate my problem, simply uncomment the commented out one, and comment out the one in the "interrupt_triggered()" function. Why is it doing this???

Thanks for your help!

/*
PWM Reader2
-this program is meant to read a PWM signal from an RC receiver (Rx) in order to tell you what pulse width is being sent out by the Rx on any given channel.
-this version of the code uses attachInterrupt, and is more accurate (but less precise) than using pulseIn().
--this version has an accuracy AND precision of ~+/- 4us, whereas the version using pulseIn() instead of attachInterrupt has an accuracy of approximately +0 / -10 ~ -12 us,
and a precision of 1~2us.
--Ex: in this version, if the actual high pulse width is 2000us it will read either 1996, 2000, or 2004us. However, in the version using th pulseIn function instead,
it will read something more like 1988us.

Written by Gabriel Staples
http://ElectricRCAircraftGuy.blogspot.com/
4 Nov. 2013

SETUP:
-The only input required is to connect a jumper FROM one of the signal wires on an RC reciever (Rx) (ex: the throttle signal wire) TO digital pin 2 on your Arduino.
--Also connect the grounds together (Rx ground to Arduino ground)
-Then, open the Serial Monitor to see the pulse width coming from the Rx!

*/

//Global Initializations
const int input_pin = 2;
const int LED = 13; //output LED, to blink

//make all variables volatile if they must be modified in the interrupt function
volatile int input_state = LOW; //initialize input state
volatile boolean process_data = false; //is it time to process the data?
volatile unsigned long t_start = 0; //us; the start time of a HIGH pulse
volatile unsigned long t_end = 0; //us; the end time of a HIGH pulse
volatile unsigned long t_end_old = 0; //us; the previous t_end value (ie: from the previous iteration)
volatile unsigned long duration = 0; //us; the time of a high pulse

int LED_state = LOW; //state of the indicator LED built-in to the board, and on pin 13
unsigned long dt = 0; //us; the period of the entire PWM cycle (ex: for a 50Hz PWM RC communication signal, the period, or dt = 20ms = 20000us)
unsigned long counter = 0; //loop counter
float freq = 1; //Hz; the PWM frequency

//initial calcs
//calculate length of the data arrays
int array_length = sizeof(duration)/sizeof(unsigned long);

void setup() {
pinMode(input_pin, INPUT);
pinMode(LED, OUTPUT);
attachInterrupt(0, interrupt_triggered, CHANGE); //attach an interrupt to interrupt 0 (on digital pin 2)
Serial.begin(115200);

}

void loop() {

if (process_data)
{
// noInterrupts(); //turn off interrupts during data processing //////////FOR SOME WEIRD REASON, ENABLING THIS LINE BREAKS THE ENTIRE CODE!!!!!!!!!
LED_state = !LED_state; //update
digitalWrite(LED, LED_state); //blink LED 13

//get PWM period
dt = t_end - t_end_old; //us; the PWM period in microseconds
//get PWM freq
freq = 1.0/(dt*(float)1e-6); //Hz; the PWM frequency

counter++; //update the loop counter

//output the data every ___ cycles (if you set this ___ to 'freq,' you get an output once per second)
if (counter%((unsigned int)freq/10)==0){ //if counter modulus ___ == 0 //for a 45Hz PWM signal being read, output data at ~4 Hz.
// if (counter%1==0){ //}if counter modulus ___ == 0 //output every iteration
Serial.print("High Time(us): ");
Serial.print(duration); //the high pulse time
Serial.print(", Low Time(us): ");
Serial.print(t_end - duration - t_end_old); //us, the low pulse time which occured just BEFORE the high pulse time
Serial.print(", Period(us): ");
Serial.print(t_end - t_end_old); //the total PWM period
Serial.print(", Freq(Hz): ");
freq = 1.0/((t_end - t_end_old)*(float)1e-6); //freq, Hz
Serial.println(freq);
}

process_data = false; //reset
interrupts(); //turn interrupts back on
} //end of processing data

}

void interrupt_triggered()
{
//begin interrupt code
volatile int val = digitalRead(input_pin);

if (val!=input_state){ //if a change actually occured and it was NOT just noise on the pin
input_state = val; //update the pin input state
if (input_state==HIGH) //if the pin is now HIGH
{
t_start = micros(); //capture the time stamp of the rising edge
}
else //if the pin is now LOW
{
t_end_old = t_end; //us; update this value just before you replace t_end with a new value
t_end = micros(); //capture the time stamp of the falling edge (remember, we already ensured that an input_state change DID occur)
duration = t_end - t_start; //us; the duration of the HIGH pulse
process_data = true; //update variable in order to process & output data
noInterrupts(); //turn off interrupts during data processing
}
}

}

Serial uses interrupts to manage the outgoing buffers. If you buffer fills and you try to send a character with the interrupts disabled the Serial output routines will lock up waiting for an interrupt to indicate that a character has been sent.

Interrupts are disabled by default in interrupt service routines. Disabling them again does nothing and they will be re-enabled when the ISR exits.

ah, so using noInterrupts in my interrupt function code (ISR) is useless then, from what I understand, and using noInterrupts before a Serial.print will lock my code. Thanks for this important info! I'll do some playing around.

What do I do then to keep interrupts I've added via attachInterrupt from bothering my program when I am trying to process data? In my code obviously this is not an issue, since I have so much time to process data, but if I was reading in a pulse with 10us high followed by 5us low, and I wanted to read in 5 cycles then process data, leaving interrupts on during data processing could really bog down data processing, and even overwrite variables i'm using in my data processing, right?

Interrupts take over 5 µS to enter and leave the ISR, so hoping you can not only do that, but do something useful every 5 µS is wishful thinking.

Read the link above about serial prints in interrupts, turning interrupts off, and protecting variables from interrupts.

Thanks Nick and John, your answers are very helpful. Nick, I will read your link. I have marked this thread solved since my problem is clearly the "noInterrupts" command being called just before I try to do Serial.print commands.

However, since I have two very knowledgeable people momentarily at my disposal, perhaps you could answer a few more questions for me on this topic?

  1. On a side note, is the way that I am reading in the PWM pulses a smart way of doing things, or would you recommend another approach?

  2. Also, even though this may be a silly way of doing it, how precise (& accurate) can you get simply by calling the digitalRead command as fast as possible in a loop while looking for a change in the value, as a way to detect rising & falling edges?
    --I haven't tried this yet, but I suppose I could try it just to see what happens.

  3. Lastly, how does attachInterrupt detect a rising or falling edge on a pin? I understand this is somehow "done in the hardware," but I have no idea what that means. It seems there is some hysteresis in the sampling anyway, slightly skewing the time an edge is detected.

panther3001:

  1. On a side note, is the way that I am reading in the PWM pulses a smart way of doing things, or would you recommend another approach?

I'd like to see your new code. I wouldn't be turning interrupts off like that.

  1. Also, even though this may be a silly way of doing it, how precise (& accurate) can you get simply by calling the digitalRead command as fast as possible in a loop while looking for a change in the value, as a way to detect rising & falling edges?
    --I haven't tried this yet, but I suppose I could try it just to see what happens.

If you are doing nothing else, a tight loop can be faster than interrupts. However you would have to turn off timer interrupts, in which case you wouldn't know the time between pulses.

  1. Lastly, how does attachInterrupt detect a rising or falling edge on a pin? I understand this is somehow "done in the hardware," but I have no idea what that means. It seems there is some hysteresis in the sampling anyway, slightly skewing the time an edge is detected.

attachInterrupt doesn't detect edges, it turns on the interrupt hardware. Once the programmed edge arrives (eg. rising, falling or both) then the interrupt is noted for processing when possible. When that is depends on what else the processor is doing (eg. if interrupts happen to be off right now because it is handling a timer interrupt).

You can use the timers hardware to detect exact distances between edges, see:

However a bit depends on how precise it needs to be. Surely the RC system doesn't operate at the nanosecond level?

"I'd like to see your new code. I wouldn't be turning interrupts off like that."

**--simply delete the lines where I call "interrupts()" and "noInterrupts()" and you have my new code. It works fine like that. **

"However a bit depends on how precise it needs to be. Surely the RC system doesn't operate at the nanosecond level?"

**--no, definitely don't need nanosecond precision, though 1us precision would be very nice. On RC systems, the tightest I've seen a high pulse width range for a throttle command is 1300us for 0% throttle and 1800us for 100% throttle. That means that each 1% is 5us. However, I wouldn't mind 1us precision, and my code above, since it uses micros(), only has +/-4us precision, since that's all micros() can give me. **

Either way, looks like I'll have to read up on your links. I looked briefly and I can tell you have a ton of very valuable information!

OK, so speed isn't really the essence, right? You have heaps of µS to do things, your main desire is an accurate pulse width measurement. Read one of the sketches on that page:

That uses the input capture unit to get a very accurate timing. Basically the hardware remembers the timer count at the point of the interrupt condition, thus eliminating the time taken to enter the ISR and capture it using micros().

The limitation in that sketch was not the timer accuracy, it was how fast you could store the results (TIMER1_CAPT_vect). Since in your case you have at least 1300µS this wouldn't be an issue.

You would need to modify things a bit, that sketch counts rising pulses only. In your case you need to know exactly one edge, and then the other edge. But you have plenty of time to set up to detect that. The datasheet (page 121) backs up what I said:

Measurement of an external signal’s duty cycle requires that the trigger edge is changed after each capture. Changing the edge sensing must be done as early as possible after the ICR1 Register has been read. After a change of the edge, the Input Capture Flag (ICF1) must be cleared by software (writing a logical one to the I/O bit location). For measuring frequency only, the clearing of the ICF1 Flag is not required (if an interrupt handler is used).

I think you will get accuracy with one or two clock cycles using that technique (ie. 125 nS).

I decided to test this theory. :slight_smile:

Code:

// Duty cycle calculation using input capture unit
// Author: Nick Gammon
// Date: 5 November 2013

// Input: Pin D8 

volatile boolean first;
volatile boolean triggered;
volatile unsigned long overflowCount;
volatile unsigned long startTime;
volatile unsigned long finishTime;

// timer overflows (every 65536 counts)
ISR (TIMER1_OVF_vect) 
{
  overflowCount++;
}  // end of TIMER1_OVF_vect

ISR (TIMER1_CAPT_vect)
  {
  // grab counter value before it changes any more
  unsigned int timer1CounterValue;
  timer1CounterValue = ICR1;  // see datasheet, page 117 (accessing 16-bit registers)
  unsigned long overflowCopy = overflowCount;
  
  // if just missed an overflow
  if ((TIFR1 & bit (TOV1)) && timer1CounterValue < 0x7FFF)
    overflowCopy++;
  
  // wait until we noticed last one
  if (triggered)
    return;

  if (first)
    {
    startTime = (overflowCopy << 16) + timer1CounterValue;
    TIFR1 |= bit (ICF1);     // clear Timer/Counter1, Input Capture Flag
    TCCR1B =  bit (CS10);    // No prescaling, Input Capture Edge Select (falling on D8)
    first = false;
    return;  
    }
    
  finishTime = (overflowCopy << 16) + timer1CounterValue;
  triggered = true;
  TIMSK1 = 0;    // no more interrupts for now
  }  // end of TIMER1_CAPT_vect
  
void prepareForInterrupts ()
  {
  noInterrupts ();  // protected code
  first = true;
  triggered = false;  // re-arm for next time
  // reset Timer 1
  TCCR1A = 0;
  TCCR1B = 0;
  
  TIFR1 = bit (ICF1) | bit (TOV1);  // clear flags so we don't get a bogus interrupt
  TCNT1 = 0;          // Counter to zero
  overflowCount = 0;  // Therefore no overflows yet
  
  // Timer 1 - counts clock pulses
  TIMSK1 = bit (TOIE1) | bit (ICIE1);   // interrupt on Timer 1 overflow and input capture
  // start Timer 1, no prescaler
  TCCR1B =  bit (CS10) | bit (ICES1);  // plus Input Capture Edge Select (rising on D8)
  interrupts ();
  }  // end of prepareForInterrupts
  

void setup () 
  {
  Serial.begin(115200);       
  Serial.println("Duty cycle width calculator");
  // set up for interrupts
  prepareForInterrupts ();   
  } // end of setup

void loop () 
  {
  // wait till we have a reading
  if (!triggered)
    return;
 
  // period is elapsed time
  unsigned long elapsedTime = finishTime - startTime;
  
  Serial.print ("Took: ");
  Serial.print (float (elapsedTime) * 62.5e-9 * 1e6);  // convert to microseconds
  Serial.println (" uS. ");

  // so we can read it  
  delay (500);

  prepareForInterrupts ();   
}   // end of loop

Output:

Duty cycle width calculator
Took: 97.56 uS. 
Took: 97.56 uS. 
Took: 97.56 uS. 
Took: 97.56 uS. 
Took: 97.56 uS. 
Took: 97.56 uS. 
Took: 97.56 uS. 
Took: 97.56 uS. 
Took: 97.56 uS. 
Took: 97.62 uS. 
Took: 97.56 uS. 
Took: 97.56 uS. 
Took: 97.56 uS.

Measured input:

So most of the time the sketch was 100 nS out (100 nS too low, in fact). That was inside the predicted error range (125 nS) and isn't too bad, as you can see, we are well within 1 µS of accuracy.

The comment in your code about "not just noise on the pin" doesn't make sense.

There is nothing in your interrupt routine to distinguish between a genuine pulse and noise.

Nick, thanks for doing that! It will be quite some time before I understand all that you did. I'm on a steep upward climb in my understanding of this stuff. I'm not a programmer by training or trade, and I've only been using Arduino for a few months. I am studying hard though. I saved your code for reference, and have printed out a bunch of your stuff from your website. I see you measured the input very precisely, and used that measurement as "truth" data, to be compared to the Arduino reading. What did you use to get that measurement? And how did you create the signal?

michinyon:
The comment in your code about "not just noise on the pin" doesn't make sense.

There is nothing in your interrupt routine to distinguish between a genuine pulse and noise.

The other day I was reading in pulses from a brushless motor, reading the voltage across one of the 3 motor leads, to the battery ground, after passing it through a voltage divider first to get it to below 5V. While reading this code it seemed to me that I was getting what I called "phantom reads." The motor had a high frequency PWM throttling wave on top of a low frequency wave. The high frequency PWM wave was at 8kHz. That gives it a period of 125us. With the throttle set at 99%, that means that the high-pulse is supposed to be 123.75us, and the low pulse 1.25us. I used the attachInterrupt command set to "CHANGE" in order to detect both rising and falling edges. However, I was getting these weird cases where the interrupt would trigger and create unusual results. It seems to me that the interrupt would detect a falling edge but not the subsequent rising edge, since it was too short of a time period. Nick tells me that it takes several microseconds just to enter and leave an Interrupt Service Routine anyway. That means that by the time it enters the routine, it's already missed the next edge. Frankly, however, I didn't care about the short pulses, though, and I wanted it to ignore them. I wrote this code with that in mind. I figured that if I simply polled the pin via digitalRead, within the interrupt routine, that if it was a super short pulse I really don't care about, it would poll the pin and find no change. By finding no change, I would simply choose not to execute the code in the interrupt routine, thereby avoiding unusual data.

This is what I'm referring to in my comment you referenced. I don't know if my understanding is correct or not, but that's my logic. I figure that if the pulse was too short, then the value returned by digitalRead will be equal to the previous state (High or Low), since it will have changed then changed back again before I even got to this point in the interrupt code.

void interrupt_triggered()
{
  //begin interrupt code 
  volatile int val = digitalRead(input_pin); 

  if (val!=input_state){ //if a change actually occured and it was NOT just noise on the pin

Since then, I've moved on to other things, as I've found a workable solution for the time-being, and ended up scrapping that version of motor reader code anyway. I don't know if that code I am discussing just above makes any real difference or not, but I suspect it does. Therefore, when I made this code to read RC communication PWM signals (1~2ms high pulses, 18~19ms low pulses; total frequency of 45~50Hz, depending on brand), I retained what I learned from my previous experience and left in the code just above as a way to reject "noise" [short pulses] < a few microseconds in length. Does that make sense?

panther3001:
I see you measured the input very precisely, and used that measurement as "truth" data, to be compared to the Arduino reading. What did you use to get that measurement? And how did you create the signal?

I used a Saleae logic analyzer:

The signal was created by a function generator in "pulse" mode but I could have got an Arduino to do it.

The logic analyzer had its own reported error amount (+/- 0.1%) so you could take its figures with a grain of salt as well. If it was in fact 0.1% low then the real reading would be closer to what the Arduino sketch reported. Then there is the error (if any) in the Arduino clock to take into account. I think they use a resonator as a clock reference and I think that has a certain error amount as well.

Having said all that, I think you can probably say the measurement was easily within 1% precision, and probably accurate to one µS.

Just wanted to say thanks to all of you who posted here, especially Panther 3001.
I was looking for a way to control an outcome based on PWM signal and this sketch works great for that purpose.
Also, the others who have posted here gave me allot more in-site to PWM processing.
Thanks again for posting.

Rodney, glad we could help! Also, since this post I've come a long ways in Arduino, and I have since modified Timer2 to make a new micros() function with 0.5us precision. Now, I have this code accurate to 1us, as desired. I hope to post it on my blog soon. When I do, I'll try to remember to post a link here too.

I'd also like to thank Nick Gammon. His 60 pages of info. on his site, covering Timers & Counters, & Interrupts, has been invaluable---in addition to some other sources I found online too.

RodneyD:
Just wanted to say thanks to all of you who posted here, especially Panther 3001.
I was looking for a way to control an outcome based on PWM signal and this sketch works great for that purpose.
Also, the others who have posted here gave me allot more in-site to PWM processing.
Thanks again for posting.

Rodney, here's my timer to replace "micros()." It has a precision of 0.5us, so your PWM reads will be much more consistent, precise, and accurate now.
ElectricRCAircraftGuy.com--RC, Arduino, Programming, & Electronics