1

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,"Humidity:%d \n",f); 
  Serial.print(msg);
}
  • Your code works for me but, as @Jot noted, it would be more robust if you make f volatile and read it with interrupts blocked. – Edgar Bonet Jun 29 '17 at 20:27
2

As stated in my comment, your code is kind of fine. Id mostly works, but it looses a few microseconds on each iteration. If you are looking at multi-kHz frequencies, you probably want to avoid loosing microseconds. The solution is simple: never reset the counter. I changed you ISR as follows:

uint16_t previous_counter;

void pin_ISR(){
    uint16_t counter = TCNT1;
    f = 2000000/(counter - previous_counter);
    previous_counter = counter;
}

and now it gives the correct frequency... on average. There are some variations though, dues mostly to delays caused by other interrupts.

Edit 1: If you want to avoid the fluctuations caused by other interrupts, your best bet is probably to use the input capture feature of Timer 1.

Edit 2: Here is why your measurement was somewhat off. Your original ISR essentially does something like this:

void pin_ISR() {
    uint16_t temporary_value = TCNT1;
    loose_some_time();
    TCNT1 = 0;
}

The time you loose in the ISR is the part of the signal period you are not measuring, which means you are underestimating the period by that much. In the ISR you posted, this is mostly the time taken by a 32-bit division, which is quite significant. A simple improvement would be to clear TCNT1 right after reading it:

void pin_ISR() {
    uint16_t temporary_value = TCNT1;
    TCNT1 = 0;
    loose_some_time();
}

and now the average period you measure is almost right. It is not perfect though, as the whole reading and writing of TCNT1 takes 8 CPU cycles.

The alternative I am suggesting, where you never clear the timer, is the correct way of measuring a period with a timer interrupt: you don't loose a single CPU cycle. The best way is to use input capture: you get rid of the software-induced jitter.

Note: You should also take care of the very valid points raised in Jot's answer, about making f volatile and reading it with interrupts disabled.

  • This worked perfectly. Thanks! But I don't get it, my timer was ticking on a 2Mhz clock, and the number of TCNT is usually around 297, 298 if I change the code to display that, and with the way you told me to change it, it still remains very much the same. I don't know why this is happening! Although I must say that I am simulating my program in Proteus.maybe it's a bug. – Farhood ET Jun 29 '17 at 22:12
0

Your approach is basically a reciprocal counter and it can work, if you start and stop the timer in the isr. The logic is essentially this:

If timer is on stop the timer and save the timer count. Else initialize the timer and start counting.

For counting a wide range of frequencies, you may need to consider timer over flow.

Edit: any solution that reads a multi byte counter while it is running will produce the wrong results. Google atomicity to understand why.

0

Did you find an example that works that way ? Is it, for example, allowed that the TCNT1 is cleared in an interrupt routine? The TCCR1A is cleared, but not set to a value ?
[edit] Clearing TCCR1A is okay. Clearing TCNT1 is also okay, but can be improved as @EdgarBonet shows.

The calculation uses a long of 2MHz, but the others are 16 bits. Could you force that calculation with long ? f = int( 2000000L / long( TCNT1));
[edit] It is not needed to force the compiler into a 'long' calculation, it already does so.

The 'f' variable is used in an interrupt and outside that interrupt. It must be made 'volatile': volatile int f=0;

The 'f' is used in the loop, but the interrupt could happen when 'f' is only half read or half written. The ATmega328P is a 8-bit microcontroller and 'f' is a 16-bits variable. I suggest to disable the interrupts and make a copy.

noInterrupts();
int fcopy = f;
interrupts();
sprintf(msg,"Humidity:%d \n",fcopy);

What is the range that you want to measure ?

Do you know that there are good libraries to measure a frequency ? They are the FreqMeasure Library and the FreqCount Library.

[ADDED]
Try to slow down the loop with a delay. If you fill the transmit buffer of the Arduino Uno TX, then the Serial.println wait for each character if there is a place in the transmit buffer.

  • You wrote: “Is it, for example, allowed that the TCNT1 is cleared in an interrupt routine?” Of course, why not? “The TCCR1A is cleared, but not set to a value?” It is set to zero, which is a perfectly valid value. “Could you force that calculation with long?” That's exactly what the compiler already does as part of the usual arithmetic conversions. – Edgar Bonet Jun 29 '17 at 20:32
  • I'm measuring a frequency between 5khz to 7khz, it is showing me 8-9 khz for signals in the ranges of 6500~6600 hz. I knew about the libraries but the code was so simple that I thought it would be a piece of cake, which was not. – Farhood ET Jun 29 '17 at 20:41
  • That's not way off, it's actually kind of OK. You are just loosing about 50 µs. – Edgar Bonet Jun 29 '17 at 20:56
  • @EdgarBonet, you are correct. The compiler did make a 'long' calculation, but I like to force a long calculation in code. I like to tell the compiler what to do, instead of letting the compiler think about it, because it's so easy to make a mistake in the code. The result of clearing TCNT1 depends on the frequency range, that's why I asked. – Jot Jun 29 '17 at 21:02
  • @EdgarBonet It's way off because the number of TCNT and the division does not make sense. I usually get about 297~307 in the TCNT on the interrupts and if you divide it by hand it should not give you 8000! – Farhood ET Jun 30 '17 at 7:21
0

So I have had asked this question on the Arduino forum and some one has fixed my code which gives me the accuracy that I expect. The problem was that the division was better not to be in the ISR routine but in the main loop. Here's the link to that answer because that guy should be credited as he helped me big time. https://forum.arduino.cc/index.php?topic=486573.0

Thanks to all who replied to me, I have learned many things about ISR's and their limitations.

  • Note that the answer you got in the forum is almost good. See my expanded answer for why it's still not as accurate as it could be. – Edgar Bonet Jun 30 '17 at 7:53
  • @EdgarBonet Yeah I merged his code with yours and it works very accurate, and error about 4~6 Hzs which is awesome. Thank you thank you so much. – Farhood ET Jun 30 '17 at 10:03

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.