Hi, I'm working on an Arduino UNO board and am using C programming language. I seem to have a problem correctly returning an array from function. I did some reading on this and understand the fact that a function cannot return the array itself, but instead a pointer.

This seems to work for the first ~25 values of the array and then I get inconsistent results. I print off the values in the function as they are gathered and they are correct (I'm reading digital 0/1's). However, once I return the values to my main function every value after ~25 is a very large number (several 1000's) when I clearly saw only 0/1's in the function. Here's my code that's giving issues. Any thoughts?

#include <LiquidCrystal.h>
LiquidCrystal lcd(2, 3, 4, 5, 6, 7);
int * sensorReadbackAll;

void setup() {
//lcd init and digital pin states
}

void loop() {
  //do stuff
  sensorReadbackAll = sensorReadAll();

  for(i=0;i<64;i++){
    lcd.setCursor(10,0);
    lcd.print(sensorReadbackAll[i]);
    lcd.setCursor(7,0);
    lcd.print(i);
    delay(100);
  }

//do more stuff
}

int * sensorReadAll(){
  int i = 0;
  int sensorDataAll[64];
  while(i < 64){
   //stuff
    sensorDataAll[i] = digitalRead(tempSensorPin);

    lcd.setCursor(6,0);
    lcd.print(sensorDataAll[i]);
    delay(100);
    i++;
  }
  return(sensorDataAll);
}

 

Yes. When you use pointers you have no idea inside the function what size the array is. It is incredibly easy to run past the end of an array. Similarly, if the function returns a pointer to the array, you have no idea in the calling program where the array ends or how many elements it has. If you are going to use this method you need to implement a more disciplined approach. Since your array is defined inside the function it probably went on the stack. After the function returns, the data on the stack is essentially gone or replaced by other stuff. It's a very very very bad practice.

 

This looks like you will have 64 cell array called up 64 times.

I would make the data reading function. Then call that function 64 times from the "main" (loop{}) and store the individual results in the array in the "main" (loop{}).

 

As Papa says, that is very bad programming.

Make sensorDataAll[64] global then you don't have to pass pointers.

 

Thanks all for the quick replies/information. I changed to a global variable for now and will look into a more appropriate approach for passing arrays as needed.

Solved.

 

You can make the function variable sensorDataAll on the heap instead of stack with a static int declaration and pass the pointer of that. This would encapsulate the variable name to the functions scope but you should declare a global type array size constant unless it's a zero terminated string. It's generally safe but not reentrant (and only a slight abuse of C) and you can modify the elements outside of the function if it's not defined as a const inside the function.

 

I think I'm agreeing with NSASpook, but the way I'd put it would be to point out that an ordinary array inside a function isn't guaranteed to hold any consistent values once you leave the function. If you need to hold all the data until it gets processed, you have to make it static. But that's wasteful of memory, because it's dedicated to that function and will never be released. An alternative way to do it would be to declare the array in main() and pass a pointer to it as an argument to the function, so the array is the property of main(). Or make it global, as MrChips said, and pass no pointers at all. Does the compiler you're using allow malloc()? That would let you use a block of RAM and then release it again.

Also, you're begging for trouble by using the number 64 in both main() and the function. Use a #define command to create a constant like SENSOR_READINGS with value 64, and then if you decide to have some other number, you can change it in one place and never worry about keeping it the same everywhere.

 

Thanks all for the quick replies/information. I changed to a global variable for now and will look into a more appropriate approach for passing arrays as needed.

If you want to write a universal function that can fill up or manipulate any array, pass the address of the array as an input parameter to the function. You should also pass the size of the array.

void ReadData(int *array_adr, int array_size);

 

For his simple function on a uC a global is usually the best option if writes are restricted to only that function in a single thread. Where returning static const (in ROM) array variables in functions are really useful are to return text strings from functions as results or errors strings for display.

 

The array need not be global, it depends on where it is used.

Here the pointer sensorReadbackAll is declared as a global but may only be used inside the setup() routine. To keep the memory uncluttered sensorReadbackAll could (would would say should) be declared inside setup(). Likewise the sensorDataAll array could be defined in the same routine for the same scope.

Then to actually use that array pass a reference to it into sensorReadAll() instead of creating a local place (very bad) or just using a global (wasteful but works).

 

You guys beat me to it! I just got out of the shower and was going to comment along the lines that you guys have.

To me, the fact that the array is instantiated within the function means that it's use is local in scope. That is, once the return is executed, its memory becomes available for other use. Hence, the pointer that is returned may or may not point to valid data.

 

As others have said/hinted, the function sensorReadAll() defines a local array, sensorDataAll[], that is stored on the stack. You return a pointer to the beginning of this array when the function terminates. The moment you return and start doing ANYTHING that involves stack operations (which is almost EVERYTHING), you start overwriting the contents of this array. The four pretty obvious ways to deal with it, depending on other considerations, are to work with a global array (not a fan of global variables, but they have their place, particularly in embedded code), declare the array in main() and pass a pointer to it, declare the array as static in the sensorDataAll[] function (not too thrilled with this one), have sensorDataAll[] call malloc to dynamically allocated the array and return a pointer to that.

 

Here's an example of Arduino code that implements the second alternative that WBahn listed. It declares the array in "loop" (Arduino's version of "main"), and when the the function is called, it is called with the pointer to the defined array.

#define arraySize 64

void setup() {
  // put your setup code here, to run once:
  Serial.begin(9600);

}
// function defined with array reference as input
void SensorReadALL(int mySensorDataAll[]) {

// no array definition here; array pointer is passed to function instead
// note sketch assumes array size is constant

for(int i=0; i<=arraySize; i++) {
   // fill array with test values;
   mySensorDataAll[i]=i;
 }
return; // done here
}

void loop() {
  // define array SensorDataAll local to the main loop
  int SensorDataAll[arraySize];

  // Pass array by reference to function; array name without
  // brackets passes to the function, a pointer to the array
  SensorReadALL(SensorDataAll);

for(int i=32; i<=42; i++) {
   // print out a few values in the middle to confirm successful loading
   Serial.println(SensorDataAll[i]);
}
while(true); // stop here
}