Passing an array into a function optimization

Using Arduino Programming Questions
#1

I am brand new to Arduino programming. I'm calling the function "writeDisp" and passing on 6 integers.
Is there a better wat to write to a global array than this?

I have the array defined before setup:

int disp[6];

Here is the function call:

writeDisp(171, 198, 255, 230, 198, 42);

here is the function:

void writeDisp(int d1, int d2, int d3, int d4, int d5, int d6){
  digitalWrite(latPIN, 0);
  disp[0] = d1;
  disp[1] = d2;
  disp[2] = d3;
  disp[3] = d4;
  disp[4] = d5;
  disp[5] = d6;
  lastdisp[0] = d1;
  lastdisp[1] = d2;
  lastdisp[2] = d3;
  lastdisp[3] = d4;
  lastdisp[4] = d5;
  lastdisp[5] = d6;
  
  for (int i = 5; i >= 0; i--) {
    shiftOut(datPIN, clkPIN, MSBFIRST, disp[i]);
    }
}
#2

Since you have the array defined before setup, it's global so you don't need to pass it at all. However it is good practice to keep variables local to where they're actually used. That way you can use one function to work on many different inputs.

Without totally re-writing your code, I think this will do what you want...

void writeDisp(int d[]){
  digitalWrite(latPIN, 0);

  for (int i = 5; i >= 0; i--) {
    lastdisp[i] = d[i];
    shiftOut(datPIN, clkPIN, MSBFIRST, d[i]);
    }
}

You would call this by first setting all the elements of the disp array to whatever you want and then call writeDisp(disp);

Note that this has the side-effect of updating another global array. Plus it has no idea how long the disp array actually is. If disp or lastdisp were accidentally defined with only 4 elements then this will write random stuff to other variables that you're using.

#3

Thank you! I was thinking about it completely incorrectly. I changed everything now, keeping an array that i'll use over and over global and localizing everything else. The function code I have now starts like this and creates 2 local arrays and one global (disp):

void writeDisp(int d1, int d2, int d3, int d4, int d5, int d6){
 int nowdisp[6] = {d1, d2, d3, d4, d5, d6};
 int voldisp[6];
 for (int i = 5; i >= 0; i--) {
   voldisp[i] = disp[i];
   disp[i] = nowdisp[i];  
 }
#4

Without context, it is difficult to say if you're on the right track. Why all the copying?

The usual way was shown by MorganS

void writeDisp(int d[]){
  ...
  ...
}

I would add a 'number of bytes' component

void writeDisp(int d[], uint8_t numBytes)
{
  digitalWrite(latPIN, 0);

  for (int i = numBytes - 1; i >= 0; i--) {
    lastdisp[i] = d[i];
    shiftOut(datPIN, clkPIN, MSBFIRST, d[i]);
  }
}

And then in e.g. loop()

writeDisp(someArray, sizeof(someArray)/sizeof(someArray[0]));

where someArray is the array that contains your d1..d6. The sizeof(someArray)/sizeof(someArray[0]) calculates the size of the array in elements (not in bytes) so if you decide that you only need 5 numbers or that you need 10 numbers (and hence resize someArray), it will automatically follow and you do not need to adjust writeDisp. Usually one uses a macro for sizeof(someArray)/sizeof(someArray[0]) like shown below that can be applied to any array of any type.

// macro to calculate number of elements in an array
#define NUMELEMENTS(x) sizeof(x)/sizeof(x[0])

// some arrays for demo purposes
int intArray[5];
byte byteArray[6];
float floatArray[4];

void setup()
{
  Serial.begin(57600);
  
  Serial.print("The size of intArray is ");
  Serial.print(sizeof(intArray));
  Serial.println(" bytes");
  Serial.print("intArray contains ");
  Serial.print(NUMELEMENTS(intArray));
  Serial.println(" elements");

  Serial.print("The size of byteArray is ");
  Serial.print(sizeof(byteArray));
  Serial.println(" bytes");
  Serial.print("byteArray contains ");
  Serial.print(NUMELEMENTS(byteArray));
  Serial.println(" elements");

  Serial.print("The size of floatArray is ");
  Serial.print(sizeof(floatArray));
  Serial.println(" bytes");
  Serial.print("floatArray contains ");
  Serial.print(NUMELEMENTS(floatArray));
  Serial.println(" elements");

}

void loop()
{
}
#5

I’m making a a software display subroutine with assignable effects for six shift registered seven segment diplays. You can set the effect with the variable “effect” (0,1,2,3,4). I am having trouble compiling the “void writeDisp(int d)” line. It doesn’t seem to want to parse the input into an array. And i need a persistent array that survives the function (disp) for the wipe/shutter effect, hence the first copy of the array. The other two copies of the array get destroyed by the wipe/shutter. I’m still in ATARI BASIC programming mode. Here is the whole test code so far (the setup is just sending test display info).

int datPIN = 2;
int latPIN = 3;
int clkPIN = 4;
int disp[6];
int effect;
int effdel;
static int alphnum[] = {192, 249, 164, 176, 153, 146, 131, 248, 128, 144, 127};


void setup(){
 pinMode(datPIN, OUTPUT);
 pinMode(latPIN, OUTPUT);
 pinMode(clkPIN, OUTPUT);
 digitalWrite(latPIN, 0);
 effdel = 50;
 effect = 4;
 writeDisp(255, 255, 255, 255, 255, 255); //blank 
 delay(1000);
 writeDisp(191, 198, 199, 192, 198, 138); //title "-CLOCK"
 }

void loop(){
}


// -------------------------------------------------------------
// writeDisp (digit 1, 2, 3, 4, 5, 6) 
// effect 0 = pop on, 1 = left to right, 2 = right to left
// effect 3 = shutter left right left, 4 shutter left right left
// -------------------------------------------------------------

void writeDisp(int d1, int d2, int d3, int d4, int d5, int d6){
 int nowdisp[6] = {d1, d2, d3, d4, d5, d6};
 int voldisp[6];
 for (int i = 5; i >= 0; i--) {
   voldisp[i] = disp[i];
   disp[i] = nowdisp[i];  
 }
 
 // POP ON
 if (effect == 0) {
   for (int i = 5; i >= 0; i--) {
     shiftOut(datPIN, clkPIN, MSBFIRST, nowdisp[i]);
     }
   digitalWrite(latPIN, 1);
   digitalWrite(latPIN, 0);
 }
 
 // WIPE LEFT TO RIGHT
 else if (effect == 1) {
 int blkdisp[6] = {255,255,255,255,255,255};
   for (int g = 0; g <= 5; g++) {
   for (int i = 5; i >= 0; i--) {
     shiftOut(datPIN, clkPIN, MSBFIRST, voldisp[i]);
     }
     digitalWrite(latPIN, 1);
     digitalWrite(latPIN, 0);
     voldisp[g] = 255;
     delay(effdel);
   }
   for (int g = 0; g <= 5; g++) {
     blkdisp[g] = nowdisp[g];
   for (int i = 5; i >= 0; i--) {
     shiftOut(datPIN, clkPIN, MSBFIRST, blkdisp[i]);
     }
     digitalWrite(latPIN, 1);
     digitalWrite(latPIN, 0);
     delay(effdel);  
   }
 }   
 // WIPE RIGHT TO LEFT
 else if (effect == 2) {
 int blkdisp[6] = {255,255,255,255,255,255};
   for (int g = 5; g >= 0; g--) {
   for (int i = 5; i >= 0; i--) {
     shiftOut(datPIN, clkPIN, MSBFIRST, voldisp[i]);
     }
     digitalWrite(latPIN, 1);
     digitalWrite(latPIN, 0);
     voldisp[g] = 255;
     delay(effdel);
   }
   for (int g = 5; g >= 0; g--) {
     blkdisp[g] = nowdisp[g];
   for (int i = 5; i >= 0; i--) {
     shiftOut(datPIN, clkPIN, MSBFIRST, blkdisp[i]);
     }
     digitalWrite(latPIN, 1);
     digitalWrite(latPIN, 0);
     delay(effdel);  
   }
 }
 // SHUTTER LEFT TO RIGHT TO LEFT
 else if (effect == 3) {
 int blkdisp[6] = {255,255,255,255,255,255};
   for (int g = 0; g <= 5; g++) {
   for (int i = 5; i >= 0; i--) {
     shiftOut(datPIN, clkPIN, MSBFIRST, voldisp[i]);
     }
     digitalWrite(latPIN, 1);
     digitalWrite(latPIN, 0);
     voldisp[g] = 255;
     delay(effdel);
   }
   for (int g = 5; g >= 0; g--) {
     blkdisp[g] = nowdisp[g];
   for (int i = 5; i >= 0; i--) {
     shiftOut(datPIN, clkPIN, MSBFIRST, blkdisp[i]);
     }
     digitalWrite(latPIN, 1);
     digitalWrite(latPIN, 0);
     delay(effdel);  
   }
 }
 
 // SHUTTER RIGHT TO LEFT TO RIGHT
 else if (effect == 4) {
 int blkdisp[6] = {255,255,255,255,255,255};
   for (int g = 5; g >= 0; g--) {
   for (int i = 5; i >= 0; i--) {
     shiftOut(datPIN, clkPIN, MSBFIRST, voldisp[i]);
     }
     digitalWrite(latPIN, 1);
     digitalWrite(latPIN, 0);
     voldisp[g] = 255;
     delay(effdel);
   }
   for (int g = 0; g <= 5; g++) {
     blkdisp[g] = nowdisp[g];
   for (int i = 5; i >= 0; i--) {
     shiftOut(datPIN, clkPIN, MSBFIRST, blkdisp[i]);
     }
     digitalWrite(latPIN, 1);
     digitalWrite(latPIN, 0);
     delay(effdel);  
   }
 }
}
#6 
   for (int i = 5; i >= 0; i--) {

shiftOut(datPIN, clkPIN, MSBFIRST, blkdisp[i]);
    }

This little snippet is repeated in many many places in your code. Try to make that into a function. You should have only one function which writes to the display. Everything else should make changes to the display through that function. Well, maybe two functions - sometimes it's neater to have a clearDisp() function or there might be some other operation you do a lot that warrants a function.

#7

ebasta:
I am having trouble compiling the "void writeDisp(int d)" line. It doesn't seem to want to parse the input into an array.

In which case you have to show us the code that does not work and the (full) error message.

#8

When I try to pass the display information to writeDisp by using

writeDisp(255, 255, 255, 255, 255, 255)

void writeDisp(int d) gives this error:

invalid conversion from 'int' to 'int*' [-fpermissive]

#9

You can't pass 6 integers to a function which expects a single array.

Either do this...

  int tempArray[] = {255,255,255,255,255,255};
  writeDisp(tempArray);

Or do this...

void writeDisp(int d1, int d2, int d3, int d4, int d5, int d6){
  int nowdisp[6] = {d1, d2, d3, d4, d5, d6};
  writeDisp(nowdisp);
}

Yes, you are allowed to have two functions with the same name. They must have different parameter types however.

#10