frequency meter does not work as it is supposed to.

Hello there!

I am trying to build a frequency meter using Interrupts and Timer1, which works as if the system receives an Interrupt it would get the timer’s data and would calculate

Signal’s Frequency = 1 / (Counter * Timer’s Period) => F_s = F_t/TCNT

but the result is way off, and it does not even make sense. I have had altered my code to look that if the timer’s data is not valid or sth, but the number that the timer counter has counted is sth that I expect.

Here is my code:

int f=0;
char msg[50];

void setup()
{
  Serial.begin(115200);
  noInterrupts();           // disable all interrupts
  pinMode(13, OUTPUT);
  digitalWrite(13, LOW);
  TCCR1A = 0;
  TCCR1B = 0;
  OCR1A = 0;
  OCR1B = 0;
  TCCR1B |= (0<<CS12)|(1<<CS11)|(0<<CS10);    // Clock 2MHZ 
  TCNT1  = 0;  
  TIMSK1 = 0; // disable timer overflow interrupt 
  interrupts(); 
  attachInterrupt(1,pin_ISR,RISING); 
}

void pin_ISR(){
  //digitalWrite(13,!digitalRead(13));
  f = 2000000/TCNT1;
  TCNT1=0;
}

void loop()
{
  sprintf(msg,"Freq:%d \n",f); 
  Serial.print(msg);
}

f = 2000000UL/TCNT1 And now?

AWOL:
f = 2000000UL/TCNT1 And now?

still off. I'm simulating my board in Proteus(I don't have the board to generate the signals) and with a given pulse with a frequency of 6k, it shows 7.78k, which is way off.

I don't have a simulator and I don't trust them, so I can't help you any more.

There are many issues with your code, beginning with the declaration of f as an int instead of volatile int.The timer should be stopped and restarted when the interrupt picks up TCNT1, and interrupts should be disabled and renenabled when the values modified in the ISR are accessed.

That said, the cause of your reported 7.78K instead of 6K is the division of the 2000000 within the ISR. I’ve tried it several ways, but can’t get it to be accurate. I don’t understand the root cause but when the division is put in loop you see 6K.

I am testing with two arduinos. One generating tone at 6K and the other running your code.

int f=0;
char msg[50];

void setup()
{
  Serial.begin(115200);
  noInterrupts();           // disable all interrupts
  pinMode(13, OUTPUT);
  digitalWrite(13, LOW);
  TCCR1A = 0;
  TCCR1B = 0;
  OCR1A = 0;
  OCR1B = 0;
  TCCR1B |= (0<<CS12)|(1<<CS11)|(0<<CS10);    // Clock 2MHZ 
  TCNT1  = 0;  
  TIMSK1 = 0; // disable timer overflow interrupt 
  interrupts(); 
  attachInterrupt(1,pin_ISR,RISING); 
}

void pin_ISR(){
  //digitalWrite(13,!digitalRead(13));
  //f = 2000000/TCNT1;
  f = TCNT1;
  TCNT1=0;
}

void loop()
{
  f = 2000000/f;
  sprintf(msg,"Freq:%d \n",f); 
  Serial.print(msg);
}

cattledog:
There are many issues with your code, beginning with the declaration of f as an int instead of volatile int.The timer should be stopped and restarted when the interrupt picks up TCNT1, and interrupts should be disabled and renenabled when the values modified in the ISR are accessed.

That said, the cause of your reported 7.78K instead of 6K is the division of the 2000000 within the ISR. I’ve tried it several ways, but can’t get it to be accurate. I don’t understand the root cause but when the division is put in loop you see 6K.

I am testing with two arduinos. One generating tone at 6K and the other running your code.

int f=0;

char msg[50];

void setup()
{
  Serial.begin(115200);
  noInterrupts();          // disable all interrupts
  pinMode(13, OUTPUT);
  digitalWrite(13, LOW);
  TCCR1A = 0;
  TCCR1B = 0;
  OCR1A = 0;
  OCR1B = 0;
  TCCR1B |= (0<<CS12)|(1<<CS11)|(0<<CS10);    // Clock 2MHZ
  TCNT1  = 0; 
  TIMSK1 = 0; // disable timer overflow interrupt
  interrupts();
  attachInterrupt(1,pin_ISR,RISING);
}

void pin_ISR(){
  //digitalWrite(13,!digitalRead(13));
  //f = 2000000/TCNT1;
  f = TCNT1;
  TCNT1=0;
}

void loop()
{
  f = 2000000/f;
  sprintf(msg,“Freq:%d \n”,f);
  Serial.print(msg);
}

Thank you very much! This fixed my code and it’s currently working brilliantly!

I’m curious tho about what exactly should not be in ISR routine usually? to avoid future issues.

I'm curious tho about what exactly should not be in ISR routine usually? to avoid future issues.

The isr should be kept as short as possible, and there should be nothing within the isr which requires interrupts(e.g. Serial printing, delay()). Interrupts are temporarily disabled within the isr. See Nick Gammon's turorial on interrupts Gammon Forum : Electronics : Microprocessors : Interrupts

Because accessing the variable 'f' is not an atomic operation, you should really disable interrupts in your loop () function and make a local copy of it, then reenable interrupts.

@AWOL

Do you understand why the division does not work within the ISR?

When testing the OP's program I tried the following which still gave the incorrect frequency.

void pin_ISR(){
  //digitalWrite(13,!digitalRead(13));
  //f = 2000000/TCNT1;
  f = TCNT1;
  f = 2000000/f
  TCNT1=0;
}

I tested with the UL specifer as well as
a) f declared as volatile,
b) with the protected transfer from the isr as you suggest
c) with the timer stopped when assigning TCNT1.

I could never get 6K unless I pulled the division into loop().

Try to reset TCNT1 BEFORE the division - as soon as it is read. This way ticks needed for division are "skipped" leading to lesser count = higher frequency. Input capture would be better for this.

cattledog:
@AWOL

Do you understand why the division does not work within the ISR?

When testing the OP’s program I tried the following which still gave the incorrect frequency.

void pin_ISR(){

//digitalWrite(13,!digitalRead(13));
  //f = 2000000/TCNT1;
  f = TCNT1;
  f = 2000000/f
  TCNT1=0;
}




I tested with the UL specifer as well as 
a) f declared as volatile, 
b) with the protected transfer from the isr as you suggest 
c) with the timer stopped when assigning TCNT1. 

I could never get 6K unless I pulled the division into loop().

I’m wondering if the compiler “optimizes out” the f = TCNT1 and essentially executes the code as f = 2000000/TCNT1 as in the original post. Since division is a fairly slow operation and TCNT1 hasn’t been stopped, division may be making multiple accesses to TCNT1 which changes between steps of the division operation. If this is the case, stopping the timer prior to the division operation in the interrupt routine should fix it.

Moving the division operation out of the interrupt routine would likely make a non-volatile copy of TCNT1 somewhere that then gets used in the division operation.

Per this hypothesis, as suggested in post #9, the following might also work since the compiler shouldn’t imagine that f and TCNT1 are equivalent at the time of the division operation.

  f = TCNT1;
  TCNT1=0;
  f = 2000000/f ;

If this is the case, stopping the timer prior to the division operation in the interrupt routine should fix it.

Surprisingly, it doesn’t. Thee root cause of the anomaly does not appear to a running TCNT1.

I believe that there must be some sort of compiler optimization going on. Here is the code with the volatile declaration of f , the timer stopped when assigning TCNT1 to f, and a protected copy of the value used in loop for display

I have placed the division 2000000UL/f in three possible places. – two within the isr and one in loop. One division within the isr, as well as the division in loop() gives the expected 6K result. The other division within the isr gives the 7.8K.

volatile int f = 0;
int copy_f = 0;
char msg[50];

void setup()
{
  Serial.begin(115200);
  noInterrupts();           // disable all interrupts
  pinMode(13, OUTPUT);
  digitalWrite(13, LOW);
  TCCR1A = 0;
  TCCR1B = 0;
  OCR1A = 0;
  OCR1B = 0;
  TCCR1B |= (0 << CS12) | (1 << CS11) | (0 << CS10); // Clock 2MHZ
  TCNT1  = 0;
  TIMSK1 = 0; // disable timer overflow interrupt
  interrupts();
  attachInterrupt(1, pin_ISR, RISING);
}

void pin_ISR() {
  TCCR1B = 0;///stop timer
  f = TCNT1; //approx 333 counts at .5 us prescaler
  TCNT1 = 0;
  f = 2000000UL / f; //gives frequency approx 7.8K
  TCCR1B |= (0 << CS12) | (1 << CS11) | (0 << CS10); //restart timer
  //f = 2000000UL / f; //gives frequency approx 6K
}

void loop()
{
  noInterrupts();
  copy_f = f;
  interrupts();
  //copy_f = 2000000UL/copy_f;//gives frequency 6K
  sprintf(msg, "Freq:%d \n", copy_f);
  Serial.print(msg);
}

Test code from a second UNO jumpered to pin 3

void setup() {
pinMode(11,OUTPUT);
tone(11,6000);
}

void loop() {}

Input capture would be better for this.

I agree. But its not the external interrupt latency causing the anomalous behavior.

OMG no optimization or volatile or some other nonsense. If you read TCNT1 and THEN waste time by division and THEN clear TCNT1 you miss the time spent on division in counting time of one period.

EDIT: you measure time from TCNT1 = 0 to f = TCNT1. Also nonconstant interrupt latency (other interrupt being serviced) introduces a glitch. That is why input capture is better.

If you read TCNT1 and THEN waste time by division amd THEN clear TCNT1 you miss the time spent on division in counting time of one period.

I don't understand. The anomalous result shows in the code above which clears TCNT1 before the division.

f = TCNT1; //approx 333 counts at .5 us prescaler
  TCNT1 = 0;
  f = 2000000UL / f; //gives frequency approx 7.8K