Math problem while trying to average readings

[Jamesik]

Hi,

I'm having a strange issue where my Arduino doesn't seem to understand simple math equations... or is it me?
I will post just a part of the code because everything else is pretty much generic. Please help me figure out what the problem is.
I've found a solution to the problem but I'm just confused why I'm getting the strange results. Here's what is happening:

I'm reading a voltage at analog input (current measurement in a circuit) and multiplying it by a known factor to get a correct end result to be displayed. To get nice and stable readings I'm reading the input 100 times and later dividing it to get an average. It's simple and should be good enough. Here's the code:

    for(int i = 0; i < 100; i++)
     {
        current = current + (0.004887 * analogRead(pin));
     }
    current = current / 100;
    return current;

The strange part is that even when input is at 0 and "analogRead(pin)" is at 0 for all 100 reads the end value is not zero. For some reason the 0.004883 value is added to every reading as well as being used as a multiplier.

Doing it manually when (pin) = 0 and current = 0 the end result should be 0 + 0 = 0
BUT
it seems that Arduino is doing something more like this:
current = current + 0.004883 + (0.004883 * analogRead(pin))
and as result:
current = 0 + 0.004883 +0 = 0.004883

The simple solution is to use the multiplier at the end but that causes a separate issue where the added values from 100 readings cause an overflow and crash. Here's a code that works but only for small values that do not cause an overflow:

 for(int i = 0; i < 100; i++)
    {
       current = current + (analogRead(pin));
    }
   current = current / 100;
   return current * 0.004883;

Please help me understand what I'm doing wrong here. I'm just guessing now but it's a bit embarrassing to get lost on such a simple math equation

Thanks!

[system]

Please post the full code, showing datatypes of the variables.

[system]

The strange part is that even when input is at 0 and "analogRead(pin)" is at 0 for all 100 reads the end value is not zero.

How do you KNOW that analogRead() returns 0 AS AN INTERMEDIATE VALUE?

You don't, so don't make that claim.

Read the pin. Store the value in a variable. Print the value of the variable. Use the variable in the calculation. Then, you can claim to know what was read from the pin.

[nickmulder]

When you say in testing it that the input is at 0, how are you confirming that? In code, or hardware?

If it were '1' - which at 0.004883V is only a 0.1% deviation from 0V - you'd get:

current = current + (0.004883 * 1)

[nickgammon]

Help available at http://snippets-r-us.com/

[nickgammon]

The strange part is that even when input is at 0 and "analogRead(pin)" is at 0 for all 100 reads the end value is not zero.

What do you mean "the input is at 0"?

The ADC converter has an error component. It will not necessarily return 0 if you have 0V on the input. I would expect an error of at least +/- 1 from what you are inputting.

[nickgammon]

my Arduino doesn't seem to understand simple math equations... or is it me?

The Arduino does what it is told. The code you upload follows the C++ language. There are no known bugs in its "understanding". So yes, it is you.

[nickmulder]

Hrrrm,

mind you, the results should be the same either way ...

[nickgammon]

Hard to tell from a snippet, eh?

[system]

1:1:
Hrrrm,

mind you, the results should be the same either way ...

I can't imagine why you say that. The results when the reading is 0 and when the reading is 1 are going to be quite different.

[stoutfiles]

Output the analogRead value and then the variable "current" value to Serial each time through the loop. That will likely show you what's going on with the math. At the very least, you'd have something interesting to post here instead of an assumption of what the analogRead value is.

[nickmulder]

PaulS:
I can't imagine why you say that. The results when the reading is 0 and when the reading is 1 are going to be quite different.

Agree.
But I wasn't saying that. I was saying for the same input, both code snippets (as given) would result in the same answer. Which is the concern as the OP states that they give different results for the same input.

[Jamesik]

Wow, guys... I didn't expect such a lively conversation Great!
To address your doubts...

How do I know that the input is 0?
From software mostly. I did use an oscilloscope to confirm that the input pin is at 0V but to make sure I really got 0 from the A/D converter I read the values from in the input. I added a little bit of code that showed min and max values and they were all zeros. I wasn't assuming that input value was 0, I actually checked it.

I didn't post the entire code because it is rather lengthy because of my PWM code and LCD config. Since this is a math problem rather than an Arduino problem I believed that only a part of the code is needed.

Here's the code:

#include <PWM.h>
#include <LCD.h>
const int currentPin1 = A1; //current sensor 1
const int fan1 = 10;         // fan1 pin
const int currentPin2 = A2; //current sensor 2
const int fan2 = 9;         // fan2 pin
int pwm;
int i;
double currentF1 = 0;
double currentMin=0;
double currentMax=0;
double currentMin2=0;
double currentMax2=0;
double currentF2 = 0;

LiquidCrystal lcd(8, 7, 6, 5, 4, 3);

void setup()
{

  lcd.begin(20,2); 
  pinMode(fan1, OUTPUT);
  pinMode(fan2, OUTPUT);
  InitTimersSafe();

}

void loop()
{
  lcd.clear();
  for (i = 0; i<256; i++)
    {
      pwmWrite(fan1, i);
      pwmWrite(fan2, i);
      delay(80);

      lcd.setCursor(0,0);
      lcd.print(currentF1);

      currentF1=analogCurrent(currentPin1);
      if(currentF1 < currentMin) currentMin = currentF1;
      lcd.setCursor(6,0); 
      lcd.print(currentMin);
      if(currentF1 > currentMax) currentMax = currentF1;
      lcd.setCursor(11,0); 
      lcd.print(currentMax);
      
      lcd.setCursor(0,1);
      lcd.print((float)currentF2);

      currentF2=analogCurrent(currentPin2);
      if(currentF2 < currentMin2) currentMin2 = currentF2;
      lcd.setCursor(6,1); 
      lcd.print(currentMin2);
      if(currentF1 > currentMax2) currentMax2 = currentF2;
      lcd.setCursor(11,1); 
      lcd.print(currentMax2);
     
      delay(200);
    }
delay(500);

for ( i = 255; i> 0; i--)
    {
      pwmWrite(fan1, i);
      pwmWrite(fan2, i);
      delay(80);

      lcd.setCursor(0,0);
      lcd.print((float)currentF1);

      currentF1=analogCurrent(currentPin1);
      if(currentF1 < currentMin) currentMin = currentF1;
      lcd.setCursor(6,0); 
      lcd.print(currentMin);
      if(currentF1 > currentMax) currentMax = currentF1;
      lcd.setCursor(11,0); 
      lcd.print(currentMax);
      
      lcd.setCursor(0,1);
      lcd.print((float)currentF2);

      currentF2=analogCurrent(currentPin2);
      if(currentF2 < currentMin2) currentMin2 = currentF2;
      lcd.setCursor(6,1); 
      lcd.print(currentMin2);
      if(currentF1 > currentMax2) currentMax2 = currentF2;
      lcd.setCursor(11,1); 
      lcd.print(currentMax2);
      delay(200);
    }
 }

double analogCurrent(int pin)
 {
    double current;
    for(int i = 0; i < 100; i++)
     {
        current = current + (0.004887 * analogRead(pin));
     }
    current = current / 10;
    return current;
 }

This is a test setup to read the current consumption from two DC fans. The code does a sweep from 0% to 100% and back to 0% PWM and displays current values (now/min/max)
The question here is why changing the code gives different results while hardware remains exactly the same?
Try it on your Arduino and see if you get the same results. With the above code my instant current value is displayed on the LCD as 0.22 (with input at 0V).
After changing the last lines to look like this:

        current = current + (analogRead(pin));
     }
    current = current / 10;
    return current * 0.004883;

The LCD shows 0.00

With about 5V supplied to the analog input pin the results are:

1. 55.24 - first code
2. 49.97 - after changing the code
   Yes, I am aware that I'm taking 100 readings and dividing it by 10 later on. This is on purpose.
   What gives guys? Shouldn't the output value be the same if I'm not changing anything else? It's just that one part of the code.
   Nick - I'm sure it's me... but why?

[system]

double analogCurrent(int pin)
 {
    double current;
    for(int i = 0; i < 100; i++)
     {
        current = current + (0.004887 * analogRead(pin));

Local variables are NOT initialized unless YOU do it. Adding to garbage does not make the garbage go away.

[Jamesik]

Paul, I'm afraid I don't understand what you're trying to tell me.
I'm not an experienced Arduino user and most of the code comes from a friend of mine who himself is not very advanced. I've been working with hardware for years and just recently started experimenting with this. So, please explain what you mean. I don't understand why I get wrong readings with the first code but everything is perfect when the code is changed as mentioned previously.
Thank you for understanding.

[econjack]

What Paul is trying to tell you is: Never assume an uninitialized variable is equal to 0. In your analogCurrent() function, you define current as a double, but never initialize it to anything. This means its content (i.e., its rvalue) is whatever random bit pattern may have existed at that memory location when current comes into scope. You then have the statement:

     current = current + (0.004887 * analogRead(pin));

which is going to take the garbage presently stored in current and add more stuff to it. There is a well-known equation that states: garbage + garbage = more garbage. You should never assume an initial value for a variable when used in the type of statement shown above. You can at least start off right by using:

double analogCurrent(int pin)
 {
    double current = 0.0;

However, since the IDE doesn't support the double data type, it would be more honest to define current as a float.

[system]

Local variables, like "current" are not zeroed, unless you do it explicitly.

I will post just a part of the code because everything else is pretty much generic.

Not posting code from the off has wasted an awful lot of time.

[Jamesik]

OK, thank you for your explanation.
Still it doesn't explain why my little change in the code results in correct readings. If my initial code adds to the existing garbage then the change does exactly the same. Yet, I get correct readings every time and all the time. Besides, the result is strangely stable and repeatable.

[jboyton]

Jamesik:
Still it doesn't explain why my little change in the code results in correct readings. If my initial code adds to the existing garbage then the change does exactly the same. Yet, I get correct readings every time and all the time. Besides, the result is strangely stable and repeatable.

The variable isn't initialized so its value is subject to whatever came before it. You probably just lucked out with your apparently innocent change and ended up with the variable being set to zero. It's repeatable since your processor does the same thing every time. But if you change your code again it's difficult to predict what will happen.

[system]

But if you change your code again it's difficult to predict what will happen.

But it's trivially easy to see what current equals before you add anything to it.

It's equally trivially easy to always type:

type name = value;

That way you KNOW what you are starting with.

I worked on a system once that initialized all ints, not explicitly initialized, to 3087. I have no clue what horse's rear end came up with that value, but it was easy to recognize when printing the value of a variable for debugging purposes.