1

I was working on a program to measure the pulse width , for that I thought of using Timer 2 in Arduino mega along with pin change interrupt,

The program I have written is as follows

volatile float ovfCount = 0;


typedef struct{
  float curr_ovfCount,prev_ovfCount;
  uint8_t curr_tcnt2, prev_tcnt2;
  uint16_t width;
  bool stateHigh = true;
}Pulse;

Pulse ch1, ch2, ch3, ch4, ch5, ch6;


void setup() {
  Serial.begin(115200);

  /*
   * Pin Change Interupt for 
   * measuring the pulse width 
   * from the receiver
   * 
   */

  DDRK = 0; // A8:A15 -> direction as Input
  PORTK = B00111111; // A8:A15 -> pullupResistor

  PCICR = B00000100; // activating 2 nd PCINT
  PCIFR = 0; // resetting the flags
  PCMSK2 = B00111111; // activating from A8:21 :: 6

  /*
   * 8 bit timer setup for counting
   * timer 2 is used
   */
   TCCR2B=0x00;
   TCCR2A = 0x00; // wave generation is normal:: all zeros
   TCNT2 = 0x00;
   TIFR2 = 0x00; // resetting all flags
   TIMSK2 = 0x01; //timer overflow enable
   TCCR2B = 0x01; // no prescaling 
  
}

ISR(TIMER2_OVF_vect){
  ovfCount++;
}

ISR(PCINT2_vect){
  float ovf_count = ovfCount;
  uint8_t tcnt2 = TCNT2;

  if( (PINK & 1 << PINK0) & ch1.stateHigh ){
    //Low-High
    ch1.prev_ovfCount = ovf_count;
    ch1.prev_tcnt2 = tcnt2;
    ch1.stateHigh = false;
  } else if( ! ((PINK & 1 << PINK0) & ch1.stateHigh)){
//    High-Low
    ch1.curr_ovfCount = ovf_count;
    ch1.curr_tcnt2 = tcnt2;
    ch1.width = (256.0*(ch1.curr_ovfCount - ch1.prev_ovfCount) + (ch1.curr_tcnt2 - ch1.prev_tcnt2) ) * 1000 / 16e6;
    ch1.stateHigh = true;
  }

}


void loop() {
  Serial.println(ch1.width);
  pinMode(13, OUTPUT);
  digitalWrite(13,!digitalRead(13));
  delay(1000);

} 

and the output I am getting is ,

16:22:05.667 -> 0
16:22:05.847 -> 927
16:22:06.826 -> 927
16:22:07.820 -> 1001
16:22:08.847 -> 1001
16:22:09.837 -> 1001
16:22:10.843 -> 1001
16:22:11.835 -> 1002
16:22:12.863 -> 1002
16:22:13.871 -> 1002
16:22:14.854 -> 1002
16:22:15.873 -> 1003
16:22:16.875 -> 1003
16:22:17.887 -> 1003
16:22:18.860 -> 1003
16:22:19.875 -> 1003
16:22:20.867 -> 1003
16:22:21.907 -> 1003
16:22:22.895 -> 1003
16:22:23.922 -> 1008
16:22:24.921 -> 1008
16:22:25.938 -> 1008
16:22:26.922 -> 1008
16:22:27.939 -> 1008
16:22:28.961 -> 1008
16:22:29.946 -> 1008
16:22:30.971 -> 1008
16:22:31.993 -> 1008
16:22:32.974 -> 1008
16:22:34.011 -> 1008
16:22:35.016 -> 1008
16:22:36.026 -> 1008
16:22:37.012 -> 1008
16:22:38.024 -> 1008
16:22:38.939 -> 1008
16:22:39.802 -> 906
16:22:40.638 -> 906
16:22:41.508 -> 844
16:22:42.345 -> 844
16:22:43.189 -> 842
16:22:44.059 -> 842
16:22:44.905 -> 843
16:22:45.750 -> 843
16:22:46.593 -> 842
16:22:47.428 -> 842
16:22:48.305 -> 842
16:22:49.139 -> 842
16:22:50.011 -> 845
16:22:50.839 -> 845
16:22:51.707 -> 842
16:22:52.539 -> 842
16:22:53.419 -> 842
16:22:54.260 -> 842
16:22:55.111 -> 842
16:22:55.975 -> 842
16:22:56.791 -> 842
16:22:57.642 -> 842
16:22:58.492 -> 842
16:22:59.372 -> 842
16:23:00.212 -> 842
16:23:01.046 -> 842
16:23:01.918 -> 842
16:23:02.775 -> 842
16:23:03.600 -> 849
16:23:04.473 -> 849
16:23:05.329 -> 842
16:23:06.159 -> 842
16:23:07.022 -> 842
16:23:07.862 -> 842
16:23:08.709 -> 842
16:23:09.574 -> 842
16:23:10.412 -> 843
16:23:11.292 -> 843
16:23:12.116 -> 848
16:23:12.996 -> 848
16:23:13.899 -> 848
16:23:14.779 -> 848

The output I am expecting is 1000 as the connection to the PCINT is directly form the digital pin13 of the Arduino and it is made to blink at that interval

I want to know

  • What mistake I am making here in measuring the counts
  • How can I generalise this method for measuring width of 6 channels simultaneously.

===== Edit after input from Edgar Bonet ===========

I have written a program , can you please tell me is this a good way to accomplish my requirement

volatile uint32_t ovfCount = 0;

struct Pulse {
  uint32_t last_toggle;
  uint32_t width;
  bool stateHigh;
  bool input_is_high;
  uint32_t get_width() {
    noInterrupts();
    uint32_t width_copy = width;
    interrupts();
    return width_copy;
  }
};

Pulse ch1, ch2, ch3, ch4, ch5, ch6;

void setup() {
  Serial.begin(115200);

  /*
     Pin Change Interupt for
     measuring the pulse width
     from the receiver

  */

  DDRK = 0; // A8:A15 -> direction as Input
  PORTK = B00111111; // A8:A15 -> pullupResistor

  PCIFR = 0; // resetting the flags
  PCMSK2 = B00111111; // activating from A8:21 :: 6
  PCICR = B00000100; // activating 2 nd PCINT

  /*
     8 bit timer setup for counting
     timer 2 is used
  */
  TCCR2B = 0x00;
  TCCR2A = 0x00; // wave generation is normal:: all zeros
  TCNT2 = 0x00;
  TIFR2 = 0x00; // resetting all flags
  TIMSK2 = 0x01; //timer overflow enable
  TCCR2B = 0x01; // no prescaling

  DDRH = B00111000;
  fastPWM_init();

}

void fastPWM_init() {
  //    clearing
  TCCR4A = 0; TCCR4B = 0; TCCR4C = 0;
  //    Initializing
  TCCR4A = B10101010; // OC4A, OC4B, OC4C : ICR-WGM
  TCCR4B = B00011010; // ICR-WGM prescalar : 8
  //    Setting frequency
  ICR4 = 39999;
  //    Setting PWM
  OCR4A = 2000; OCR4B = 2000; OCR4C = 2000;
}

ISR(TIMER2_OVF_vect) {
  ovfCount++;
}


ISR(PCINT2_vect) {
  uint8_t tcnt2 = TCNT2;
  uint32_t ovf_count = ovfCount;

  if ( bit_is_set(TIFR2, TOV2) && tcnt2 < 128 ) {
    ovf_count ++;
  }

  uint32_t time = ovf_count << 8 | tcnt2;

  pinChangeFunction(&ch1, time, PK0);
  pinChangeFunction(&ch2, time, PK1);
  pinChangeFunction(&ch3, time, PK2);
  pinChangeFunction(&ch4, time, PK3);
  pinChangeFunction(&ch5, time, PK4);
  pinChangeFunction(&ch6, time, PK5);

}


void pinChangeFunction(Pulse *channel, uint32_t time, uint8_t pin){
  channel->input_is_high = bit_is_set(PINK, pin);
  if (channel->input_is_high && !channel->stateHigh) {
    channel->stateHigh = true;
  } else if (! channel->input_is_high && channel->stateHigh) {
    channel->width = time - channel->last_toggle;
    channel->stateHigh = false;
  }
  channel->last_toggle = time;
}

void loop() {

  if(Serial.available() > 0){
    OCR4A = Serial.parseInt();
  }
  
  Serial.print(ch1.get_width() / 16e3);Serial.print("\t");
  Serial.println(ch6.get_width() / 16e3);
  

}

The PWM is only used for testing.

3
  • 1
    This program is doing floating point calculations (which are super-slow on AVR) within an ISR that is triggered every 16 µs. I am surprised it works at all. Feb 27 at 11:14
  • thanks for the suggestion , so what datatype should I use for overFlow count .?
    – Lawliet
    Feb 27 at 14:12
  • Pin change interrupts are slower than normal interrupts. Also, for the resolution you are working with, just use millis() or micros() to get the current timestamp instead of messing with timers.
    – Majenko
    Feb 27 at 14:27
1

There are quite a few issues with this program. The most obvious is the use of floating point calculations in interrupt context. Interrupts should be served as fast as possible, and floating point is really slow on the AVR.

Some other issues:

  • Interrupt flags should be cleared by writing a logic 1 to them, however silly it may sound.

  • A couple if & operators in ISR(PCINT2_vect) are probably meant to be &&.

  • The logic for detecting rising and falling edges is all wrong.

  • There is no need to store so much data in the Pulse objects.

  • ch1.width should be read with interrupts disabled in order to avoid a data race.

  • There is a subtle race condition when reading ovfCount and TCNT2: if the timer overflows right when ISR(PCINT2_vect) starts running, that overflow will not be counted.

Here is a version of the program that attempts to fix all these points. I also changed it for running on an Uno (with pin 13 = PB5 = PCINT5), so that I could test it:

/*
 * Pulse width via PCINT.
 *
 * https://arduino.stackexchange.com/questions/81688
 */

volatile uint32_t ovfCount = 0;

struct Pulse {
    uint32_t last_toggle;
    uint32_t width;
    bool stateHigh;
    uint32_t get_width() {
        noInterrupts();
        uint32_t width_copy = width;
        interrupts();
        return width_copy;
    }
};

Pulse ch1, ch2, ch3, ch4, ch5, ch6;

void setup() {
    Serial.begin(115200);
    pinMode(13, OUTPUT);

    /* Setup Pin Change Interrupt for detecting pulses. */
    PCMSK0 = _BV(PCINT5);  // sense change on pin 13 = PB5 = PCINT5
    PCIFR  = _BV(PCIF0);   // reset interrupt flag
    PCICR  = _BV(PCIE0);   // enable PCINT0_vect: pins PCINT[7:0]

    /* Setup Timer 2 for counting. */
    TCCR2B = 0;           // stop
    TCCR2A = 0;           // normal mode
    TCNT2  = 0;           // reset timer
    TIFR2  = _BV(TOV2);   // reset interrupt flag
    TIMSK2 = _BV(TOIE2);  // enable timer overflow interrupt
    TCCR2B = _BV(CS20);   // count at F_CPU/1
}

ISR(TIMER2_OVF_vect) {
    ovfCount++;
}

ISR(PCINT0_vect) {
    uint8_t tcnt2 = TCNT2;
    uint32_t ovf_count = ovfCount;

    // Was there an overflow that has not yet been accounted for?
    if (bit_is_set(TIFR2, TOV2) && tcnt2 < 128) {
        ovf_count++;
    }

    uint32_t time = ovf_count << 8 | tcnt2;

    bool input_is_high = bit_is_set(PINB, PB5);
    if (input_is_high && !ch1.stateHigh) {  // low -> high
        ch1.stateHigh = true;
    } else if (!input_is_high && ch1.stateHigh) {  // high -> low
        ch1.width = time - ch1.last_toggle;
        ch1.stateHigh = false;
    }
    ch1.last_toggle = time;
}

void loop() {
    digitalWrite(13, HIGH);
    delay(1000);
    digitalWrite(13, LOW);
    delay(200);
    Serial.println(ch1.get_width() / 16e3, 5);
}

And here is the output:

1000.02349
1000.01959
1000.01916
1000.02496
1000.01959
1000.02032
1000.01947
1000.02795
1000.02642
1000.01855
1000.01916
1000.02105
1000.02490
1000.01782
1000.03033
1000.01898
...

The printed values are larger than 1000 because of the time needed to execute digitalWrite().

Also note that an 8-bit timer running at the full CPU speed overflows every 16 µs. That's quite a lot of pressure on the CPU, and many operations will take longer that expected because of these interrupts. If I wanted to time something with single-cycle resolution, I would reach for a 16-bit timer. And I would prefer using input capture, although sadly only two input capture channels are available on the Mega (on pins 48 and 49).


Edit: Answering the questions in the comments.

please have a look at my question edits and please advice.

I did not test it, but it seems to me this code should work. I would prefer, however, having a different interrupt for each channel. Otherwise, the successive calls to pinChangeFunction() can make the execution of this ISR quite long.

Also note that Pulse::input_is_high serves no useful purpose. Storing this value in the object slows down the ISR, as it has to access memory, whereas a local variable would be kept in a (much faster) internal CPU register.

what will we do if we have 8 channels as in the radio receiver?

You could use 6 external interrupts and 2 pin change interrupts.

Why did you take tcnt2 < 128 when tcnt2 max is 256?

This one is tricky. I'll try to explain the whole logic of the test.

Consider the following naive version of the ISR:

ISR(PCINT0_vect) {
    // ← A
    uint8_t tcnt2 = TCNT2;
    uint32_t ovf_count = ovfCount;
    uint32_t time = ovf_count << 8 | tcnt2;
    // ...
}

There is a small chance that Timer 2 overflows at point A, i.e. while this ISR is executing its prologue and has not yet read TCNT2. If this happens, the TIMER2_OVF ISR will not run immediately, because we are already executing an ISR, and ISRs do not nest. Then, ovfCount is not updated. If this happens, we then read a value of TCNT2 affected by the overflow, and a value of ovfCount that does not account for that overflow. We then combine together those inconsistent values into a time value that misses 256 timer ticks.

We can detect this situation by reading the TOV2 flag in TIFR2. This flag is set only when there is a pending interrupt request for TIMER2_OVF, i.e. if the counter has overflowed and that overflow has not yet been accounted for in ovfCount. Thus the “fixed” ISR looks like this:

ISR(PCINT0_vect) {
    // ← A
    uint8_t tcnt2 = TCNT2;
    // ← B
    uint32_t ovf_count = ovfCount;
    // ← B
    if (bit_is_set(TIFR2, TOV2)) {
        ovf_count++;
    }
    uint32_t time = ovf_count << 8 | tcnt2;
    // ...
}

With this, if the overflow happens at point A, we manage to compute the proper time. But consider now what would happen if the timer overflows at either of the points marked B, i.e. after reading TCNT2 but before reading TOV2. If this happens, we read the pre-interrupt values of both TCNT2 and ovfCount. No correction is thus needed for computing a consistent time. Yet, TOV2 is raised when we test it: we then apply an unneeded correction, and we end up with a time that is too large by 256 ticks.

In order to fix this problem, we have to make sure the correction is applied if the overflow happens at point A, but not if it happens at point B. How can we tell apart those two situations? The answer is: by looking at the value recorded in tcnt2. In case A, the counter was read right after the overflow, so it had a very small value (close to 0) when it was copied to tcnt2. In case B, the counter was read when it was just about to reach the overflow, so it had a very large value (close to 255). If TOV2 is set when we read it, this means an overflow happened during the very beginning of the current ISR, very close to the point where we read TCNT2. So the value recorded in tcnt2 has to be either very small or very large. It cannot be in the middle (close to 128).

So we have to decide whether tcnt2 is very small or very large. There are many possible values that could be used as a threshold. The most natural is the mid range: 128. It is also the most efficient to compute, as the compiler can optimize the comparison into a simple test of the most significant bit. Thus the final version of the test:

    if (bit_is_set(TIFR2, TOV2) && tcnt2 < 128) {
        ovf_count++;
    }
9
  • How to modify the same for using with 6 channels .? sorry but I am getting confused ..
    – Lawliet
    Feb 27 at 14:49
  • @Lawliet: You have only 3 pin change interrupts. Reading 6 channels would make the ISRs more complex, as they would need to find out which channels triggered the interrupt. I would rather use regular external interrupts, as there are 6 of them available on the Mega (INT0–INT5). You could use a template to void repeating code. Feb 27 at 16:19
  • please have a look at my question edits and please advice.
    – Lawliet
    Feb 27 at 16:30
  • Also what will we do if we have 8 channels as in the radio receiver. ?
    – Lawliet
    Feb 27 at 16:32
  • 1
    @Lawliet: See amended answer. Feb 27 at 22:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.