Can't explain program (mis)behavior

This was part of a larger project. The idea was to read a photocell at regular intervals and do a six-point moving average to smooth out the readings. For reasons I can’t understand, every call to the readPhotocell function appears to be making three trips through! The code below has been gutted to the minimum I could get it to, and still have it exhibit the problem. I’ve found several things that make the problem go away as indicated in the comments but can’t make any sense of it all.

Sure hope someone can help, this is one of those that just make you crazy. It feels like a program that’s stepping all over itself on a von Neumann machine due to a subscript out of bounds or something. It’s simple enough that I don’t see where that’s happening. Not sure how or if such a situation would manifest itself on the Harvard architecture, anyway.

I’m running this on a bare Arduino Uno SMD edition, with no external connections to anything.

void setup() {
    pinMode(A0, INPUT);

void loop(void) {
    int p;

    p = constrain(readPhotocell(), 100, 900);    //changing to p = readPhotocell(); makes problem go away

int readPhotocell() {
    static int readings[6];
    int reading, avgReading = 0;
    reading = analogRead(A0);    //changing to reading = constant; makes problem go away
    Serial.print("readPhotocell ");
    Serial.print(reading, DEC);
    Serial.print(" ms=");
    Serial.println(millis(), DEC);

    readings[5] = reading;            //commenting this out makes problem go away
    for (int i=0; i<6; i++) {
        avgReading += readings[i];    //commenting this out (or the whole loop) makes problem go away
    return avgReading;    //changing to return a constant makes problem go away

//typical output:
//readPhotocell 792 ms=3
//readPhotocell 791 ms=7
//readPhotocell 788 ms=11
//readPhotocell 670 ms=1014
//readPhotocell 672 ms=1019
//readPhotocell 672 ms=1024
//readPhotocell 579 ms=2028
//readPhotocell 578 ms=2033
//readPhotocell 579 ms=2037


have studied your code and I don’t have a clear idea why it behaves that way.
But I wonder if this line

readings[5] = reading; //commenting this out makes problem go away = reading;

is maybe a part of the problem because I can’t find a line in the code where readings[0-4] are filled with values. I would expect some kind of for…next loop in which readings[0-4] are set with integers from analogRead.

Finally you are trying to use readings[0-4]

for (int i=0; i<6; i++) {
avgReading += readings*; //commenting this out (or the whole loop) makes problem go away*

  • }*
    but I would expect them to be empty or not initialised. In my codes this has caused all kind of strange results.

Ah hah! This is one of C’s famous “issues”; when is something that looks like a function call NOT a function call?

“constrain” is a macro:

#define constrain(amt,low,high) ((amt)<(low)?(low):((amt)>(high)?(high):(amt)))

You’ll notice that the first argument is used three times in the macro. Since you have a function call as the first argument, that means that the function will be called up to three times (depending on how the conditions work out.)
It translates as (after substitution):

if (readPhotocell() < 100) { return 100; }
  else if (readPhotocell() > 900) { return 900; }
    else {return readPhotocell(); }

Try to change this line:

p = constrain(readPhotocell(), 100, 900); //changing to p = readPhotocell(); makes problem go away


int pcValue;

pcValue=readPhotocell(); p = constrain(pcValue, 100, 900);

Also as the previous poster mentions your code seems to be a little messy around the averaging.

You actually never calculate an average, just a sum: avgReading += readings*;* There should be a division in there somewhere to qualify as an average .-)

It appears you are working with an empty array. You initialize it, but never assign any values to it. (edit: except for number 5)

I didn’t run your code or anything, and this does not explain why removing the “constraints” solves it, so I assume there is something else going on as well… (edit: I am slow this morning… guess someone else solved the constrains problem for you…)

But yeah, maybe fixing your array could help clean things up


westfw, that has got to be it! I did notice at times that it would only go once or twice through the loop. Thanks a million, I can sleep now! :D I will say that the Arduino doc isn't always as complete as it could be. OTOH, had I known it was a macro, that may or may not have made any lights go on! Lesson Learned, Thank You!!!

Thanks for the other replies as well. I know the function won't accomplish its intended purpose as posted. That's because I was trying to eliminate as much code as possible for brevity and clarity. Uninitialized variables, etc., may cause odd return values, but I couldn't see where they would affect program flow.