Problems using PCINTs and Timer2 to measure two DC motors angular velocity

Hi, I am working on a project in which I need to measure angular velocity (RPM) of two DC motors (with a 5:1 gear ratio) using two magnetic encoders (12 counts per revolution) and an Arduino Nano. The project requires a sampling period as low as possible, for this reason I decided to set it to 10 ms. To realize this, I have developed a code handling the registers of the microcontroller, which should:

  • Count the edge changes in the two channels of both encoders via PCINT on ports PC4, PC5, PD4 and PD5.
  • Calculate the angular velocity of both motors (the encoder performs 60 counts per revolution with a transmission ratio of 5:1) and send them by USB serial communication every 10 ms through Timer2.
volatile unsigned int changes[2] = {0, 0};
unsigned int rpm[2] = {0, 0};
unsigned int previousRpm[2] = {0, 0}  ;

ISR(PCINT2_vect) {
  // PD PCINT
  changes[1]++;
}

ISR(PCINT1_vect) {
  // PC PCINT
  changes[0]++;
}

ISR(TIMER2_COMPA_vect) {
  for (int i = 0; i < 2; i++) {
    rpm[i] = calculateRpm(previousRpm[i], changes[i]);
    previousRpm[i] = rpm[i];
    changes[i] = 0;
    if (i == 0) {
      sendRpm('C', changes[i]);
    }
    if (i == 1) {
      sendRpm('D', changes[i]);
    }
  }
  Serial.println();
}

int calculateAverage(int x, int y) {
  return (int) (x + y) / 2;
}

int calculateRpm(int pPreviousRPm, int pChanges) {
  return (pPreviousRPm == 0) ? (pChanges * 99.5) : calculateAverage(pChanges * 99.5, pPreviousRPm);
}

void sendRpm(char pPort, int pRpm) {
  Serial.print(pPort); Serial.print(":"); Serial.print(pRpm); Serial.print(", ");
}

int main(void) {
  cli();
  //---------------------------------------------------------------------------
  // When TCCR2B is non-PWM mode, FOC2A must be set
  TCCR2B |= (1 << FOC2A);
  //---------------------------------------------------------------------------
  // Enable compare match A
  TIMSK2 |= (1 << OCIE2A);
  //---------------------------------------------------------------------------
  // Set CTC mode
  TCCR2A |= (1 << WGM21);
  //---------------------------------------------------------------------------
  // Set prescaler 1024
  TCCR2B |= (1 << CS22) | (1 << CS21) | (1 << CS20);
  //---------------------------------------------------------------------------
  // Set compare register A to 156 to count 10 ms
  OCR2A = 156;
  //---------------------------------------------------------------------------
  // Enable PCMSK1 (PCINT8 to PCINT14) and PCMSK2 (PCINT16 to PCINT23) and
  PCICR |= (1 << PCIE2) | (1 << PCIE1);
  //---------------------------------------------------------------------------
  // PD4, PD5, PC4(A4) and PC5(A5) will trigger interrupt
  PCMSK2 |= (1 << PCINT21) | (1 << PCINT20);
  PCMSK1 |= (1 << PCINT13) | (1 << PCINT12);
  //---------------------------------------------------------------------------
  // PD4, PD5, PC4(A4) and PC5(A5) as INPUT
  DDRD &= ~(B00110000);
  DDRC &= ~(B00110000);
  //---------------------------------------------------------------------------
  sei();
  //---------------------------------------------------------------------------
  Serial.begin(9600);
  //---------------------------------------------------------------------------
  while(1);
}

To perform the tests, I supply the motors with a 6V voltage, driving them to an angular velocity at around 3000 RPM. The problem is that the code gives me this output.

...
20:40:11.444 -> C:0, D:0, 
20:40:11.477 -> C:0, D:0, 
20:40:11.477 -> C:0, D:0, 
20:40:11.477 -> C:0, D:0, 
20:40:11.510 -> C:0, D:0, 
20:40:11.510 -> C:0, D:0, 
20:40:11.510 -> C:0, D:0, 
20:40:11.544 -> C:0, D:0, 
20:40:11.544 -> C:0, D:0, 
20:40:11.577 -> C:0, D:0, 
20:40:11.577 -> C:0, D:0, 
20:40:11.577 -> C:0, D:0, 
20:40:11.610 -> C:0, D:0, 
20:40:11.610 -> C:0, D:0, 
20:40:11.610 -> C:0, D:0, 
20:40:11.643 -> C:0, D:0, 
20:40:11.643 -> C:0, D:0, 
20:40:11.676 -> C:0, D:0, 
20:40:11.676 -> C:0, D:0, 
20:40:11.676 -> C:0, D:0, 
20:40:11.709 -> C:0, D:0, 
...

Initially, I set the sampling period using the Timer0, but the same problem occurred, but looking at forums I found that altering the Timer0, could affect the USB serial communication, so I decided to do it with the Timer2.

PS: I can' t set the sampling period with Timer1, because with this Timer I am generating two PWM signals for a dual-channel DC motor driver.

Everything were easier if you were using the fine Arduino framework.

Counting pulses in interrupt routines is okay, but everything in ISR(TIMER2_COMPA_vect) should be done outside the ISR.

Study some of the many rpm measuring projects in the forum to find out how everything can be done easier and more reliable.

Learn to use debug output messages for the raw and intermediate values.

1 Like

You should never do serial I/O of any sort in an interrupt routine.

Although the Arduino IDE mistakenly allows you to use Serial.print(), it will just crash on most other systems.

1 Like

As others have said, your ISR is too lengthy and should not have Serial output.

That said, there are some errors in your code which explains the 0's being sent.

You set changes[i] = 0 right before you use it as a parameter in the function. Then you are just printing that parameter.

changes[i] = 0;
    if (i == 0) {
      sendRpm('C', changes[i]);
    }
    if (i == 1) {
      sendRpm('D', changes[i]);
    }
void sendRpm(char pPort, int pRpm) {
  Serial.print(pPort); Serial.print(":"); Serial.print(pRpm); Serial.print(", ");
}
1 Like

You could use Timer2 for 8-bit PWM if you need the additional capabilities of Timer1 (16-bit, ICR) for something else.

1 Like

Hi guys, following the suggestions that you all gave me, I modified the code a little bit, and it looks like this.

#define PD_POS 1
#define PC_POS 0

volatile unsigned int changes[2] = {0, 0};
bool sampleTime = false;
unsigned int rpm[2] = {0, 0};
unsigned int previousRpm[2] = {0, 0}  ;

ISR(PCINT2_vect) {
  changes[PD_POS]++;
}

ISR(PCINT1_vect) {
  changes[PC_POS]++;
}

ISR(TIMER2_COMPA_vect) {
  sampleTime = true;
}

int calculateAverage(int x, int y) {
  return (x + y) / 2;
}

int calculateRpm(int pPreviousRPm, int pChanges) {
  return (pPreviousRPm == 0) ? (pChanges * 99.5) : calculateAverage(pChanges * 99.5, pPreviousRPm);
}

void sendRpm(int pPosition, int pRpm) {
  if (pPosition == PD_POS) {
    Serial.print("D: "); Serial.print(pRpm); Serial.print(", ");
  }
  if (pPosition == PC_POS) {
    Serial.print("C: "); Serial.print(pRpm); Serial.println(", ");
  }
}

int main(void) {
  cli();
  //---------------------------------------------------------------------------
  // Set PD4, PD5, PC4(A4) and PC5(A5) to Input configuration
  DDRD &= ~(B00110000);
  DDRC &= ~(B00110000);
  //---------------------------------------------------------------------------
  // Enable Pin Change Interrupts on ports D and C
  PCICR |= (1 << PCIE2) | (1 << PCIE1);
  //---------------------------------------------------------------------------
  // Enable Pin Change Interrupts on PD4, PD5, PC4(A4) and PC5(A5)
  PCMSK2 |= (1 << PCINT21) | (1 << PCINT20);
  PCMSK1 |= (1 << PCINT13) | (1 << PCINT12);
  //---------------------------------------------------------------------------
  //---------------------------------------------------------------------------
  // Set the mode of operation as Clear Timer on Compare Match
  TCCR2A |= (1 << WGM21);
  //---------------------------------------------------------------------------
  // Must set FOC2A when TCCR2B is non-PWM mode
  // Set 1024 prescaler
  TCCR2B |= (1 << FOC2A) | (1 << CS22) | (1 << CS21) | (1 << CS20);
  //---------------------------------------------------------------------------
  // Set Output Compare Register A to 156 to count 10 ms
  OCR2A = 156;
  //---------------------------------------------------------------------------
  // Enable Compare Match A Interrupt
  TIMSK2 |= (1 << OCIE2A);
  //---------------------------------------------------------------------------
  sei();
  //---------------------------------------------------------------------------
  Serial.begin(9600);
  //---------------------------------------------------------------------------
  while (1) {
    if (sampleTime) {
      for (int i = 0; i < 2; i++) {
        rpm[i] = calculateRpm(previousRpm[i], changes[i]);
        previousRpm[i] = rpm[i];
        sendRpm(i, rpm[i]);
        changes[i] = 0;
      }
      sampleTime = false;
    }
  }
}

Despite the changes implemented, the Serial Monitor output didn't receive any value. However, I received an output doing this.

...
while (1) {
    Serial.print("");
    if (sampleTime) {
      for (int i = 0; i < 2; i++) {
        rpm[i] = calculateRpm(previousRpm[i], changes[i]);
        previousRpm[i] = rpm[i];
        sendRpm(i, rpm[i]);
        changes[i] = 0;
      }
      sampleTime = false;
    }
  }
...

I was perplexed by not knowing why that code line inside the while loop allowed me to get an output at the Serial Monitor. Now, the issue is that the values received are inconsistent considering they should be very close to 3000 RPM. I'm receiving values like this (Ignore the zero values of angular velocity measured at Port D).

...
12:14:13.068 -> D: 0, C: 4319, 
12:14:13.068 -> D: 0, C: 3403, 
12:14:13.101 -> D: 0, C: 3343, 
12:14:13.101 -> D: 0, C: 3661, 
12:14:13.134 -> D: 0, C: 4218, 
12:14:13.134 -> D: 0, C: 3303, 
12:14:13.167 -> D: 0, C: 3193, 
12:14:13.167 -> D: 0, C: 3536, 
12:14:13.200 -> D: 0, C: 4106, 
12:14:13.200 -> D: 0, C: 3147, 
12:14:13.234 -> D: 0, C: 3066, 
12:14:13.234 -> D: 0, C: 3373, 
12:14:13.267 -> D: 0, C: 3925, 
12:14:13.267 -> D: 0, C: 3007, 
12:14:13.300 -> D: 0, C: 2896, 
12:14:13.334 -> D: 0, C: 3239, 
12:14:13.334 -> D: 0, C: 3808, 
12:14:13.367 -> D: 0, C: 2899, 
12:14:13.367 -> D: 0, C: 2842, 
12:14:13.400 -> D: 0, C: 3162, 
12:14:13.400 -> D: 0, C: 3670, 
12:14:13.433 -> D: 0, C: 4322, 
12:14:13.433 -> D: 0, C: 3454, 
12:14:13.466 -> D: 0, C: 3418, 
12:14:13.466 -> D: 0, C: 3798, 
12:14:13.500 -> D: 0, C: 4336, 
12:14:13.500 -> D: 0, C: 3411, 
12:14:13.533 -> D: 0, C: 3297, 
12:14:13.533 -> D: 0, C: 3638, 
12:14:13.566 -> D: 0, C: 4157, 
...

Testing for errors, I found that applying the same voltage to both motors there is a miscounting of the changes in Port C.

...
while (1) {
    Serial.print("");
    if (sampleTime) {
      Serial.print("Changes_D: "); Serial.print(changes[PD_POS]); Serial.print(", ");
      Serial.print("Changes_C: "); Serial.print(changes[PC_POS]); Serial.println();
      changes[PD_POS] = 0;
      changes[PC_POS] = 0;
      sampleTime = false;
    }
  }
...

Serial Monitor Outputs:

// Correct counting
...
13:12:16.512 -> Changes_D: 36, Changes_C: 0
13:12:16.545 -> Changes_D: 36, Changes_C: 0
13:12:16.578 -> Changes_D: 36, Changes_C: 0
13:12:16.611 -> Changes_D: 36, Changes_C: 0
13:12:16.645 -> Changes_D: 36, Changes_C: 0
13:12:16.645 -> Changes_D: 36, Changes_C: 0
13:12:16.678 -> Changes_D: 36, Changes_C: 0
13:12:16.711 -> Changes_D: 36, Changes_C: 0
13:12:16.744 -> Changes_D: 35, Changes_C: 0
13:12:16.777 -> Changes_D: 36, Changes_C: 0
...
...
13:13:25.484 -> Changes_D: 0, Changes_C: 82
13:13:25.517 -> Changes_D: 0, Changes_C: 82
13:13:25.551 -> Changes_D: 0, Changes_C: 82
13:13:25.584 -> Changes_D: 0, Changes_C: 81
13:13:25.617 -> Changes_D: 0, Changes_C: 82
13:13:25.650 -> Changes_D: 0, Changes_C: 82
13:13:25.683 -> Changes_D: 0, Changes_C: 82
13:13:25.717 -> Changes_D: 0, Changes_C: 82
13:13:25.750 -> Changes_D: 0, Changes_C: 82
13:13:25.783 -> Changes_D: 0, Changes_C: 82
...

You miss to read the counters with interrupts disabled (atomic access).

Perhaps it were better to measure the time between two samples and calculate the rpm using that time.

1 Like

Why are you mixing integer and floating point calculations?

there is a miscounting of the changes in Port C.

That is guaranteed to happen, because you do not protect "changes[]" from being modified by the interrupt, while it is being accessed by the main program. Make a copy with the interrupts off, and print the copy.

noInterrupts();
int c0_copy = changes[0];
interrupts();
Serial.println(c0_copy);
1 Like

Hi, I researched about atomic access and modified the code. I still have to put the print line between the while and the if, but now the counters don't increment correctly. On the Serial Monitor Output, I observed that an initial time interval the counters incremented, but after that time interval they only incremented by one.

#include <util/atomic.h>

#define PD_POS 1
#define PC_POS 0

volatile unsigned int changes[2] = {0, 0};
bool sampleTime = false;
unsigned int rpm[2] = {0, 0};
unsigned int previousRpm[2] = {0, 0}  ;

ISR(PCINT2_vect) {
  changes[PD_POS]++;
}

ISR(PCINT1_vect) {
  changes[PC_POS]++;
}

ISR(TIMER2_COMPA_vect) {
  sampleTime = true;
}

int calculateAverage(int x, int y) {
  return (x + y) / 2;
}

int calculateRpm(int pPreviousRPm, int pChanges) {
  return (pPreviousRPm == 0) ? (pChanges * 100.16) : calculateAverage(pChanges * 100.16, pPreviousRPm);
}

void sendRpm(int pPosition, int pRpm) {
  if (pPosition == PD_POS) {
    Serial.print("D: "); Serial.print(pRpm); Serial.print(", ");
  }
  if (pPosition == PC_POS) {
    Serial.print("C: "); Serial.print(pRpm); Serial.println();
  }
}

int main(void) {
  cli();
  //---------------------------------------------------------------------------
  // Set PD4, PD5, PC4(A4) and PC5(A5) to Input configuration
  DDRD &= ~(B00110000);
  DDRC &= ~(B00110000);
  //---------------------------------------------------------------------------
  // Enable Pin Change Interrupts on ports D and C
  PCICR |= (1 << PCIE2) | (1 << PCIE1);
  //---------------------------------------------------------------------------
  // Enable Pin Change Interrupts on PD4, PD5, PC4(A4) and PC5(A5)
  PCMSK2 |= (1 << PCINT21) | (1 << PCINT20);
  PCMSK1 |= (1 << PCINT13) | (1 << PCINT12);
  //---------------------------------------------------------------------------
  //---------------------------------------------------------------------------
  // Set the mode of operation as Clear Timer on Compare Match
  TCCR2A |= (1 << WGM21);
  //---------------------------------------------------------------------------
  // Must set FOC2A when TCCR2B is non-PWM mode
  // Set 1024 prescaler
  TCCR2B |= (1 << FOC2A) | (1 << CS22) | (1 << CS21) | (1 << CS20);
  //---------------------------------------------------------------------------
  // Set Output Compare Register A to 156 to count 9.984 ms
  OCR2A = 156;
  //---------------------------------------------------------------------------
  // Enable Compare Match A Interrupt
  TIMSK2 |= (1 << OCIE2A);
  //---------------------------------------------------------------------------
  sei();
  //---------------------------------------------------------------------------
  Serial.begin(9600);
  //---------------------------------------------------------------------------
  while (1) {
    Serial.print("");
    if (sampleTime) {
      ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
        for (int i = 0; i < 2; i++) {
          rpm[i] = calculateRpm(previousRpm[i], changes[i]);
          sendRpm(i, rpm[i]);
          previousRpm[i] = rpm[i];
          changes[i] = 0;
        }
      }
      sampleTime = false;
    }
  }
}

Serial Monitor Output:

15:36:52.709 -> D: 3104, C: 3054
15:36:52.709 -> D: 3054, C: 3029
15:36:52.742 -> D: 3029, C: 3016
15:36:52.742 -> D: 3016, C: 3010
15:36:52.776 -> D: 3010, C: 3007
15:36:52.809 -> D: 3007, C: 3005
15:36:52.809 -> D: 3005, C: 2604
15:36:52.842 -> D: 2604, C: 1352
15:36:52.842 -> D: 1352, C: 726
15:36:52.875 -> D: 726, C: 413
15:36:52.875 -> D: 413, C: 256
15:36:52.908 -> D: 256, C: 178
15:36:52.908 -> D: 178, C: 139
15:36:52.942 -> D: 139, C: 119
15:36:52.942 -> D: 119, C: 109
15:36:52.975 -> D: 109, C: 104
15:36:52.975 -> D: 104, C: 102
15:36:53.008 -> D: 102, C: 101
15:36:53.008 -> D: 101, C: 100
15:36:53.041 -> D: 100, C: 150
15:36:53.041 -> D: 100, C: 125
...

You still don't fully understand it. Access in an ISR is already atomic because interrupts are turned off so long. Instead protect reading the counters outside an ISR.

1 Like

I do integer and floating point calculations trying to give more resolution to the result of the operation.

For example:
36 * 100 = 3600 -> Truncating -> 3600
36 * 100.16 = 3605.76 -> Truncating -> 3605

On the other hand, I think I' ve already done the second thing you mention. What do you think?

trying to give more resolution to the result

  1. Where does a number like "100.16" come from? Out of a hat?

I think I' ve already done the second thing you mention

  1. No.
1 Like
  1. The RPM equation is:
    rpm = changes * 60 / sampleTime / changesPerRevolution
    rpm = changes * 60 / 9.984e-3 / 60
    rpm = changes * 100.16

  2. Then I don't know what you mean. Could you explain?

Which of the four lines of code I posted do you not understand?

None of this should be performed with the interrupts off (which is what ATOMIC_BLOCK does):

        for (int i = 0; i < 2; i++) {
          rpm[i] = calculateRpm(previousRpm[i], changes[i]);
          sendRpm(i, rpm[i]);
          previousRpm[i] = rpm[i];
          changes[i] = 0;
        }
1 Like

bool sampleTime = false;

This value is being changed within an ISR, and should be declared as volatile.

volatile bool sampleTime = false;

1 Like

I did these changes in the code.

...
volatile unsigned int changesCopy[2] = {0, 0};
volatile bool sampleTime = false;
...
void sendSerial(int pPosition, int pData) {
  if (pPosition == PD_POS) {
    Serial.print("D: "); Serial.print(pData); Serial.print(", ");
  }
  if (pPosition == PC_POS) {
    Serial.print("C: "); Serial.print(pData); Serial.println();
  }
}
...
  while (1) {
    if (sampleTime) {
      cli();
      changesCopy[PD_POS] = changes[PD_POS];
      changesCopy[PC_POS] = changes[PC_POS];
      changes[PD_POS] = 0;
      changes[PC_POS] = 0;
      sei();
      sendSerial(PD_POS, changesCopy[PD_POS]);
      sendSerial(PC_POS, changesCopy[PC_POS]);
      sampleTime = false;
    }
  }
...

Serial Monitor Output:

...
17:58:31.346 -> D: 30, C: 30
17:58:31.346 -> D: 60, C: 60
17:58:31.346 -> D: 30, C: 30
17:58:31.379 -> D: 61, C: 61
17:58:31.379 -> D: 30, C: 30
17:58:31.412 -> D: 61, C: 61
17:58:31.412 -> D: 30, C: 30
17:58:31.445 -> D: 30, C: 30
17:58:31.445 -> D: 61, C: 61
17:58:31.445 -> D: 30, C: 30
17:58:31.479 -> D: 61, C: 61
17:58:31.479 -> D: 30, C: 30
17:58:31.512 -> D: 61, C: 61
17:58:31.512 -> D: 30, C: 30
17:58:31.545 -> D: 61, C: 61
17:58:31.545 -> D: 31, C: 31
17:58:31.578 -> D: 61, C: 61
17:58:31.578 -> D: 30, C: 30
17:58:31.578 -> D: 30, C: 30
17:58:31.612 -> D: 61, C: 61
...

It seems likely that there are problems in the code you did not post.

#define PD_POS 1
#define PC_POS 0

volatile unsigned int changes[2] = {0, 0};
volatile unsigned int changesCopy[2] = {0, 0};
volatile bool sampleTime = false;
unsigned int rpm[2] = {0, 0};
unsigned int previousRpm[2] = {0, 0}  ;

ISR(PCINT2_vect) {
  changes[PD_POS]++;
}

ISR(PCINT1_vect) {
  changes[PC_POS]++;
}

ISR(TIMER2_COMPA_vect) {
  sampleTime = true;
}

int calculateAverage(int x, int y) {
  return (x + y) / 2;
}

int calculateRpm(int pPreviousRPm, int pChanges) {
  return (pPreviousRPm == 0) ? (pChanges * 100.16) : calculateAverage(pChanges * 100.16, pPreviousRPm);
}

void sendSerial(int pPosition, int pData) {
  if (pPosition == PD_POS) {
    Serial.print("D: "); Serial.print(pData); Serial.print(", ");
  }
  if (pPosition == PC_POS) {
    Serial.print("C: "); Serial.print(pData); Serial.println();
  }
}

int main(void) {
  cli();
  //---------------------------------------------------------------------------
  // Set PD4, PD5, PC4(A4) and PC5(A5) to Input configuration
  DDRD &= ~(B00110000);
  DDRC &= ~(B00110000);
  //---------------------------------------------------------------------------
  // Enable Pin Change Interrupts on ports D and C
  PCICR |= (1 << PCIE2) | (1 << PCIE1);
  //---------------------------------------------------------------------------
  // Enable Pin Change Interrupts on PD4, PD5, PC4(A4) and PC5(A5)
  PCMSK2 |= (1 << PCINT21) | (1 << PCINT20);
  PCMSK1 |= (1 << PCINT13) | (1 << PCINT12);
  //---------------------------------------------------------------------------
  //---------------------------------------------------------------------------
  // Set the mode of operation as Clear Timer on Compare Match
  TCCR2A |= (1 << WGM21);
  //---------------------------------------------------------------------------
  // Must set FOC2A when TCCR2B is non-PWM mode
  // Set 1024 prescaler
  TCCR2B |= (1 << FOC2A) | (1 << CS22) | (1 << CS21) | (1 << CS20);
  //---------------------------------------------------------------------------
  // Set Output Compare Register A to 156 to count 9.984 ms
  OCR2A = 156;
  //---------------------------------------------------------------------------
  // Enable Compare Match A Interrupt
  TIMSK2 |= (1 << OCIE2A);
  //---------------------------------------------------------------------------
  sei();
  //---------------------------------------------------------------------------
  Serial.begin(9600);
  //---------------------------------------------------------------------------
  while (1) {
    if (sampleTime) {
      cli();
      changesCopy[PD_POS] = changes[PD_POS];
      changesCopy[PC_POS] = changes[PC_POS];
      changes[PD_POS] = 0;
      changes[PC_POS] = 0;
      sei();
      sendSerial(PD_POS, changesCopy[PD_POS]);
      sendSerial(PC_POS, changesCopy[PC_POS]);
      sampleTime = false;
    }
  }
}

volatile unsigned int changesCopy[2] = {0, 0};
This does not need to be declared as volatile because it does not change inside the ISR.

Serial.begin(9600);

You should be printing at the fastest baudrate you can.

1 Like