Measuring a pulse width by triggering an ISR on sequential edges

So I’m trying to measure the pulse width from a rotary encoder. The actual encoder will provide a pulse at 250 Hz that can vary from 800 to 1500 microseconds. For now, I’m testing generating a 50% duty-cycle pulse on pin 3 from an Arduino Duemilanove, which I believe should have a frequency of 490Hz. That pin output is wired into pin 2 on an Arduino Uno that is running my code.

For various reasons, I want to handle inputs with interrupts, rather than in the main loop. So I’m using a timer ISR that fires every 12.8mS (78Hz) to check the what a switch is doing. And at the end of this ISR, I am setting up an interrupt on pin 2 to capture the start and stop of a pulse. In other words, I don’t want to constantly be looking at rotary encoder, measuring a pulse every 12.8mS, after checking on a switch, is more than fast enough. (Eventually, I need to expand this so that I’m measuring the gap between pulses too, so that I can make a correction for temperature effects on the rotary encoder.)

My immediate problem is that I’m getting pretty random pulse durations. Here is a sampling of what I print out every 100 mS: 2652, 132, 1688, 1208, 5376, 0, 272, 3868, 1352, 872, 392, 480, 4076, 1556, 1076, 596, …

And here is the relevant code for the two ISRs:

ISR (TIMER1_COMPA_vect) // timer compare interrupt service routine, switch takes at least 4 samples of the pin being in the same state before it will be declared changed.
  {
    static unsigned int fwdSwitchCounter = 0;
    static unsigned int revSwitchCounter = 0;
   
    static byte portBHistory = 0xA;        // Binary is 0b00001010, PINB1 (digital pin 9) and PINB3 (digital pin 11) are high because of the pullups
    
    byte portBNew = PINB;
    byte changedBits = portBNew ^ portBHistory;   // XOR the PortB register with the portDHistory register gives 0's on the bits that are the same, and 1's in the bits that are different 
    portBHistory = portBNew;
    
    
    // If pins  have changed state between interrupts, then switch is probably bouncing, so reset the counter for switch
    // For pins that have not changed state, the switch is not bouncing. Increment a counter that starts timing how long the switch remains in this state. 
    if ((changedBits & FWD_PIN_MASK))   
      fwdSwitchCounter = 0;       
    else                               
      fwdSwitchCounter++; 
      
    if ((changedBits & REV_PIN_MASK)) 
      revSwitchCounter = 0;            
    else                                
      revSwitchCounter++; 
     
    // Debounce the switch by waiting for at least four interrupts before declaring a state change
    if (fwdSwitchCounter >= DEBOUNCE_COUNT || revSwitchCounter >= DEBOUNCE_COUNT)  
     {
       
       // Check if the switch is in the open position, which will be true more than 99% of the time.
       if ((portBNew & FWD_PIN_MASK) && (portBNew & REV_PIN_MASK))
       {
         switchState = SWITCH_OPEN;                         
         fwdSwitchCounter = 0;
         revSwitchCounter = 0; 
       }
           
       
       else if (!(portBNew & FWD_PIN_MASK))
       {
         if (fwdSwitchCounter <= MAX_MOMENTARY_TIME)
           switchState = FWD_SWITCH_CLICK;
         else if ((fwdSwitchCounter > MAX_MOMENTARY_TIME) && (fwdSwitchCounter < STUCK_TIME))
           switchState = FWD_SWITCH_HELD;
         else
           switchState = SAFE_MODE;
       }
       
       else if (!(portBNew & REV_PIN_MASK))
       {
         if (revSwitchCounter <= MAX_MOMENTARY_TIME)
           switchState = REV_SWITCH_CLICK;
         else if ((revSwitchCounter > MAX_MOMENTARY_TIME) && (revSwitchCounter < STUCK_TIME))
           switchState = REV_SWITCH_HELD;
         else
           switchState = SAFE_MODE;
       }       
     }
     
     // Set up to measure the pulse width with another interrupt
       attachInterrupt(0, encoderInterrupt, RISING);
       encoderState = FIRST_MEASUREMENT;
   
 } // End of ISR
            

// encoder interrupt service routing
void encoderInterrupt ()
  {
    
    if (encoderState == FIRST_MEASUREMENT)
    {
      startTime = micros ();
      attachInterrupt (0, encoderInterrupt, FALLING);
      encoderState = SECOND_MEASUREMENT;
    }
  
    else if (encoderState == SECOND_MEASUREMENT)
    {
      stopTime = micros ();
      pulseDuration = stopTime - startTime;
      detachInterrupt (0);
    } 
  }// end of encoderInterrupt

I’m both pretty new to coding, and I’m very new to Arduino. One possibility that has occurred to me is that the Arduino is just going to miss the falling edge of a pulse because it takes too long to exit and then re-enter the ISR, but that would surprise me a bit. Any suggestions welcome.

The actual encoder will provide a pulse at 250 Hz...

Is it necessary to capture every pulse?

No, it's not at all necessary to capture every pulse. I'd like to look at the encoder's pulse at about the same frequency that I look at the switch to see what it's doing. So looking a pulse at 78 Hz (every 12.8mS) is more than frequent enough, hence the reason I have the counter-triggered ISR setting up the interrupt for the encoder on pin 2 and then disabling that interrupt after I've measured a pulse.

Why not use the attachInterrupt() with the CHANGE event. Depending if the pulse is low or high you maybe mark the time of the falling edge (pulse start) then on a rising edge (pulse end) subtract the current time from the marked time to determine pulse width and store this for pickup from the main code. The ISR runs continually and you sample the result at what ever speed you want from the main code.
Expanding on this, if you also mark the rising edge then you can also calculate the frequency.

Although the sample code below is for monitoring the pulse width of a Radio Control receiver and acting differently if the pulse width is above or below 1500 microseconds the principle is the same as you describe and the actions in the ISR would be applicable to what you want to do.

#include <Servo.h>
Servo myServo;

const byte rcPin = 2;  //Rx output connected to this input pin
volatile int pulseLength;  //holds the length of the input pulse.  volatile because it is updated by the ISR
volatile unsigned long pulseStart = 0;  //time the pulse started.  Used in calculation of the pulse length
volatile boolean newPulse = false;  //flag to indicate that a new pulse has been detected
const byte servoPin = 7;    //servo ouyput pin

unsigned long stepPeriod = 5;  //time between servo steps when sweeping
unsigned long stepStart = 0;  //time the sweep step started
unsigned long currentTime = 0;  //name says it all
int servoIncrement = 1;  //how many degrees to move the servo with each sweep step
const byte sweepStart = 50;  //angle to start the sweep at
const byte sweepEnd = 130;    //angle to stop the sweep at
const byte servoHome = 90;    //servo home position when not sweeping
byte servoPos = servoHome;    //set initial servo position
boolean sweeping = false;    //indicate sweeping is not initially happening

void setup()
{
  attachInterrupt(0, rcInput, CHANGE);  //when interrupt 0 (pin 2 of Uno) detects a chanhe of state execute this function
  Serial.begin(115200);
  myServo.attach(servoPin);
  myServo.write(servoHome);
}

void loop()
{
  if (newPulse)  //if we have detected a new pules
  {
    newPulse = false;  //turn off the indicator
    if (pulseLength >= 1500)  //if pulse length greater than 1500 microseconds
    {
      sweeping = true;  //turn on sweeping flag
    }
    else
    {
      myServo.write(servoHome);  //otherwise move servo to home position
      sweeping = false;  //turn off sweeping flag
    }
  }

  if (sweeping)  //if sweeping is true
  {
    currentTime = millis();  //get the current time
    if (currentTime - stepStart > stepPeriod)  //if enough time has elapsed  
    {
      stepStart = currentTime;  //save vthe current time
      servoPos += servoIncrement;  //increment the servo position
      if (servoPos <= sweepStart || servoPos >= sweepEnd)  //if servo position is past either limit
      {
        servoIncrement *= -1;  //change the servo direction
      }
    }
    myServo.write(servoPos);  //move the servo to its new position
  }
}

// the interrupt routine starts here
//it is executed at any time that pin 2 (interrupt 0 on a Uno) changes state.
void rcInput()
{
  if (digitalRead(rcPin) == HIGH)  //if the pin is HIGH
  {
    pulseStart = micros();  //save the current time in microseconds
  }
  else
  {
    if (pulseStart && (newPulse == false))  //otherwise if we are timing and this is not a new pulse
    {
      pulseLength = (int)(micros() - pulseStart);  //calculate the pulse length
      pulseStart = 0;  //reset the pulse start time to zero
      newPulse = true;  //set flag to indicate that we are timing a new pulse
    }
  }
}

The code is heavily commented as I used it as an example for a query about using an interrupt to measure the pulse width of an RC signal. Obviously you can disregard the code that sweeps or homes the servo and do what you want with the pulse width information.

Thanks for the sample code UKHeliBob. It did help me spot an error in my code, which was that I was not reseting pulseStart to zero after measuring each pulse. In the other respects, I think I'm doing something very similar to your code.

I noticed that you're only detecting whether the pulse is above or below 1500, whereas I need to accurately measure the pulse width that will vary between 900 and 1500 microseconds. It seems that I'm still struggling to accurately measure the pulse width.

I noticed that you're only detecting whether the pulse is above or below 1500,

Yes, that was what I set out to do as an example. A 1500 microsecond pulse is the mid point of an RC signal and represents the neutral position on control surfaces.

As I said in my earlier reply you can take out any code that you don't need once the pulse width has been calculated and do what you like with the figure for the pulse width.

Okay, so I'm now successfully measuring my pulse widths, but when I get down to around 400microseconds, every so often I see a pulse of 396 microseconds. This isn't critical to my code, since this low by 4 microseconds goes away by the time pulses are 480 microseconds, but I'm curious if there is an inherent reason why short pulses would occasionally be time fast by 4 microseconds?

Thanks to UKHeliBob for helping me out on this.

Yes ther is a reason. The microsecond timer only has a resolution of 4 uS so any measurement can be out by +/- 4uS.

I'm not 100% sure, but I think that you'll want your variables in ISR (fwdSwitchCounter, revSwitchCounter, static byte portBHistory) to be declared as volatile variables outside of the ISR function

(e.g. "volatile byte portBHistory = 0x00000000;" at the top of the code with the other variables)