int array passed through function has wrong values only after multiple passes

Question

I have this code:

for(int i = 0; i < its; i++) {
    x1 = random(0,4);
    y1 = random(0,4);
    multiLamp(new int[2] {3,1}, new int[2] {x1,x2}, new int[2] {y1,y2}, wait, 2);
    multiLamp(new int[2] {2,0}, new int[2] {x1,x2}, new int[2] {y1,y2}, wait, 2);
    x2 = random(0,4);
    y2 = random(0,4);
    multiLamp(new int[2] {1,3}, new int[2] {x1,x2}, new int[2] {y1,y2}, wait, 2);
    multiLamp(new int[2] {0,2}, new int[2] {x1,x2}, new int[2] {y1,y2}, wait, 2);
}

which calls the function "multiLamp" and passes 3 int arrays with 2 values each. Here is the function:

void multiLamp(int epos[], int xpos[], int ypos[], int del, int len) {
  for(int it = 0; it < del; it++) {
    for(int i = 0; i < len; i++) {
      lamp(epos[i],xpos[i],ypos[i],2);
    }
  }
}

This function is supposed to call some function "lamp" with a few integers from the arrays. In the beginning this works fine but after more than 50 calls of the "multiLamp" function the values in the arrays become {0,128,128} and {3,3,3} or something similar even though the values passed into it from my main code are equally valid as before (they are somehow changed after being passed to the "multiLamp" function).

I can't seem to find the issue here. Any help is appreciated!

UPDATE:

Here is the full code, although I don't know if it will be of any use because I wrote it to control some LEDs and if you don't have the setup it's difficult to see if the code is working or not.

int e[] = {12,13,11,10};
int xy[] = {2,3,4,9,6,7,8,5,1,A0,A1,0,A3,A4,A5,A2};

void setup() {
  for(int a=0;a < 4;a++){
         pinMode(e[a], OUTPUT);
         digitalWrite(e[a], LOW);
  }
  for(int b=0;b < 16;b++){
         pinMode(xy[b], OUTPUT);
         digitalWrite(xy[b], HIGH);
  }
}

void loop() {
  bigCubes(100);
  rain(20, 30);
  miniCubes(15);
  every(80);
}

void lamp(int epos, int xpos, int ypos, int del) {
  int xypos = 4*xpos+ypos;
  digitalWrite(e[epos], HIGH);
  digitalWrite(xy[xypos], LOW);
  delay(del);
  digitalWrite(e[epos], LOW);
  digitalWrite(xy[xypos], HIGH);
}

void multiLamp(int epos[], int xpos[], int ypos[], int del, int len) {
  for(int it = 0; it < del; it++) {
    for(int i = 0; i < len; i++) {
      lamp(epos[i],xpos[i],ypos[i],1);
    }
  }
}

void every(int wait) {
  for(int i = 0; i < 4; i++) {
    for(int j = 0; j < 4; j++) {
      for(int k = 0; k < 4; k++) {
        lamp(i,j,k,wait);
      }
    }
  }
}

void miniCubes(int wait) {
  int cube[8][3] = {{0,0,0},{0,0,1},{0,1,1},{0,1,0},{1,1,0},{1,1,1},{1,0,1},{1,0,0}};
  for(int c = 0; c < 8; c++) {
    for(int it = 0; it < wait; it++) {
      for(int i = 0; i < 8; i++) {
        lamp(cube[i][0]+2*cube[c][0],cube[i][1]+2*cube[c][1],cube[i][2]+2*cube[c][2],2);
      }
    }
  }
}

void bigCubes(int wait) {
  multiLamp(new int[32] {0,0,0,0,0,0,0,0,0,0,0,0,1,1,1,1,2,2,2,2,3,3,3,3,3,3,3,3,3,3,3,3}, new int[32] {0,0,0,0,1,1,2,2,3,3,3,3,0,0,3,3,0,0,3,3,0,0,0,0,1,1,2,2,3,3,3,3}, new int[32] {0,1,2,3,0,3,0,3,0,1,2,3,0,3,0,3,0,3,0,3,0,1,2,3,0,3,0,3,0,1,2,3}, wait, 32);
}

void rain(int its, int wait) {
  int x2 = random(0,4);
  int y2 = random(0,4);
  int x1;
  int y1;
  for(int i = 0; i < its; i++) {
    x1 = random(0,4);
    y1 = random(0,4);
    multiLamp(new int[2] {3,1}, new int[2] {x1,x2}, new int[2] {y1,y2}, wait, 2);
    multiLamp(new int[2] {2,0}, new int[2] {x1,x2}, new int[2] {y1,y2}, wait, 2);
    x2 = random(0,4);
    y2 = random(0,4);
    multiLamp(new int[2] {1,3}, new int[2] {x1,x2}, new int[2] {y1,y2}, wait, 2);
    multiLamp(new int[2] {0,2}, new int[2] {x1,x2}, new int[2] {y1,y2}, wait, 2);
  }
}

Answer 1

You are wrongly using the new operator for your arrays. That will dynamically allocate these arrays. They are put on the heap (not the stack like local variables) and thus don't get freed automatically, when the function exits. That has to be done by your code with the delete keyword. Your code just eats up all the available memory. Thats called a memory leak.

That you need to free your dynamically created variables is not an Arduino thing; this is correct for all C/C++ code. But the limited memory on AVR based Arduinos like the Uno makes it worse.

If you do the math, one iteration of void loop() will block 432 bytes of memory (216 array elements, each of type int thus 2 byte). The 2KB of memory (which is also needed for local and global variables) is eaten up rather fast.

I don't see, why you even would use dynamically created arrays. Why not just using locally defined arrays, like in the miniCubes() function?

---

That said, it is also a generally bad idea to use dynamic memory allocation microcontrollers, especially those with small memory. When creating and deleting objects of different sizes (which is the main advantage of dynamic allocation), you will leave small holes in your RAM, which are too small for one of your objects to fit. At some point your RAM is completely filled with these holes, so that no more memory can be allocated for your objects to save. Thats called memory fragmentation.

On your PC that isn't a problem because of 2 reasons:

• The PC has loads of RAM available, and
• the OS manages the RAM, defragmenting it, if necessary.

Since microcontrollers don't have an OS managing the RAM and often very limited RAM, you need to make sure, that your RAM doesn't get fragmented too much in the runtime of your code.