analogRead messes up my frequency counter function

I'm trying to make a multi-function program that acts as both a frequency counter (512 Hz range) and a voltage level reader. I've written the code for the frequency counter, and it functions fine on it's own, but as soon as I add an analogRead function to test the voltage it messes up my frequency count. I've tried adding delays before and after calling the frequency function, but this caused greater error in my results. I'm thinking that the delay created by the analogRead function is interfering with the frequency counter code, but shouldn't the for loop negate that?

bool cur_Val = false;
unsigned long cur_Time = 0;
unsigned long old_Time = 0;
bool old_Val= false;
unsigned long time_arr[200];
int counter = 0;
unsigned long freq = 0;

unsigned long test_Freq();

void setup() 
{
  Serial.begin(9600);
  pinMode(5, INPUT);
  pinMode(13, INPUT);
}


void loop() 
{
  Serial.println(analogRead(5));
  Serial.println(test_Freq());
 
}

unsigned long test_Freq()
{
  for(; counter < 199;)
  {
    cur_Val = digitalRead(12);
    
    if(old_Val == 0 && cur_Val == 1)
    {
      cur_Time = micros();
      int time_Delta = cur_Time - old_Time;
      time_arr[counter] = time_Delta;
      counter++;
      old_Time = cur_Time;
    }
    
    old_Val = cur_Val;
    
    if (counter == 199)
    {
      unsigned long sum = 0;
      for(int i = 0; i < 200; i++)
      {
        sum = sum + time_arr[i];
      }
      freq = 1 / (.000001*(sum / 200));
    }  
  }
  counter = 0;
  return(freq);
}

The delay caused by the analogRead is way less than the delay due to printing the value it returns.

I'm not sure I understand your loop nesting.

pinMode(5, INPUT);

You don't do anything with pin 5 - is this important?

So do you think this delay is what's throwing off my frequency counter code? I originally wrote the frequency counter code in the main loop, but I was encountering this issue so I thought I'd try a for loop. I figured if I put the code into a for loop, and called it in a separate function it would run exclusively and avoid any time delays created by other code... but my results are identical. My frequency count is thrown off by -8Hz. I'm assuming some sort of delay occurs during the loop causing it to miss cycles of my test frequency when I add that function.

As for

pinMode(5, INPUT);

I'm using analog pin 5 for my voltage test. Maybe the pin initialization isn't necessary, but I don't think this is related to my issue.

So I tried removing Serial.println(analogRead(5)); , and replaced it with just (analogRead(5)); and the problem went away. This still begs the question of how I can return the results of the voltage test without messing up my frequency count.

Okay... well I tried increasing the serial baud rate to 57600 and now it seems to work fine! I still find it strange though...

Maybe the pin initialization isn't necessary

It isn't.
An analogue input is just that - an input, so no need to set the mode. (unless you want to use it as a digital pin, in which case, you use a different pin number).

well I tried increasing the serial baud rate to 57600 and now it seems to work fine! I still find it strange though

At 9600 baud, one character takes about 1ms to transmit, and the call is blocking...until the very last character has been written to the UART, when the functon returns.

Still not sure about the nesting.

When you say nesting, are you referring to how I call test_Freq() from within a print command? If this is the case, you'll notice that the function test_Freq() is returning the value in variable freq, which is the calculated frequency. Basically the function test_Freq() is called and returns a value to the print function.

If that's not it, let me know.

I appreciate the help on this!

When you say nesting, are you referring to how I call test_Freq() from within a print command

No, I mean how you nest the two loops in "test_Freq".

Maybe it's late, but they don't look right to me.

You're right, it does look funky... probably because it was a quick port from a looped command that didn't use a for loop. This was the old code:

bool cur_Val = false;
unsigned long cur_Time = 0;
unsigned long old_Time = 0;
bool old_Val= false;
unsigned long time_arr[200];
int counter = 0;

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

}

void loop() 
{
  
  cur_Val = digitalRead(12);
  
  //Serial.println(cur_Time);
  if(old_Val == 0 && cur_Val == 1)
  {
    cur_Time = micros();
    int time_Delta = cur_Time - old_Time;
    //Serial.println(time_Delta);
    time_arr[counter] = time_Delta;
    //Serial.println(time_arr[counter]);
    counter++;
    old_Time = cur_Time;
  }
  
  old_Val = cur_Val;
  
  if (counter == 199)
  {
    unsigned long sum = 0;
    unsigned long freq = 0;
    for(int i = 0; i < 200; i++)
    {
      sum = sum + time_arr[i];
     // Serial.println(sum);
    }
    //Serial.println(sum/100);
    freq = 1 / (.000001*(sum / 200));
    counter = 0;
    Serial.println(freq);
    Serial.println(analogRead(5));
   // Serial.println("");    
  }
}

Probably what was throwing him off was:

for(; counter < 199;)
{

It would probably be better stated as:

while (counter < 199)
{

While both pieces of code are equivalent, the second is easier to read, whereas the first is a kind of "for-loop" bastardization...

:slight_smile:

for(int i = 0; i < 200; i++)

I'm confused - is there or isn't there a valid time_arr [199]?

@cr0sh: You're right, this was pretty silly of me... I suppose some of my coding has become a bit rusty and needs some dusting off :stuck_out_tongue:

@AWOL:

Yes, there is a valid time_arr[199], I figure using < 200 still addresses it no?

Looking at the code I would simplify a few things (highlighted).

Question comes in my mind if time_arr[] is not needed as you can calculate the sum directly at very low costs. But the individual values may be usefull when extending the application.

[glow]unsigned long sum = 0;
unsigned long freq = 0;[/glow]

void loop()
{
  cur_Val = digitalRead(12);
  
  //Serial.println(cur_Time);

  if(old_Val == 0 && cur_Val == 1)
  {
    cur_Time = micros();
    int time_Delta = cur_Time - old_Time;
    //Serial.println(time_Delta);
    time_arr[counter] = time_Delta;   [glow] // not needed ??[/glow]

[glow]    // calculate the sum immediately
    sum += time_Delta;[/glow]

    //Serial.println(time_arr[counter]);
    counter++;
    old_Time = cur_Time;
  }
  
  old_Val = cur_Val;
  
  if (counter == 199)
  {
     // Serial.println(sum);
     //Serial.println(sum/100);  [glow]<== why /100 iso /200 !! 200 measurements
[/glow]

    freq = 200 * 1000000 / sum;  [glow]// simplified floating point to int math[/glow] 

    Serial.println(freq);
    Serial.println(analogRead(5));
   // Serial.println("");  
  
    // reset all
    counter = 0;
    sum = 0;
  }
}

Yes, there is a valid time_arr[199], I figure using < 200 still addresses it no

Maybe I wasn't clear; yes, there is an array element [199], but is there a valid value in it? (think "post increment and equality test")

(Somewhat academic in light of robtillart's superior solution, I know)

there is an array element [199], but is there a valid value in it?

No it isn't , look at the code-snippet below and set your mental "counter" to 198.

   time_arr[counter] = time_Delta;         [glow]COUNTER = 198[/glow]
    //Serial.println(time_arr[counter]);   [glow]FILL time_arr[198]  199 is not filled[/glow]
    counter++;                           [glow]COUNTER = 199[/glow]
    old_Time = cur_Time;
  }
  
  old_Val = cur_Val;
  
  if (counter == 199)                [glow] YES IT IS !![/glow]
  {
    unsigned long sum = 0;
    unsigned long freq = 0;
    for(int i = 0; i < 200; i++)
    {
      sum = sum + time_arr[i];        [glow]HERE [199] IS USED 
[/glow]

if (counter == 199) => if (counter == 200) would improve it

QED :slight_smile:

You guys were right on all counts. I've modified the code using your changes and it seems to be working a lot better. I have found, however, that the frequency counter still isn't quite accurate - a 512Hz signal reads 512, but a 513 Hz signal reads 512 still. A 514 Hz signal reads correctly... and I get the same results with frequencies lower than 512 Hz. Any ideas why? Thanks again for all the help!