Pages: [1]   Go Down
Author Topic: Can't explain program (mis)behavior  (Read 575 times)
0 Members and 1 Guest are viewing this topic.
Grand Blanc, MI, USA
Offline Offline
Faraday Member
**
Karma: 93
Posts: 3967
CODE is a mass noun and should not be used in the plural or with an indefinite article.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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.

Code:
void setup() {
    pinMode(A0, INPUT);
    Serial.begin(57600);
}

void loop(void) {
    int p;

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

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
Logged

MCP79411/12 RTC ... "One Million Ohms" ATtiny kit ... available at http://www.tindie.com/stores/JChristensen/

Offline Offline
Newbie
*
Karma: 0
Posts: 4
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Hi,

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.

MidiMan
Logged

SF Bay Area (USA)
Offline Offline
Tesla Member
***
Karma: 124
Posts: 6652
Strongly opinionated, but not official!
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:
Code:
#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):
Code:
if (readPhotocell() < 100) { return 100; }
  else if (readPhotocell() > 900) { return 900; }
    else {return readPhotocell(); }

Logged

Copenhagen / Denmark
Offline Offline
Edison Member
*
Karma: 6
Posts: 2360
Do it !
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Try to change this line:

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

to

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 .-)
« Last Edit: February 26, 2011, 11:22:16 am by MikMo » Logged

Offline Offline
God Member
*****
Karma: 2
Posts: 711
a, b = b, a+b
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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

p.
« Last Edit: February 26, 2011, 11:27:27 am by fkeel » Logged


Grand Blanc, MI, USA
Offline Offline
Faraday Member
**
Karma: 93
Posts: 3967
CODE is a mass noun and should not be used in the plural or with an indefinite article.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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!   smiley-grin   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.
Logged

MCP79411/12 RTC ... "One Million Ohms" ATtiny kit ... available at http://www.tindie.com/stores/JChristensen/

Pages: [1]   Go Up
Jump to: