I think that the following code should sum 10 positive numbers into the variable total. But about half of the time the value of total printed out is negative. The value read by analogRead is near 512;
#define NUMREADINGS 10
long total = 0; // the running total
int inputPin = 0;
void setup()
{
Serial.begin(9600); // initialize serial communication with computer
}
void loop()
{
total = 0;
for (int i = 0; i < NUMREADINGS; i++) total += abs(512-analogRead(inputPin));
Serial.println(total); // send it to the computer (as ASCII digits)
delay(100);
}
To see what's going on, why not try the following code?
#define NUMREADINGS 10
long total = 0; // the running total
int inputPin = 0;
void setup()
{
Serial.begin(9600); // initialize serial communication with computer
}
void loop()
{
total = 0;
for (int i = 0; i < NUMREADINGS; i++) {
total += abs(512-analogRead(inputPin));
Serial.println(total); // send it to the computer (as ASCII digits)
delay(100);
}
}
This is what I don't understand, abs(512-analogRead(inputPin)) should always be positive. Therefore total should be monotonically increasing. Using the suggested code above, I see that total sometimes is increasing and sometimes decreasing. The behavior is if the code were total += 512 - analogRead(inputPin)
This works. It looks like abs cannot have a function call as part of its argument. I don't understand why.
#define NUMREADINGS 10
long total = 0; // the running total
int temp;
int inputPin = 0;
void setup()
{
Serial.begin(9600); // initialize serial communication with computer
}
void loop()
{
total = 0;
for (int i = 0; i < NUMREADINGS; i++) {
temp= 512-analogRead(inputPin);
total += abs(temp);
}
Serial.println(total);
delay(100);
}
The abs(x) is implemented as a macro, not a function, and thus the argument (or formula) you give may be evaluated more than once. This is pretty typical:
#define abs(x) ( ((x) < 0)? -(x) : (x) )
Since it's a macro, and since the argument appears multiple times, any function calls in your argument will be called twice.
The second time you evaluate (512-analogRead(pin)), it might not be the same sign as the first evaluation. This would return a negative number from abs().
The fix is just as you've posted:
int biased = 512-analogRead(pin); total += abs(biased);
John, the problem is that abs() is a macro and not a function. If you look in wiring.h, you'll see that it is defined as
#define abs(x) ((x)>0?(x):-(x))
This means that the expression passed to it is evaluated twice, which is bad if the expression contains a call to a function which may change rapidly like analogRead can.
Consider the case where your analogRead call returned 511 the first time and then 513 the next. The value returned by abs would in that case be
@mellis, this is just a part of life with C and C++, there's no "right way."
The C++ language could allow you to define overloads for inlined functions, such as double abs(double x), but that's usually regarded as overkill. And like the int() syntax, that works for C++ but not for C, so mixing tab types will fail. The C language was not designed for mistake-proofing, and coders just need to learn the gotchas.
@mikalhart, you were more specific with the actual definition from wiring.h, and had a nice numerical example. And you also don't use yellow smileys. You rock.
this is just a part of life with C and C++, there's no "right way."
It seems to me that C++ provides a very nice solution for this:
template<class T> inline T abs(T val) { return val < 0 ? -val : val; }
This line causes inline code to be generated for any type T, but only as needed. It only evaluates the argument once and works for any type that can be compared to 0 using the < operator. If you feel uncomfortable with templates for some reason, or wish to limit the use to, say, integral types, you could also just write
inline long abs(long val) { return val < 0 ? -val : val; }
inline double abs(double val) { return val < 0.0 ? -val : val; }
which is just as effective as the macro, but again only evaluates the argument once.
However, this solution will not work well, as Halley notes, for users with tabs that are written in C.
At some point in Arduino's history, the decision was made to #undef the stdlib implementation of abs and replace it with a new one in wiring.h. I'm curious why.
PS: Halley, lest you give me too much credit for avoiding yellow smilies, I confess that I had thought about using them, but it messed up the abs definition and made it look like this:
It seems to me that the real best solution is to simply remove abs entirely from wiring.h. "abs", "labs", and "fabs" are already defined by the system. These built-ins don't require any special #includes and only evaluate their arguments once.
Each time you see the same argument evaluated more than once in a macro, the solution is the same: evaluate into a variable, and use the variable in the macro.
They are defined as macros, so that they work with ints, longs, floats, etc., without modification. While C++ can handle overloaded functions that can do multiple types of arguments, C cannot.
I think the min and max functions have similar problems.
min(x++, 20);
ends up counting by two's because it gets evaluated twice - but now I know why.
Aren't these functions included in AVR math libc library? Is there some big disadvantage (in memory) to using that library?
The upside is that you get the rest of the library. At one point I thought the library was already included, so wouldn't these functions then be "free"?
Aren't these functions included in AVR math libc library? Is there some big disadvantage (in memory) to using that library?
As mentioned above, if you have a function, then the return type and the argument types must be known and stable. You have to have a byte bmax(byte, byte), a int imax(int, int) and a double fmax(double,double), and you have to remember which one to call. A macro does not require this complexity, it just does the right thing, has the same name in all situations, and takes no additional library space to do it. (The C++ language has another answer to this problem, but it is not compatible with C, and I think it's better to keep Arduino stuff compatible with both where possible.)
Thanks, Halley.