Problem referencing arrays in a function

Projects Programming
1

Hi All,

I know this topic is a common pitfall for folks, as I have read many posts on the subject, but remain puzzled with my program's behavior.

I am trying to maintain a moving average of analog readings from two sensors. Six readings are stored in an array for each sensor. At a specified interval, I want to call my "update_local_read" function, which takes an array reference and an analog pin number as arguments. The function then updates the average by shifting values one position up in the referenced array, taking a new analog reading for the [0] position, and then calling another function to calculate the average of the array.

The function(s) works as I expect when I reference my temperature readings array ("indoor_Temps"), but when I add a subsequent (or preceding) call to the same function with a reference to the "indoor_Humids" array, the indoor_Humids array does not behave as I would expect it to (see output below). However, if I comment out the function call for the "indoor_Temps" array and only call update_local_read (HUMID_PIN, indoor_Humids), then the indoor_Humids array behaves as I would expect.

I was able to make a modified version of this code compile and run in C99 (using keyboard input instead of sensor input), and both function calls worked in that case. However with the Arduino, it feels like somehow, the function is not "accepting" the reference to the indoor_Humids array, but rather it ends up incorporating random numbers from the indoor_Temps array.

Thanks very much in advance for any thoughts or advice!

Mike

const byte TEMP_PIN = 0;
const byte HUMID_PIN = 1;

const byte NUM_SAMPLES = 6;
float indoor_avg_H = 0;
float indoor_avg_T = 0;
int indoor_Temps[NUM_SAMPLES];
int indoor_Humids[NUM_SAMPLES];
unsigned long local_read_time = 0;

// Array handling function prototypes
float update_local_read (byte read_pin, int indoor_reads[]);
float average(int indoor_array[]);

void setup()
{
        //initialize arrays
        for (int i = 0; i < NUM_SAMPLES; i++)
        {
                indoor_Temps[i] = 0;
                indoor_Humids[i] = 0;
        }
  
        Serial.begin(9600);
        Serial.print("===============================================");
        Serial.println();
}        

void loop()
{

if (millis() >= local_read_time)
        {
                Serial.print("Humidity: ");
                indoor_avg_H = update_local_read(HUMID_PIN, indoor_Humids);      
                Serial.print(" ");
          
                Serial.print("Temp: ");
                indoor_avg_T = update_local_read(TEMP_PIN, indoor_Temps);
                Serial.println();
                
        local_read_time = millis() + 1000;
        }
}        

// update indoor analog readings
float update_local_read (byte read_pin, int indoor_reads[])
{
        // shift array one pos right
        for (int i = NUM_SAMPLES - 1; i >= 0; i--)
                        indoor_reads[i + 1] = indoor_reads[i];
        
        //newest reading into [0]        
        indoor_reads[0] = analogRead(read_pin);
        indoor_reads[0] = analogRead(read_pin); // add delay?
        
        for (int i = 0; i < NUM_SAMPLES; i++)
        {
                Serial.print(indoor_reads[i]); 
                Serial.print(", ");
        }
                
        float indoor_avg = average(indoor_reads);
        Serial.print("avg = "); Serial.print(indoor_avg, 2);
       
        return indoor_avg; 
}

// average the array for local T or H
float average(int indoor_array[])
{
        float avg = 0;
        int sum = 0;
        
        for (byte i = 0; i < NUM_SAMPLES; i++)
                sum += indoor_array[i];
        
        avg = (float) sum / NUM_SAMPLES;
        
        return avg; 
}

Output. indoor_Temps array fills and behaves as expected, but not the indoor_Humids.

Humidity: 402, 0, 0, 0, 0, 0, avg = 67.00 Temp: 503, 0, 0, 0, 0, 0, avg = 83.83
Humidity: 401, 0, 0, 0, 0, 0, avg = 66.83 Temp: 503, 503, 0, 0, 0, 0, avg = 167.67
Humidity: 401, 0, 0, 0, 0, 0, avg = 66.83 Temp: 503, 503, 503, 0, 0, 0, avg = 251.50
Humidity: 401, 0, 0, 0, 0, 0, avg = 66.83 Temp: 503, 503, 503, 503, 0, 0, avg = 335.33
Humidity: 401, 0, 0, 0, 0, 0, avg = 66.83 Temp: 503, 503, 503, 503, 503, 0, avg = 419.17
Humidity: 402, 0, 0, 0, 0, 0, avg = 67.00 Temp: 503, 503, 503, 503, 503, 503, avg = 503.00
Humidity: 405, 0, 0, 0, 0, 0, avg = 67.50 Temp: 506, 503, 503, 503, 503, 503, avg = 503.50
Humidity: 403, 503, 0, 0, 0, 0, avg = 151.00 Temp: 503, 506, 503, 503, 503, 503, avg = 503.50
Humidity: 402, 503, 503, 0, 0, 0, avg = 234.67 Temp: 503, 503, 506, 503, 503, 503, avg = 503.50
Humidity: 402, 503, 503, 503, 0, 0, avg = 318.50 Temp: 503, 503, 503, 506, 503, 503, avg = 503.50
Humidity: 402, 503, 503, 503, 503, 0, avg = 402.33 Temp: 503, 503, 503, 503, 506, 503, avg = 503.50
Humidity: 402, 503, 503, 503, 503, 503, avg = 486.17 Temp: 503, 503, 503, 503, 503, 506, avg = 503.50
Humidity: 403, 503, 503, 503, 503, 503, avg = 486.33 Temp: 503, 503, 503, 503, 503, 503, avg = 503.00
Humidity: 404, 506, 503, 503, 503, 503, avg = 487.00 Temp: 503, 503, 503, 503, 503, 503, avg = 503.00
Humidity: 404, 503, 506, 503, 503, 503, avg = 487.00 Temp: 503, 503, 503, 503, 503, 503, avg = 503.00

Here is the program output when only update_local_read(HUMID_PIN, indoor_Humids) is called...it behaves as I want/expect it to...

Humidity: 394, 0, 0, 0, 0, 0, avg = 65.67
Humidity: 394, 394, 0, 0, 0, 0, avg = 131.33
Humidity: 393, 394, 394, 0, 0, 0, avg = 196.83
Humidity: 395, 393, 394, 394, 0, 0, avg = 262.67
Humidity: 396, 395, 393, 394, 394, 0, avg = 328.67
Humidity: 396, 396, 395, 393, 394, 394, avg = 394.67
Humidity: 396, 396, 396, 395, 393, 394, avg = 395.00
Humidity: 396, 396, 396, 396, 395, 393, avg = 395.33
Humidity: 396, 396, 396, 396, 396, 395, avg = 395.83
Humidity: 412, 396, 396, 396, 396, 396, avg = 398.67
Humidity: 457, 412, 396, 396, 396, 396, avg = 408.83
Humidity: 485, 457, 412, 396, 396, 396, avg = 423.67

2

The first question that comes to mind is why do you need to pass a global array to a function?

3

Hi PaulS,

So is there a way I can make these arrays not global? I realize it is generally desirable to keep declarations as limited in scope as possible...but how else can I keep the array from being re-initialized after each loop?

I guess to answer your question though, I was passing the arrays to the function so that I could use the same code with two different arrays without having to re-write it all...as a way or sorts to identify which array it should operate on. I do get your point that passing a global variable to a function is redundant, as the function does not need a global variable to be passed to it (like "NUM_SAMPLES" isn't passed).

I'm more than open to ideas on a better/more efficient/more elegant way to accomplish this...that is my ultimate goal of course!

Mike

4

Actually the following is passing a "pointer" to the array, no?

float average(int indoor_array[])

And isn't that the "correct" way?

What I would do, however, is also pass the array size, so your routine needn't know the
declared value of NUM_SAMPLES. Then you have a truly usable function.

Also, be careful because you've got floats and ints everywheres, and it's easy to mix them up.
Also, sum should be a long, because "int sum;" will likely overflow.

// average the array for local T or H
float average(int indoor_array[], int siz)
{
        long sum = 0L;
        int i;

       // good for siz up to 32767, better to a check on max siz value.
        for ( i = 0; i < siz; i++)     
            sum += indoor_array[i];
        
        return( (float) sum / siz );
}
5 
//initialize arrays
        for (int i = 0; i < NUM_SAMPLES; i++)
        {
                indoor_Temps[i] = 0;
                indoor_Humids[i] = 0;
        }

This is pointless - the compiler has already initialised the arrays to zero for you.

If you want to do a rolling average, subtract the oldest reading and add the newest from/to your running totoal.; there's no need for the time-wasting shifting stuff.

6

Thanks for your thoughts oric_dan and AWOL.

Point taken that there is no need to initialize the arrays...I originally hadn't done this, but thought I'd give it a try in case it was part of my trouble...obviously it was not.

Also, calculating a moving average as AWOL describes makes sense to me in the abstract, and I can see how it would be an improvement in terms of efficiency. But it still seems to me that I'd need an array to hold the values I'm going to average...and if I don't shift them, how do I easily keep track of which one is the oldest and which is the most recent? Perhaps you could sequentially fill the array, wrapping at the end...then at a given moment the new reading could be [ i] and the oldest typically [i + 1] (except for the last position)...?

Finally, back to what PaulS said before...I realize it's preferable to keep the scope of variables limited...but is there a way to avoid global variables when you have a variable that you don't want to get re-initialized with every loop of the main program (as is my case with these arrays)?

Thanks,
Mike

7

I think there's a problem in the following, as the first index writes off the end of the array,
ie, into cell indoor_reads[NUM_SAMPLES]. C doesn't care, you could write into cell
indoor-reads[1001] for all C cares.

        // shift array one pos right
        for (int i = NUM_SAMPLES - 1; i >= 0; i--)
                        indoor_reads[i + 1] = indoor_reads[i];

The way to avoid shifting is to use a circular buffer [look it up], where you basically
just increment the index into the array, and then wrap back to zero when it hits the
high-end.

back to what PaulS said before

Personally, I think that's more a matter of personal style than correctness.

8

Mike_MT:
Finally, back to what PaulS said before...I realize it's preferable to keep the scope of variables limited...but is there a way to avoid global variables when you have a variable that you don't want to get re-initialized with every loop of the main program (as is my case with these arrays)?

You can make them static, but the scope and the initialization of the variable aren't the same thing.

9

Mike_MT:
how do I easily keep track of which one is the oldest and which is the most recent?

You use a circular buffer - essentially, an array with a read pointer and a write pointer. If you google a circular buffer or circular queue, you'll find loads of examples. Just remember that the buffer capacity is one less than the array length when you come to size the array.

10

Okay, thanks a lot oric_dan, Nick and PeterH. A circular buffer it is--I will look it up and figure out how to implement--thanks a lot for your thoughts and for pointing me in the right direction on this.

It's interesting to hear peoples' thoughts on variable scope. It seems that a lot of the arduino sketches I've seen tend to use a lot of global variables, perhaps for some of the reasons I mentioned previously. It seems like in most cases, as Nick pointed out, the same could be accomplished (that is, not "throwing-out" an (automatic) variable's value with each loop iteration) by declaring static variables inside the loop() function...but I haven't come across this in many (any?) Arduino sketches that I have looked over.

Anyway, as I'm about halfway through reading my first text on C, and this discussion has been helpful for grounding some of these concepts in the practical.

Thanks all--
Mike

11

It's interesting to hear peoples' thoughts on variable scope. It seems that a lot of the arduino sketches I've seen tend to use a lot of global variables, perhaps for some of the reasons I mentioned previously. It seems like in most cases, as Nick pointed out, the same could be accomplished (that is, not "throwing-out" an (automatic) variable's value with each loop iteration) by declaring static variables inside the loop() function...but I haven't come across this in many (any?) Arduino sketches that I have looked over.

Many of the sketches you'll see on-line are written by people new to software development. There's likely a preference for just getting it to work too. Globals are easier to use and the pitfalls associated with their use can to some extent be ignored because Arduino sketches for the most part tend to be small. You can get away without a lot of software engineering rigour because of this. Personally, having a programming background, I prefer to apply as much of that rigour as possible, but it's easy to understand why others take a different view.

12

It's interesting to hear peoples' thoughts on variable scope. It seems that a lot of the arduino sketches I've seen tend to use a lot of global variables, perhaps for some of the reasons I mentioned previously. It seems like in most cases, as Nick pointed out, the same could be accomplished (that is, not "throwing-out" an (automatic) variable's value with each loop iteration) by declaring static variables inside the loop() function...but I haven't come across this in many (any?) Arduino sketches that I have looked over.

Whether you use globals or not, it's always a good idea to write generic functions, like
average(), so they don't use globals. Then, they can be called from anywheres.

I tend to use a lot of static variables "inside" of functions, so they have very limited scope, but
also persistence. You will note that, this is the basic concept behind object-oriented programming,
but implementable in regular C.

I also use global variables and #defines inside of .c source modules that are not accessible from
other source modules. Again, data encapsulation.

You get into bad trouble when you define globals that are extern and accessible everywheres in
your program, and then you embed them into your various functions. This is known as tight
data-coupling, and is guaranteed to bite you in the backside one week from now.

13

Thanks for the further explanation oric_dan and wildbill. I certainly fall into the new to software development category, but would like to develop as broadly applicable a skill set as I can. So to the extent possible, I am interested in that more widely applicable approach, with the hope that it may serve me well outside of the Arduino environment at some point. So this has been very enlightening...and I've got some revising/improvement of the above code to do now.

Mike