Declaring this Var as global , Help me understand why it only works when global.

Chrisbee · March 26, 2017, 8:26am

I’m having trouble trying understanding why this variable “int rotatePos[3];” needs to be global to work as I expect it to.

If I make this Variable global the program runs fine and you see a stream of numbers counting up at my Serial.println(rotatePos[arrow]); //<–Result. = 1,2,3,4 ETC (Expected result)

However if I include this variable within Arrows{}, it no longer works as I would expect. It compiles, it loads but at the Serial.println(rotatePos[arrow]); //<–Result =1 and appears to stop. If I close “Serial Monitor” and reopen the result = 2, close and reopen “Serial Monitor” result = 3 and so on.

Why is this? What am I missing here!! >:(

look for the <---------------------------- in the code (4 off).

NOTE – The only place the Variable is used is in this sketch. The two instances of “int rotatePos[3];” is only for the demonstration in this thread.

int rotatePos[3]; <---------------------------------------------------- Here works as expected.

void Arrows(byte RunTime) {
 
int rotatePos[3];  //<---------------------------------------------- Here, does not work
  
byte arrowArray[] = {0x18, 0x3C, 0x7E, 0x7E, 0x7E, 0x66, 0x66, 0x42};
  byte startup[3] = {1, 0, 0};
  byte myX = 8;
  byte myY = 12;
  byte myZ = 8;
  byte XY = (myX  + myY - 2) * 2; //Arrow 0
  byte ZY = (myZ + myY - 2) * 2; //Arrow 1
  byte XZ = (myX + myZ - 2) * 2;//Arrow 2
  byte lag[] = {0, ZY / 2, 0};
  byte ArrowXY[XY];
  byte ArrowZY[ZY];
  byte ArrowXZ[XZ];
  byte fadeInDone = 0;
  byte startArrow = 0;
  int myColour = 0, ChangeColour = 0;
  unsigned long StartTime = millis();

  while (millis() < StartTime + RunTime * 1000) {
    CleanCube();
    memset(ArrowXY, 0, XY * sizeof(byte));
    memset(ArrowZY, 0, ZY * sizeof(byte));
    memset(ArrowXZ, 0, XZ * sizeof(byte));

    byte temp = 0;
    for (byte arrow = 0; arrow < 3; arrow++) {
      switch (arrow) {
        case 0://ArrowXY
          for (uint8_t i = 0; i < startup[arrow]; i++) {
            ArrowXY[i + lag[arrow]] = arrowArray[i];
            //  ArrowXY[i] = arrowArray[i];
          }
          //Loop

          //shift and loop the ArrowXY until the correct position is reached
          for (int8_t n = 0; n < rotatePos[arrow]; n++) {
            //shift first column into a temp array
            temp = ArrowXY[0];

            //shift all columns down
            for (int8_t r = 0; r < XY - 1; r++) {
              ArrowXY[r] = ArrowXY[r + 1];
            }

            //move data from first column into last
            ArrowXY[XY - 1] = temp;
          }

          //increment/reset rotatePos, this is what gives the rotation effect
         
          rotatePos[arrow]++; // <------------------------------------------------- This var.
         
          Serial.println(rotatePos[arrow]); //< ----------------------------------- **** Result ******
        
          if (rotatePos[arrow] > XY - 1) {
            rotatePos[arrow] = 0;

            //Controls the start of the next arrow.
            if (!fadeInDone) {
              if (startup[startArrow] >= 8) {
                startArrow++;
              }
              if (startArrow > 2) {
                fadeInDone = 1;
              }
            }
          }// end of loop
          //Wrap ArrowXY around the outside of the cube
          for (int8_t r = 0; r < XY; r++) {
            if (r >= 0 && r < myY) {//Going down the left side of cube
              copyColumn(arrow, myY - 1 - r, myX - 1, ArrowXY[XY - 1 - r]);
            } else if (r >= myY && r < myY + myX - 1) {//Going along the bottom
              copyColumn(arrow, 0, myX - (r - myX - (myY - myX)) - 2, ArrowXY[XY - 1 - r]);
            } else if (r >= myY + myX - 1 && r < myY + myX + myY - 2) {// Going up the right side
              copyColumn(arrow, r - (myY + myX - 2), 0, ArrowXY[XY - 1 - r]);
            } else if (r >= myY + myY + myX - 2 && r < (myY + myX) * 2 - 4) {//Going across the top
              copyColumn(arrow, myY - 1, r - (myY + myX + myY - 3), ArrowXY[XY - 1 - r]);
            }
          }//for (int8_t r = 0; r < XY; r++)
          break;//Case 0

        //************************************************************************************
        case 1://ArrowZY
          for (uint8_t i = 0; i < startup[arrow]; i++) {
            ArrowZY[i + lag[arrow]] = arrowArray[i];
            // ArrowZY[i] = arrowArray[i];
          }
          //Loop

          //shift and loop the ArrowZY until the correct position is reached
          for (int8_t n = 0; n < rotatePos[arrow]; n++) {
            //shift first column into a temp array
            temp = ArrowZY[0];

            //shift all columns down
            for (int8_t r = 0; r < ZY - 1; r++) {
              ArrowZY[r] = ArrowZY[r + 1];
            }

            //move data from first column into last
            ArrowZY[ZY - 1] = temp;
          }

          //increment/reset rotatePos, this is what gives the rotation effect
          rotatePos[arrow]++;
          if (rotatePos[arrow] > ZY - 1) {
            rotatePos[arrow] = 0;
          }//if (rotatePos[arrow] > ZY - 1)

          //Wrap ArrowZY around the outside of the cube
          for (int8_t r = 0; r < ZY; r++) {
            if (r >= 0 && r < myY) {//Going down the BACK side of cube
              copyColumn(arrow, myY - 1 - r, myZ - 1, ArrowZY[ZY - 1 - r]);
            } else if (r >= myY && r < myY + myZ - 1) {//Going along the bottom
              copyColumn(arrow, 0, myZ - (r - myZ - (myY - myZ)) - 2, ArrowZY[ZY - 1 - r]);
            } else if (r >= myY + myZ - 1 && r < myY + myZ + myY - 2) {// Going up the FRONT  side
              copyColumn(arrow, r - (myY + myZ - 2), 0, ArrowZY[ZY - 1 - r]);
            } else if (r >= myY + myY + myZ - 2 && r < (myY + myZ) * 2 - 4) {//Going across the top
              copyColumn(arrow, myY - 1, r - (myY + myZ + myY - 3), ArrowZY[ZY - 1 - r]);
            }
          }//for (int8_t r = 0; r < ZY; r++)
          break;//Case 1

        //*******************************************************************************************
        case 2://ArrowXZ
          for (uint8_t i = 0; i < startup[arrow]; i++) {
            ArrowXZ[i + lag[arrow]] = arrowArray[i];
            //  ArrowXZ[i] = arrowArray[i];
          }
          //Loop

          //shift and loop the ArrowXZ until the correct position is reached
          for (int8_t n = 0; n < rotatePos[arrow]; n++) {
            //shift first column into a temp array
            temp = ArrowXZ[0];

            //shift all columns down
            for (int8_t r = 0; r < XZ - 1; r++) {
              ArrowXZ[r] = ArrowXZ[r + 1];
            }

            //move data from first column into last
            ArrowXZ[XZ - 1] = temp;
          }

          //increment/reset rotatePos, this is what gives the rotation effect
          rotatePos[arrow]++;
          if (rotatePos[arrow] > XZ - 1) {
            rotatePos[arrow] = 0;
          }//if (rotatePos[arrow] > XZ - 1)

          //Wrap ArrowXZ around the outside of the cube
          for (int8_t r = 0; r < XZ; r++) {
            if (r >= 0 && r < myX) {//Going around the BACK side of cube
              copyColumn(arrow, myX - 1 - r, myZ - 1, ArrowXZ[XZ - 1 - r]);
            } else if (r >= myX && r < myX + myZ - 1) {//Going around right side
              copyColumn(arrow, 0, myZ - (r - myZ - (myX - myZ)) - 2, ArrowXZ[XZ - 1 - r]);
            } else if (r >= myX + myZ - 1 && r < myX + myZ + myX - 2) {// Going around the FRONT side
              copyColumn(arrow, r - (myX + myZ - 2), 0, ArrowXZ[XZ - 1 - r]);
            } else if (r >= myX + myX + myZ - 2 && r < (myX + myZ) * 2 - 4) {//Going around the left side
              copyColumn(arrow, myX - 1, r - (myX + myZ + myX - 3), ArrowXZ[XZ - 1 - r]);
            }
          }//for (int8_t r = 0; r < XZ; r++)
          break;//Case 2
          //*******************************************************************************************************
      }//switch (arrow)
    }//for (byte arrow = 0;arrow<3;arrow++)

    if (!fadeInDone) {
      (startup[startArrow] < 8) ? startup[startArrow]++ : 8;
    }
    // increment_colour
    if (ChangeColour++ > sizeof(arrowArray) / 2) {
      ChangeColour = 0;
      GetColour(myColour++);
      if (myColour >= Size_CW) myColour = myColour - Size_CW;
    }//if (ChangeColour++ > sizeof(arrowArray) / 2)
    delay(75);
  }//while (millis() < StartTime + RunTime * 1000)
  CleanCube();
}

6v6gt · March 26, 2017, 8:34am

If you declare the variable inside a function, under normal circumstances, its value is completely reset every time the function is called. Therefore, you loose any information in it which may have resulted from a previous call of the function. (The special case is when you declare the variable as static.)

If you declare a global variable and a local variable (local means inside a function) with exactly the same name, the function uses its version of the variable. Any other function uses the global version of the variable.

Chrisbee · March 26, 2017, 11:31pm

Sure, this Variable is reset when the function is called then its value is incremented while in a “While loop”.

This loop runs for 40 seconds drawing arrows on the outside edge of my RGB LRD cube then the cube is “Cleaned” and the next animation starts. Arrows(byte RunTime) - RunTime = 40.

After declaring all variables, the rest of the code runs within the “While Loop”.

The two instances of the variable on the example above, is only to show people the location of the variable in my sketch that I’m questioning.

In my operating sketch “int rotatePos[3];” is the global version only. I only have one instance of this variable.

If I make this variable Global the code works, if I make it local the codes does not work.

I don’t understand why this is so, and that is what I’m asking. To me this variable should work local too.

Chrisbee · March 27, 2017, 12:57am

6v6gt:
If you declare the variable inside a function, under normal circumstances, its value is completely reset every time the function is called. Therefore, you loose any information in it which may have resulted from a previous call of the function. (The special case is when you declare the variable as static.)

If you declare a global variable and a local variable (local means inside a function) with exactly the same name, the function uses its version of the variable. Any other function uses the global version of the variable.

Well, I made the variables static "static int rotatePos[3];" and it now works as a local variable, Thanks 6v6gt.
But I still not really understand why I need to do this.

6v6gt · March 27, 2017, 2:32am

I have not seen this "dynamic" array bounds construct in C++, but clearly the compiler has allowed it and I guess that it does what you appear to want it to do, i.e creating/initialising a 36 byte array :

byte myX = 8;
byte myY = 12;
. . .
byte XY = (myX  + myY - 2) * 2; //Arrow 0
. . .
byte ArrowXY[XY];   
. . .
memset(ArrowXY, 0, XY * sizeof(byte));
. . .

But, anyway, if declaring a variable as global works in the case you have mentioned, but does not work if the variable is declared as local, I'd start suspecting that some stack storage is being overwritten by an array bounds issue or similar overflow, and your code contains a few examples of complex array subscript calculations.

adwsystems · March 27, 2017, 2:53am

Chrisbee:
Well, I made the variables static “static int rotatePos[3];” and it now works as a local variable, Thanks 6v6gt.
But I still not really understand why I need to do this.

Seeing only the one function it is difficult to tell. But there must be some dependency on the values of rotatePos[arrow] at the end of one call of the function to the start of the next (ie., the call needs to start where it left off). In particular around line 43, “for (int8_t n = 0; n < rotatePos[arrow]; n++)” You first call this may be a zero, but 15 lines later you modify the value (you highlighted it), then throughout the remainder of the function refer to it. If it is a local variable, reset, on every call, all values of rotatePos will be 0. Declaring it as static or global will allow rotatePos to maintain its previous value for the next function call.

Being local resets, not only arrow, but also all the rotatePos values. Declaring them as static causes them to retain their value from the end of one function call to the start of the next. Global variables would have the same effect, only global variables can be seen, well, globally by all functions.

6v6gt · March 27, 2017, 8:48am

OK. To help distinguish between the 2 cases mentioned so far that could account for this phenomenon, that is (a) memory corruption and (b) program logic error , try the following:

Declare rotatePos[3] as global.
immediately clear its value when the function is called:

void Arrows(byte RunTime) {
  for ( byte i = 0 ; i < 3 ; i++ ) {
     rotatePos[i] = 0 ;
  }
  . . .
  . . .
}

and see if the behaviour is now the same as in the “local declaration” case

Chrisbee · March 28, 2017, 1:47am

6v6gt:
I have not seen this "dynamic" array bounds construct in C++, but clearly the compiler has allowed it and I guess that it does what you appear to want it to do, i.e creating/initialising a 36 byte array :
byte myX = 8;

byte myY = 12;
. . .
byte XY = (myX + myY - 2) * 2; //Arrow 0
. . .
byte ArrowXY[XY];
. . .
memset(ArrowXY, 0, XY * sizeof(byte));
. . .





But, anyway, if declaring a variable as global works in the case you have mentioned, but does not work if the variable is declared as local, I'd start suspecting that some stack storage is being overwritten by an array bounds issue or similar overflow, and your code contains a few examples of complex array subscript calculations.

The reason for the complexity is to make the code transportable to cubes with different dimensions.
My cube is 8 LEDs wide , 8 LED’s deep and 12 LEDs high. The idea was change myY to 8 then this code should work on other people cubes, which most are 8 x 8 x 8 LEDs.

econjack · March 28, 2017, 2:07am

In the post titled Useful Links at the top of this Forum is a section called C++ Programming. A subcategory in that section is one dealing with Scope. Read that section and see if it helps.

Chrisbee · March 28, 2017, 2:08am

Umm – this line of code is now redundant “for (int8_t n = 0; n < rotatePos[arrow]; n++)” , it was there before I made the arrows into case statements. I will remove it as it is only slowing things down. Right now, once all 3 arrows are running, this loop will run the same piece of code 3 times with no reason too. Thanks for making me look at that again.

adwsystems:
If it is a local variable, reset, on every call, all values of rotatePos will be 0. Declaring it as static or global will allow rotatePos to maintain its previous value for the next function call.

This is the bit I don’t understand. The only time that rotatePos is required/used is during the call of Arrows(). Arrows runs for 40 secounds, arrows move around the outside of the cube for the duration, than we clear then and start a different animation. Here is the rest of the code that is called while Arrows runs.

void copyColumn(uint8_t arrow, uint8_t i, uint8_t j, uint8_t col) {
  switch (arrow) {
    case 0:
      for (uint8_t k = 0; k < Size_Z; k++) {
        if (col & (0x01 << k)) {
          LED(i, j, k, myBlue, myGreen, myRed);
        }
      }
      break;
    case 1:
      for (uint8_t k = 0; k < Size_X; k++) {
        if (col & (0x01 << k)) {
          LED(i , k, j, myGreen, myBlue, myRed);
        }
      }
      break;
    case 2:
      for (uint8_t k = 0; k < Size_Y; k++) {
        if (col & (0x01 << k)) {
          LED(k + Size_Y / 2 - 4 , i, j, myRed, myGreen, myBlue);
        }
      }
      break;
  }
}

void LED(int CY, int CX, int CZ, int CR, int CG, int CB) { //****LED Routine****LED Routine****LED Routine****LED Routine


  CY = constrain(CY, 0, Size_Y - 1);//Cube Y axis
  CX = constrain(CX, 0, Size_X - 1);//Cube X axis
  CZ = constrain(CZ, 0, Size_Z - 1);//Cube Y axis
  CR = constrain(CR, 0, (1 << BAM_RESOLUTION) - 1); //Cube Red
  CG = constrain(CG, 0, (1 << BAM_RESOLUTION) - 1); //Cube Green
  CB = constrain(CB, 0, (1 << BAM_RESOLUTION) - 1); //Cube Blue

  int WhichByte = (CY * Size_Layer + CX * Size_X + CZ) / 8;


  int WhichBit = (CY * Size_Layer + CX * Size_X + CZ) - (8 * WhichByte) ;


 
  for (byte I = 0; I < BAM_RESOLUTION; I++) {
    //*** RED ***
    bitWrite(red[I][WhichByte], WhichBit, bitRead(CR, I));

    //*** GREEN ***
    bitWrite(green[I][WhichByte], WhichBit, bitRead(CG, I));

    //*** BLUE ***
    bitWrite(blue[I][WhichByte], WhichBit, bitRead(CB, I));
  }

}//****LED ROUTINE END****

//Clean Cube *********************************************
void CleanCube() {
  memset(red, 0, (Size_Cube / 8)* BAM_RESOLUTION * sizeof(byte));
  memset(green, 0, (Size_Cube / 8)* BAM_RESOLUTION * sizeof(byte));
  memset(blue, 0, (Size_Cube / 8)* BAM_RESOLUTION * sizeof(byte));

}// *****************************************************

void GetColour(int colour) {

  if (colour >= 0 && colour < Size_CW ) {   // Standard colours from colour wheel
    myRed = Colour[colour][0];
    myGreen = Colour[colour][1];
    myBlue = Colour[colour][2];
  } else  if (colour == Size_CW + 1) {    //White
    myRed = ((1 << BAM_RESOLUTION) - 1) * .67;
    myGreen = ((1 << BAM_RESOLUTION) - 1);
    myBlue = ((1 << BAM_RESOLUTION) - 1);
  } else  if (colour == Size_CW + 2) {    //Black
    myRed = 0;
    myGreen = 0;
    myBlue = 0;
  } else if (colour < 0) {          //If "colour" is a negitive value
    colour += Size_CW;
    if (colour < 0) {
      // colour = 0;               // Catch all ("colour" Still < 0)
      Error(0);
    }
    myRed = Colour[colour][0];
    myGreen = Colour[colour][1];
    myBlue = Colour[colour][2];
  } else {                  //If "colour" is greater than Size of Colour Wheel.
    colour -= Size_CW;
    if (colour > Size_CW) {
      //  colour = 0;               // Catch all ("colour" Still > Size of Colour Wheel)
      Error(1);
    }
    myRed = Colour[colour][0];
    myGreen = Colour[colour][1];
    myBlue = Colour[colour][2];
  }

}

And the interrupt code, which will make this post to long.

Chrisbee · March 28, 2017, 3:03am

OK, I think I understand now. When one of the other functions are called within Arrows() and because rotatePos is not protected, I’m overwrite rotatePos with a value outside what is expected elsewhere.
rotatePos is managing the position of the arrow updates, so the fact other variable get over written does not really matter, but it does matter for rotatePos. rotatePos needs to be protected for this sketch to continue.

system · May 5, 2021, 10:25pm