Performance Issue probably run out of Ram

Hi,

I built a 16x16 Matrix divided into 4 8x8 matrices when controlling it with my code, it somtimes stops working I think it run out of memory:

An example (configuration.h is included in them main programm):

/********************************************************/
/*                                                      */
/*      This file is used to adapt the following:       */
/*        -pins your matrices are connected to          */
/*        -total number of matrices                     */
/*                                                      */
/*      advanced option:                                */
/*        -how you matrices are conneted to the         */
/*         74HC595                                      */
/*                                                      */
/********************************************************/


//Pin connected to latch pin (ST_CP) of 74HC595
  const int latchPin = 6;
//Pin connected to clock pin (SH_CP) of 74HC595
  const int clockPin = 7;
//Pin connected to Data in (DS) of 74HC595
  const int dataPin = 5;

//Total number of matrices 
  const int totalNumberOfMatrices = 4;
  
//By changing byte - values you can adapt the way how your matrice is connected
//Bytes might need to be added or subtracted when more or less matrices are used
  const int bytesX[totalNumberOfMatrices][8] =  {  {127, 191, 223, 239, 247, 251, 253, 254}, 
                                                   {127, 191, 223, 239, 247, 251, 253, 254},
                                                   {254, 253, 251, 247, 239, 223, 191, 127},
                                                   {254, 253, 251, 247, 239, 223, 191, 127},
                                                };
  const int bytesY[totalNumberOfMatrices][8] =  {  {128,  64,  32,  16,   8,   4,   2,   1},
                                                   {  1,   2,   4,   8,  16,  32,  64, 128},
                                                   {128,  64,  32,  16,   8,   4,   2,   1},
                                                   {  1,   2,   4,   8,  16,  32,  64, 128},
                                                };
                          
/* Size of Matrices supported is momentary limited to 8*8 
  Please do not change these values as the programm may not work correctly afterwards  */
  
  const int height = 8;
  const int width  = 8;

Main Programm:

// Start main programm

#include "configuration.h"  // cointains information about how the matrices are built / connected
 
enum coordinates { x, y };

void lightUpLeds(int drawLeds[][64][2], int length);
void createFigure(int which, int bottomLeftCornerX, int bottomLeftCornerY);


void setup() {
  pinMode(latchPin, OUTPUT);
  pinMode(clockPin, OUTPUT);
  pinMode(dataPin, OUTPUT);
}

void loop() {
  int drawLeds[totalNumberOfMatrices][height*width][2] = {0};
  
  // Left Eye
  drawLeds[1][0][x] = 3;
  drawLeds[1][0][y] = 4;
  drawLeds[1][1][x] = 3;
  drawLeds[1][1][y] = 3;
  drawLeds[1][2][x] = 4;
  drawLeds[1][2][y] = 8;
  drawLeds[1][3][x] = 4;
  drawLeds[1][3][y] = 7;
  drawLeds[1][4][x] = 4;
  drawLeds[1][4][y] = 6;
  drawLeds[1][5][x] = 4;
  drawLeds[1][5][y] = 5;
  drawLeds[1][6][x] = 4;
  drawLeds[1][6][y] = 3;
  drawLeds[1][7][x] = 5;
  drawLeds[1][7][y] = 3;
  drawLeds[1][8][x] = 6;
  drawLeds[1][8][y] = 3;
  drawLeds[1][9][x] = 7;
  drawLeds[1][9][y] = 3;
  drawLeds[1][10][x] = 8;
  drawLeds[1][10][y] = 3;
  
  drawLeds[3][0][x] = 4;
  drawLeds[3][0][y] = 4;
    
  drawLeds[3][1][x] = 4;
  drawLeds[3][1][y] = 3;
  
////---------------------------------------------------------------------------------------
////---------------------------------------------------------------------------------------
//// When adding these two lines it stops working

 /* drawLeds[3][2][x] = 4;
  drawLeds[3][2][y] = 2;*/
  
  
////---------------------------------------------------------------------------------------
////--------------------------------------------------------------------------------------- 

  
  lightUpLeds(drawLeds, 1000000);
}

And finnaly the function

// Start ligtUpLeds function

void lightUpLeds(int drawLeds[][height*width][2], int length){

 /***                                                                  ***/
 /***    no support for non quadratic matrices(momentary only 8*8)     ***/
 /***                                                                  ***/
  
  
int time = 400;  
  
/**** Variable to count number of y values ****/

int relativizelength = 0;

/**** Variable to save if the outshifting process has to be skiped or not ****/

int skip = 0;


/**** Variables to calculate  ****/

int dataX[totalNumberOfMatrices][width][height];   // x- Values per y value
for(int a=0; a < totalNumberOfMatrices; a++)
  for(int a2=0; a2 < 8; a2++)
    for(int a3= 0; a3 < 8; a3++)
      dataX[a][a2][a3] = 255;
    
int dataOutput[totalNumberOfMatrices][2][8] = {0};  // final 8 x- and y-values  
 

/**** Checking the array for values  ****/

for(int matrixNumber = 0; matrixNumber < totalNumberOfMatrices; matrixNumber++)
  for(int possibleLed = 0; possibleLed < width*height; possibleLed++)
    for(int row = 0; row < 8; row++){
      /**** iterate y values ****/
      if((drawLeds[matrixNumber][possibleLed][y]) == (row+1) )
        dataOutput[matrixNumber][y][row] = bytesY[matrixNumber][row];
      /**** iterate x values ****/
      if(drawLeds[matrixNumber][possibleLed][x] == (row+1) )
        dataX[matrixNumber][drawLeds[matrixNumber][possibleLed][y]-1][row] = bytesX[matrixNumber][row];
    }
  



/**** adapting the length of the action to the numbers of y values to light  up ****/

for(int b = 0; b < height; b++)
  for(int matrixNumber = 0; matrixNumber < totalNumberOfMatrices; matrixNumber++)
    if(dataOutput[matrixNumber][y][b] != 0 ){ relativizelength++; break;}

length /= relativizelength;

/**** Calculating final x-Output ****/

for(int matrixNumber = 0; matrixNumber < totalNumberOfMatrices; matrixNumber++)
  for(int u = 0; u < 8; u++ ){
     dataOutput[matrixNumber][x][u] = dataX[matrixNumber][u][7];
     for(int u2 = 0; u2 < width-1; u2++)
       dataOutput[matrixNumber][x][u] += dataX[matrixNumber][u][u2];
     dataOutput[matrixNumber][x][u] -= 255*(width-1);
  }
  
 

for(int repeat = 0; repeat < length - (length*relativizelength/1000)*40* totalNumberOfMatrices ; repeat++){  
  /*if(time > 200)
    time -= 100;
  else if(time > 100)
    time -= 50;
  else if(time > 20)
    time -= 10;
  else if(time > 1)
  time = 1;
  */
  if(time - time *0.1 > 1)
    time -= time *0.1;
  else
    time = 1;
      /****  - ((length*relativizelength)/1000)* (61* totalNumberOfMatrices)) : approximate conversion factor to get the right light on time when consindering computation time of Arduino****/
  for(int u = 0; u < 8; u++ ){     
    for(int matrixNumber = 0; matrixNumber < totalNumberOfMatrices; matrixNumber++)
      if(dataOutput[matrixNumber][y][u] != 0) skip++;
    if(skip != 0){   
      skip = 0;       
      /**** Making the LEDs shine after checking if they have to light up ****/
      digitalWrite(latchPin, LOW);
      for(int matrixToDisplay = (totalNumberOfMatrices-1); matrixToDisplay > -1; matrixToDisplay--){
        if(matrixToDisplay % 2 == 0){
          shiftOut(dataPin, clockPin, MSBFIRST, dataOutput[matrixToDisplay][x][u]);
          shiftOut(dataPin, clockPin, MSBFIRST, dataOutput[matrixToDisplay][y][u]);
        }else{
          shiftOut(dataPin, clockPin, MSBFIRST, dataOutput[matrixToDisplay][y][u]);
          shiftOut(dataPin, clockPin, MSBFIRST, dataOutput[matrixToDisplay][x][u]);
        }
      } 
      digitalWrite(latchPin, HIGH);
      delay(time);
      }
    }
  }
}

The issues occurs when adding the two marked lines, same issue occurs when trying to print a line while the lightUpLeds function is looping througth the "Making the LED shine loop".

Is there any better way to controll the 16x16 matrix(connected with 8 595er), to make the programm perform better? For example not using so many 3 dimensional arrays

In general you specify which LED you want to make shine(using x and y coordinates) and then the programm calculates the output bytes.

i'm wondering why you get out of memory i guess its not the complete code, or your array is way to large.
if you program efficiently 16*16 requires 256 bytes (is your array a byte type ?)

if you go for real efficient coding then 16*16 requires only 256 bits
an array of 32 bytes containing each 8 bits could do it, but then you have to work binary

or more easy an array of 16 integers (each 16 bit) and again work binary so zero is of one is on.
the complexity will be in setting the binary values, but thats only a bit of math.
you can do set by OR gate to enable it and other functions to disable a bit.

I am not sure if i Run out of memory i am just wondering why the code works without the marked code line(see code main program above) but it doesent with the two more lines. It also won't run when I try to print a text in the loop og the lightUpLeds function.

I also now that the marked code line works when it's the only line which is compiled by me

  const int bytesX[totalNumberOfMatrices][8] =  {  {127, 191, 223, 239, 247, 251, 253, 254}, 
                                                   {127, 191, 223, 239, 247, 251, 253, 254},
                                                   {254, 253, 251, 247, 239, 223, 191, 127},
                                                   {254, 253, 251, 247, 239, 223, 191, 127},
                                                };

All those values would fit in a byte array, cutting the array size in half.

okay thanks any further improvements?

Why are you passing 1000000 to an int? The use of length in LightUpLeds() doesn't look like int to me anyway.

  lightUpLeds(drawLeds, 1000000);
void loop() {
  int drawLeds[totalNumberOfMatrices][height*width][2] = {0};

drawleds[][][] is a local array of loop(). How does that get passed to the scope of lightUpLeds()?

void lightUpLeds(int drawLeds[][height*width][2], int length){

The use of length in LightUpLeds() doesn't look like int to me anyway.

In general I want to specify the light on time by looping through the process again and again. This is not very precise anyway and depends on the number of rows having to light up. Any recommendations for improvements?

It's passed to the function. I think it's not copied. There is made use of a pointer, but I am not sure. Or what do you mean?

void lightUpLeds(int drawLeds[][height*width][2], int length)

Type int can hold -32768 to +32767.
1000000 is going to overflow on the convert and the math you do with length won't work as int.

I'll wait to see what others say about that parameter syntax. When you did call the function it took a pointer so perhaps it's not passing a copy of the array on the stack -but- if so then there should be scope problems as the array is local to loop(). Why not use a global array and pointer?

Type int can hold -32768 to +32767.
1000000 is going to overflow on the convert and the math you do with length won't work as int.

Oh I forgot about that. :roll_eyes:

Is there a better way to specify the light on time without the loop I used which is very very unprecise.

Why not use a global array and pointer?

Again I am not quite sure but I think this is done automaticly(except the global array) but it might take some tipe to initialize the array each loop again.

I can help you with the parameter/variable 'length'. Make it and everything that does math with it into 32-bit type long that can hold +/- 2 billion. You can mix int in there if you cast to long.

int N = 1000; // N is a 16-bit value
long X = 1000000; // X is a 32-bit value
long Y = X - (long) N; // they (long) before N forces the compiler to use a 32-bit value for the math

Making sure the compiler does what you have in mind can sometimes make the difference. Besides some extra typing it doesn't hurt the code a bit and bonus is it is more clear to read. Same goes with parenthesis, extras don't hurt.

If it was me and I wanted my code debugged sooner, I'd salt in some serial prints to see what's going on with the variables and try changing that function definition to use pointers.

int myArray[ 40 ];
(int *) myArrayPtr = myArray; // points to myArray, the parenthesis is for clarity only
int my2dArray[ 8 ][ 8 ];
(int **) my2dArrayPtr = my2dArray; // pointer to pointer to array
my2dArray = &(my2dArray[ 4 ]); // the pointer doesn't have to point to the start of the array

That's just a bit of using pointers, a very powerful part of C. You can have pointers to functions and objects too. There is pointer math, myArrayPtr++ points to the next element in myArray for instance. The subject of pointers is good fundamental reading and practice in C/C++.

void myFunction( int *AP, long arg1 ) { .... }; // makes sure that only a pointer is passed

However it is up to you that the pointer accesses valid data space, what I do is make the array global to avoid scope problems.

Make it and everything that does math with it into 32-bit type long that can hold +/- 2 billion.

Okay thanks

I'd salt in some serial prints to see what's going on with the variables

When trying to salt in same serial prints the displaying on the matrix stops working and no line is printed on the serial monitor(out of Ram?)

It is quite possible but that's not proof either. Being too sure of hunches can have you locked onto false trails when debugging. For Sure Do Try Things Out but don't get stuck on "it must be this". It's never "that" until "that" fixes the code and even then you might have gotten the wrong "that". :grin:
With simple code the answers are easier to be sure of though.

-- You could try making a smaller version of your project to check the logic.

-- I notice that you make and fill that drawLeds array then have lightUpLeds() do the whole picture in one go, making yet another local array to do it. You could achieve major RAM savings by shrinking that to process 1 line at a time or even 1 led at a time. Do you really -need- to have everything in data at once? Does every led depend on every other led to determine ON or OFF?

I don't want to work out the what, how and why of lightUpLeds()... it's taking long enough to post this! If you can write it, you can trim it.

======================== some day, maybe now you will want to know what's below ====

-- You could try shifting the const arrays to PROGMEM (have to be compiled into flash, can't put them in at run-time) and copy an element at a time into RAM to process.

Here is a link to the AVR Libc modules page, it's the home of the C++ Arduino is based on:
http://www.nongnu.org/avr-libc/user-manual/modules.html
I have this and other pages bookmarked, couldn't get far without them.

Here is the Program Space module with PROGMEM access functions, etc, spelled out:
http://www.nongnu.org/avr-libc/user-manual/group__avr__pgmspace.html

You see it takes special functions and pointers to store and read constants in flash.
You can easily fit your const tables, bytesX and bytesY into PROGMEM. You will need to change how you use the data, a good bit of change for a beginner but you should learn as much too.

You need some #includes to use PROGMEM:

#include <avr/io.h>
#include <avr/pgmspace.h>

Here I have a 7 line menu (80 characters per line) stored to PROGMEM, and a special pointer to get the data with:

const char PROGMEM usageMsg[][80] = { // all this text stored in flash
  "          Piezo touch sensor tuner.",
  "Adjust vertical; enter W to subtract or X to add, and a number 0-255",
  "Adjust timescaler; enter A to subtract or D to add, and 0-16535",
  "to add 250 to vertical enter W250 in Serial Monitor and press enter",
  "seperate multiple commands with spaces; ex: W25 D240",
  "    ** this message stored in flash ram **",
  "    ** and printed with a 1 byte buffer ** :-P"
};

PGM_P Msg; // pointer into flash memory

The full program stores -every- const char string (not C++ String!) in PROGMEM. It has serial user I/O error messages, etc, as well as the menu.

I made this function to print the stored strings one char at a time:

void  printMsg( PGM_P FM )
{
  do 
  {
    B = pgm_read_byte( FM++ );    // FM++ is pointer math; look Ma, no indexes!
    if ( B )  Serial.print( B );
  }
  while ( B ); // when it reaches the terminating zero, it exits
}

And this function just to print the whole menu:

void printHelp(void)
{
  for ( byte i = 0; i < HELPLINES; i++ )
  {
    printMsg( usageMsg[ i ]);
    Serial.println();
  }
  Serial.println();
}

You could achieve major RAM savings by shrinking that to process 1 line at a time or even 1 led at a time.

1 led at a time won't work because when Multiplexing each line light up after the other, so it wouldn't make sense to light up each LED in a line and then light up each LED in the next line,would it. This would take far too long to get throug all the lines and to show up an image on the matrix, or the frequency is so high, that the LED's light on time is so short, that thay are quit dark.

But I think calculating 1 line at a time makes sense. So you mean I calculate the first line save it in an array and finnaly let the line shine. Then I calculate the second line and save it in the same array, right? and so on and so on

1 line at a time would cut more than 1 of those arrays down by a factor of 8 wouldn't it?

Additional: you could process 1 position at a time to store into a 1 line array in the lightUp function and only shift the bits out when that is full. What you do with drawLeds array could simply be a series of function calls to build the line.

I should mention that I find PROGMEM data retrieve then use does work noticeably slower than using data straight from RAM. My 7 line menu print kind of flows onto the Serial Monitor.

1 line at a time would cut more than 1 of those arrays down by a factor of 8 wouldn't it?

Quite a bit. Thanks for the tipp

Hi,

this is how the code looks now: much cleaner and Ram-saving :slight_smile:
Thanks very much :stuck_out_tongue:
Any further improvements?

// Start ligtUpLeds function

void lightUpLeds(){

int bytesToShift[4];


for(int i = 0; i < 10000; i++)  
for (int row = 0; row < 8; row++)
{
    for (int a = 0; a < 4; a++)
        bytesToShift[a] = 255;

    for (int matrixNumber = 0; matrixNumber < totalNumberOfMatrices; matrixNumber++)
        for (int possibleLed = 0; possibleLed < 8; possibleLed++)
            if (drawLeds[matrixNumber][possibleLed][row] == true)
              bytesToShift[matrixNumber] -= (255 - bytesX[matrixNumber][possibleLed]);
              
    if (bytesToShift[0] != 255 || bytesToShift[1] != 255 || bytesToShift[2] != 255 || bytesToShift[3] != 255)
    {
        digitalWrite(latchPin, LOW);
        for (int matrixToDisplay = (totalNumberOfMatrices - 1); matrixToDisplay > -1; matrixToDisplay--)
        {            
            if (matrixToDisplay % 2 == 0)
            {
              //Serial.println(bytesToShift[matrixToDisplay]);
              //Serial.println(bytesY[matrixToDisplay][row]);
                shiftOut(dataPin, clockPin, MSBFIRST, bytesToShift[matrixToDisplay]);
                shiftOut(dataPin, clockPin, MSBFIRST, bytesY[matrixToDisplay][row]);
            }
            else
            {  
              //Serial.println(bytesY[matrixToDisplay][row]);
              //Serial.println(bytesToShift[matrixToDisplay]);
                shiftOut(dataPin, clockPin, MSBFIRST, bytesY[matrixToDisplay][row]);
                shiftOut(dataPin, clockPin, MSBFIRST, bytesToShift[matrixToDisplay]);                            
            }    
        } 
        digitalWrite(latchPin, HIGH);
        delay(1);
    }

}


}

Any further improvements?

Add comments.
Use Tools + Auto format to properly indent the code.
Delete the useless blank lines between closing }.

This whole part to pack led ON/OFF into the bits of bytesToShift according to 2 arrays of bytes;

    for (int a = 0; a < 4; a++)
        bytesToShift[a] = 255;

    for (int matrixNumber = 0; matrixNumber < totalNumberOfMatrices; matrixNumber++)
        for (int possibleLed = 0; possibleLed < 8; possibleLed++)
            if (drawLeds[matrixNumber][possibleLed][row] == true)
              bytesToShift[matrixNumber] -= (255 - bytesX[matrixNumber][possibleLed]);

I think you can cut that way down using & and | operations on bytes in the place where you set drawLeds[][] values. You won't need the drawLeds[][] array, just fill bytesToShift[] directly right there and dispense with the code shown above completely.

You might practice with byte-wide bit operations some in code first, printing results in HEX or BIN to make it easier to 'see the bits'. I use unsigned 8-bit (type byte) so the sign bit of type char doesn't get in the way.

byte B = 1 << 3; // B = 0b00001000 ... 0b is binary notation instead of decimal
B = B | 1; // B = 0b00001001

byte A = 0b01010000;
B = B | A; // B = 0b01011001; | (OR) sets bits, all bits from both get copied to the result

bitWrite( B, 3, 1 ); // B = 0b00001000 ... how to set single bits without using shift as above
// but with shift you can move multiple bits in one go.

A = 0b00001111; // you don't have to put in the leading 0's but they don't hurt
B = A & 0b11110101; // B = 0b00000101; & (AND) masks bits, only matching bits get copied

These are the same as what you do with

bytesToShift[matrixNumber] -= (255 - bytesX[matrixNumber][possibleLed]);

only much easier to 'see through' and can manipulate all the bits in one operation if you have bytes prepared.

It will take some time to know this enough to do what I wrote could be done above. Be sure to see how more than just one or two parts work, easier/shorter paths will be apparent even if you don't get 'perfect' right away.
Remember, you already have code that works. Mission accomplished! The rest can be regarded as either apple-polishing or good practice/training for later efforts. In most cases, time spent polishing now should pay off with much greater time saved later.

Firstly thanks GoForSmoke for the nice tipps.

fill bytesToShift[] directly right there

filling bytesToShift[matrixNumber] directly instead of drawLeds[matrixNumber][x-coordinate][y-coordinate] would mean that I have to calculate the byte for a row by myself ending with bytesToShift[matrixNumber][x-row], right? So I have better/faster code, but have to think more for myself:) ?

But using the | and the & operators could reduce the array int bytesToShift[4] by half as I can initilize it as an byte array because there woun't be intermediate results which are for example negativ.

P.S. You examples are really good and easy to understand :slight_smile:

You are drawing a picture with leds. All are fully OFF or ON, no brightness levels.

Something like below only a different picture, different size, this is a 12x6 box in 16x8 space:
// OOOOOOOOOOOOOOOO
// OOXXXXXXXXXXXXXXOO
// OOXOOOOOOOOOOXOO
// OOXOOOOOOOOOOXOO
// OOXOOOOOOOOOOXOO
// OOXOOOOOOOOOOXOO
// OOXXXXXXXXXXXXXXOO
// OOOOOOOOOOOOOOOO

My bits will be 0 for O and 1 for X, 2 bytes or 1 int per line.

If I want, I can store many different 'bit-pictures' in PROGMEM and try to animate even.