Logical error problem with printing out frequency and clock

Projects Programming
1

I'm having a current problem with printing out my frequency in decimals, it compiles but its now giving me the values im not expecting, for example, when i lower my frequency to 500 mHZ or (.5 Hz), it will show 0.50 mHZ or 1 Hz and not anything like .678 or .456 considering i noticed that there was a consisent 13% error difference in my code. also another that i have is i tried to change the delay the frequency was read from every 1 second to every 10 seconds, it did delay by 10 seconds but it also ended up multiplying my frequnecy output by 10 as well this the code i used:

void signalChanged_ISR(){counterSet++;}
const float frequencyPin = A5;//A5 will be declared as an input pin
void setup() {
// put your setup code here, to run once:
Serial.begin(9600);// set transfer speed to 9600 bits per second
attachInterrupt(0, signalChanged_ISR, CHANGE);
counterSet = 0;
pinMode(frequencyPin, INPUT);//Declared A5 as an input pin
}

void loop() {
// put your main code here, to run repeatedly:
long min = 1000000; //making sure that the voltage never goes above this limit
long max = -1000000;//making sure that the voltage never goes below this point

for(int i = 1; i<1000; i++)
{
long voltage = (long) (((float)analogRead(frequencyPin))/1024.0f * 5.0f * 1000.0f);
if(voltage > max) max = voltage; if( voltage < min) min = voltage;
delayMicroseconds(10000);//this will delay the reading by 1 second
}
Serial.print(float(counterSet/2)); Serial.print(" Hz at "); Serial.print(min); Serial.print("..."); Serial.print(max); Serial.println("mV");// this will print out the wording of the frequency that this many millvolts
counterSet = 0;
}

are there any simple solutions or tips that i can do to fix this.

2

are there any simple solutions

Use Tools + Auto Format to fix your piss-poor indenting.
Post code that actually compiles.
Use code tags when you do that.

sketch_oct19a.ino: In function 'void signalChanged_ISR()':
sketch_oct19a:1: error: 'counterSet' was not declared in this scope
sketch_oct19a.ino: In function 'void setup()':
sketch_oct19a:7: error: 'counterSet' was not declared in this scope
sketch_oct19a.ino: In function 'void loop()':
sketch_oct19a:22: error: 'counterSet' was not declared in this scope

Your delayMicroseconds() call is not taking into account the time that analogRead() takes, or the time that the floating point arithmetic takes. I can't imagine why you need to perform all that floating point arithmetic, anyway. Doing that on the minimum reading and maximum reading makes some sense. Doing it on every reading does not.

The maximum value that voltage will have is 5000. Why do you need a long to store that?

3

Please edit your post to include code tags. See How to use this forum. While you are at it, please tidy up the code. Blank lines are free, indent it like you should (press Ctrl + T) and a '{', '}' or ';' is always followed by a new line! Now you are just cramming everything in.

    delayMicroseconds(10000);//this will delay the reading by 1 second

No it does not... It delays it with 10ms. So same as

delay(10);

And after 1000 readings you have passed 10 seconds, not 1... And this time is also far from exact. Because a loop will take more then the 10ms delay. It's the 10ms + the time the reading and compare takes. In the end this will probably a noticeable error. To overcome this, drop delay() (as always) and start using millis().

And you never declare counterSet. What is it's use anyway? And keep in mind, because you use it in your main code and the ISR that it sould be declared volatile. And give the pin a name please. Now you just hard coded it to interrupt 0 aka pin2 on a Uno.

It's great you try to fix your decimal point problem by using mV instead of V. But drop all the stupid float calculations.

unsigned long voltage = analogRead(frequencyPin) * 5 * 1000 / 1024;

Or, to correctly round the answer

unsigned long voltage = (analogRead(frequencyPin) * 5 * 1000 + 512) / 1024;

Keep in mind the resolution is 5mV

Speaking about resolution, that's where your error comes from. With a 500mHz signal, how many pulses do you expect in 1 second? And for a signal of 450mHz? You can only see a real pulse, aka counterSet (shouldn't it read setCounter?) is a integer aka whole number. Dividing it by 2 will give you a whole number as well. Casting it to float afterwards doesn't help you.

So for low frequency you need to use a longer window to count pulses to reduce the error. And do the same as for the volt, use mHz as a value. So the math will be

mFreq = counterSet * 1000 / (2 * windowSize);
//with windowSize the time you counted pulses in in seconds
//or to compensate for the wrong rounding
mFreq = (counterSet * 1000 + windowSize) / (2 * windowSize);

Btw, I have no idea why you call it a frequencyPin. I see no relation to frequency with that pin...