Having trouble with static variable in loop() "not changing"

[budfriendguy]

Full disclosure that I am not very familiar with C++, which will be clear after review of the sample below.

I am using an Arduino based light controller to toggle sequences of lights, and am making use of the RX/TX pins due to the board's lack of I/O pins. Info on the specific board here.

In loop() there is a static variable declared to keep track of the number of button presses. In the simple example below, everything works as expected and the appropriate lights turn on with each subsequent button press. This tells me that the counter pressCount is working as expected.

  // this works and will light up the correct lights (pins 2-7) with subsequent button presses
  debounceReadButton(TX_PIN, buttonState, buttonHistory); 
  if (!lastButtonState && buttonState) {
    ++pressCount;
    digitalWrite(
      pressCount + 1,
      HIGH
    );
  }
  lastButtonState = buttonState;

However, as shown below, once pressCount is used as an index to reference an array value on SEQUENCES; the loop() function seems to ignore value changes on pressCount. As pressCount changes, it would seem that each time loop() runs it would be referencing a new light sequence. This is not the case, however.

#define RX_PIN 0
#define TX_PIN 1

#define RED_A 7
#define YELLOW_A 6
#define GREEN_A 5

#define RED_B 4
#define YELLOW_B 3
#define GREEN_B 2

const int LIGHTS[] = {
  RED_A, YELLOW_A, GREEN_A,
  RED_B, YELLOW_B, GREEN_B
};



/*
  LIGHT MAP
  
  [ ]      RED_A
    [ ]    YELLOW_A
      [ ]  GREEN_A
    [ ]    YELLOW_B
  [ ]      RED_B

  LIGHT SEQUENCE STRUCT
  [0] = lights
  [1] = level
  [2] = duration
*/



struct instruction {
  int lights[6];
  int level;
  int duration;
};



// TURN OFF ALL LIGHTS
void lightsOut() {
  // cite: forum.arduino.cc/t/understanding-sizeof-question/574698/4
  const size_t lightCount = sizeof(LIGHTS) / sizeof(*LIGHTS); 
  for (int i = 0; i < lightCount; i++) {
    pinMode(LIGHTS[i], OUTPUT);
    digitalWrite(LIGHTS[i], LOW);
  } // for lightCount
} // initializeLights



// START TRACKING BUTTON PRESSES
void initializeButtons() {
  pinMode(RX_PIN, INPUT_PULLUP);
  // pinMode(TX_PIN, INPUT_PULLUP);
}



void setup() {
  delay(500);  
  initializeButtons();
} // setup



void loop() {

  const instruction SEQUENCES[][20] = {
    // BLINK
    {
      {
        { RED_A, RED_B, YELLOW_A, YELLOW_B, GREEN_A },
        HIGH,
        1000
      }, {
        { RED_A, RED_B, YELLOW_A, YELLOW_B, GREEN_A },
        LOW,
        1000
      },
      /* PADDING */ {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}
    },
    // LEFT TO RIGHT BLINK
    {
      {
        { RED_A, RED_B },
        HIGH,
        1000
      }, {
        { RED_A, RED_B },
        LOW,
        100
      }, {
        { YELLOW_A, YELLOW_B },
        HIGH,
        1000
      }, {
        { YELLOW_A, YELLOW_B },
        LOW,
        100
      }, {
        { GREEN_A },
        HIGH,
        1000
      }, {
        { GREEN_A },
        LOW,
        100
      },
      /* PADDING */ {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}
    },
    // LEFT TO RIGHT FILL
    {
      {
        { RED_A, RED_B },
        HIGH,
        1250
      }, {
        { YELLOW_A, YELLOW_B },
        HIGH,
        1250
      }, {
        { GREEN_A },
        HIGH,
        1875
      }, {
        { RED_A, RED_B, YELLOW_A, YELLOW_B, GREEN_A },
        LOW,
        1250
      },
      /* PADDING */ {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {},
    },
    // TOP TO BOTTOM BLINK
    {
      {
        { RED_A },
        HIGH,
        1000
      }, {
        { RED_A },
        LOW,
        100
      }, {
        { YELLOW_A },
        HIGH,
        1000
      }, {
        { YELLOW_A },
        LOW,
        100
      }, {
        { GREEN_A },
        HIGH,
        1000
      }, {
        { GREEN_A },
        LOW,
        100
      }, {
        { YELLOW_B },
        HIGH,
        1000
      }, {
        { YELLOW_B },
        LOW,
        100
      }, {
        { RED_B },
        HIGH,
        1000
      }, {
        { RED_B },
        LOW,
        100
      },
      /* PADDING */ {}, {}, {}, {}, {}, {}, {}, {}, {}, {}
    },
    // TOP TO BOTTOM FILL
    {
      {
        { RED_A },
        HIGH,
        1000
      }, {
        { YELLOW_A },
        HIGH,
        1000
      }, {
        { GREEN_A },
        HIGH,
        1000
      }, {
        { YELLOW_B },
        HIGH,
        1000
      }, {
        { RED_B },
        HIGH,
        1000
      }, {
        { RED_A, RED_B, YELLOW_A, YELLOW_B, GREEN_A },
        LOW,
        1000
      },
      /* PADDING */ {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}
    }
  }; // SEQUENCES



  // cite: forum.arduino.cc/t/is-it-possible-to-add-a-push-button-switch-in-arduino-port-0-or-1/427968/15
  // default state for N.O switch with pullup
  static bool buttonHistory[3] = { true, true, true };
  static bool buttonState = true;
  static bool lastButtonState = true;
  // counts button presses
  static int pressCount = 0;


  // read button input state
  debounceReadButton(TX_PIN, buttonState, buttonHistory); 
  if (!lastButtonState && buttonState) {
    ++pressCount;
    digitalWrite(
      pressCount + 1,
      HIGH
    );
  }
  lastButtonState = buttonState;



  // NOTE: the code within the for loop does not seem to ever use the updated value of `pressCount`
  // cite: forum.arduino.cc/t/understanding-sizeof-question/574698/4
  size_t stepCount = sizeof(SEQUENCES[pressCount]) / sizeof(*SEQUENCES[pressCount]); 
  for (int i = 0; i < stepCount; i++) {

    size_t lightCount = sizeof(SEQUENCES[pressCount][i].lights) / sizeof(*SEQUENCES[pressCount][i].lights); 
    if (lightCount > 0) {
      for (int j = 0; j < lightCount; j++) {
        digitalWrite(
          SEQUENCES[pressCount][i].lights[j],
          SEQUENCES[pressCount][i].level
        );
      
        delay(SEQUENCES[pressCount][i].duration);
      } // for lightCount
    }
  } // for stepCount



  // limit loop rate for sake of the debouncer
  delay(20);
} // loop



void debounceReadButton(byte pin, bool &currentState, bool history[]) {
  history[2] = history[1];
  history[1] = history[0];
  history[0] = digitalRead(pin);
  
  // return new state if history aligns
  // otherwise leave state unchanged
  if (
    (history[0] == history[1]) &&
    (history[1] == history[2])
  ) {
    currentState = history[0];
  } // if
} // debounceReadButton

It seems there is some fundamental understanding I am missing here either about Arduino or C++ development.

[red_car]

Could you please post all your code.

@budfriendguy

In the simple example below, everything works as expected

@budfriendguy

However, as shown below,

Where do you show that?

[DrDiettrich]

@budfriendguy

 static bool buttonState = true;

This makes buttonState known only inside loop(). Move all shared variable declarations to global scope, before all function definitions.

[6v6gt]

One big problem you have is that when you use the TX pin for something else you can't use Serial.print() for debugging. It may be better to temporarily free up some pins for debugging this section of code.
The second problem is the complexity of the array to represent the light pattern. Maybe an array of structs would be better.
If you are using a counter (pressCount) to index an array, you should also include code to prevent an overflow.
One common problem of variable not appearing to update is when the same variable name is defined both globally and local to a function. These then are two completely different variables.

[budfriendguy]

@red_car

Could you please post all your code.

My first code sample was missing the part that proved the value of pressCount was correctly incrementing. I have fixed the sample but will include it here also:

  // this works and will light up the correct lights (pins 2-7) with subsequent button presses
  debounceReadButton(TX_PIN, buttonState, buttonHistory); 
  if (!lastButtonState && buttonState) {
    ++pressCount;
    digitalWrite(
      pressCount + 1,
      HIGH
    );
  }
  lastButtonState = buttonState;

With the first button press, the first light will turn on, with the second, the second will turn on and so on with each subsequent button press. So the value of pressCount is correctly incrementing.

But when using that same variable to lookup the index of a light sequence on SEQUENCES the loop() function will seemingly not use the updated value of pressCount to look up any indices but the initial value.

Here is the bit of code I am referring to which is a portion of the full sample in my first post:

  // cite: forum.arduino.cc/t/understanding-sizeof-question/574698/4
  size_t stepCount = sizeof(SEQUENCES[pressCount]) / sizeof(*SEQUENCES[pressCount]); 
  for (int i = 0; i < stepCount; i++) {

    size_t lightCount = sizeof(SEQUENCES[pressCount][i].lights) / sizeof(*SEQUENCES[pressCount][i].lights); 
    if (lightCount > 0) {
      for (int j = 0; j < lightCount; j++) {
        digitalWrite(
          SEQUENCES[pressCount][i].lights[j],
          SEQUENCES[pressCount][i].level
        );
      
        delay(SEQUENCES[pressCount][i].duration);
      } // for lightCount
    }
  } // for stepCount

@DrDiettrich

This makes buttonState known only inside loop(). Move all shared variable declarations to global scope, before all function definitions.

It was my understanding that static told the computer to retain the value of the variable between loop()s. This seems to be the case based on the simpler code that lights a single light with each button press seeming to work as expected.

@6v6gt

One big problem you have is that when you use the TX pin for something else you can't use Serial.print() for debugging. It may be better to temporarily free up some pins for debugging this section of code.

Unfortunately on this board the only pins available are RX/TX. I could however just focus on getting the button press to work with the RX pin in order to have the TX free. That said, I'm not really sure how to debug using TX -- and Serial.write() doesn't seem to magically write back to the Arduino IDE console over USB like I'd hoped.

@6v6gt

If you are using a counter (pressCount) to index an array, you should also include code to prevent an overflow.

Once this functions as expected, I will be using something like pressCount % arrayLengthVar to prevent overflow.

[budfriendguy]

Alright after giving it some thought it seems the problem might be that loop() is basically constantly in the midst of running a delay() in order to control whether the lights are on/off and that is causing the button press to be ignored (since there is always a delay() running...)

It seems that managing LED state with delays is the root issue here. Does that seem accurate?

If so, I need to devise some new way to accomplish LED state toggles -- something along the lines of this sketch, perhaps? https://www.arduino.cc/en/Tutorial/BuiltInExamples/BlinkWithoutDelay

[budfriendguy]

Alright so the code has been moved away from using delay() to time the light changes and is now using a class that stores the desired sequence and determines which action to take (if any) based on elapsed time.

Example code is below these comments.

Now the problem I seem to have is the class not retaining member variable values.

Using Serial.println() I've confirmed that LightSequence::calculateLoopDuration() does in fact correctly calculate the duration of the light sequence via Serial.println(this->loopDuration) but for some reason when LightSequence::getSequenceStepNumber() references this->loopDuration its value is zero.

In the same vein, it appears the step located at this->instructions[stepNumber] being accessed from LightSequence::getSequenceStepNumber() is also an empty value as during step zero it should have a value of HIGH (1) but contains a value of LOW (0) -- presumably because it is missing and not because it is actually set to 0.

Edit: Added Serial.println() to demonstrate where member values are correct/incorrect throughout execution.

#define RX_PIN 0
#define TX_PIN 1

#define RED_A 7
#define YELLOW_A 6
#define GREEN_A 5

#define RED_B 4
#define YELLOW_B 3
#define GREEN_B 2

const int LIGHTS[] = {
  RED_A, YELLOW_A, GREEN_A,
  RED_B, YELLOW_B, GREEN_B
};



/*
  LIGHT MAP
  
  [ ]      RED_A
    [ ]    YELLOW_A
      [ ]  GREEN_A
    [ ]    YELLOW_B
  [ ]      RED_B

  LIGHT SEQUENCE STRUCT
  [0] = lights
  [1] = level
  [2] = duration
*/



struct instruction {
  int lights[6];
  int level;
  int duration;
};



class LightSequence {
  public:
    LightSequence(const instruction (&instructions)[20]) {
      Serial.begin(9600);
      Serial.println(">>>>>>>> constructor()");
      Serial.println("RUNNING...");
      Serial.println(">>>>>>>>");
      Serial.end();

      this->setInstructions(instructions);
    } // LightSequence::LightSequence


    void lightsOut() {
      const size_t lightCount = sizeof(this->lights) / sizeof(*this->lights); 
      for (int i = 0; i < lightCount; i++) {
        pinMode(this->lights[i], OUTPUT);
        digitalWrite(this->lights[i], LOW);
      } // for lightCount
    } // LightSequence::lightsOut


    void run() {
      const int stepNumber = this->getSequenceStepNumber();

      if (this->currentStepNumber != stepNumber) {
        this->currentStepNumber = stepNumber;
        this->lightsOut();

        instruction instruction = this->instructions[stepNumber];

        size_t lightCount = sizeof(instruction.lights) / sizeof(*instruction.lights); 

        for (int j = 0; j < lightCount; j++) {
          // ensure we're not setting TX/RX output due to some mistake
          if (instruction.lights[j] > 1) {
            digitalWrite(
              instruction.lights[j],
              instruction.level
            );
          }
        } // for lightCount
      }
    } // LightSequence::run


    void setInstructions(const instruction (&instructions)[20]) {
      int stepCount = 0;
      for (int j = 0; j < 20; j++) {
        // const size_t lightCount = sizeof(instructions[j].lights) / sizeof(*instructions[j].lights);
        const int duration = instructions[j].duration;

        if (duration > 0) {
          this->instructions[j] = instructions[j];
          stepCount++;
        } else {
          this->instructions[j] = {};
        }
      } // for currentSeqLength

      this->currentStepNumber = -1;

      if (stepCount > 0) {
        this->stepCount = stepCount + 0;
      }

      Serial.begin(9600);
      Serial.println(">>>>>>>> setInstructions()");
      Serial.println("STEP COUNT");
      Serial.println(this->stepCount); // CORRECT
      Serial.println(">>>>>>>>");
      Serial.end();

      this->calculateLoopDuration(instructions);
    } // LightSequence::setInstructions


  private:
    int currentStepNumber = -1;
    instruction instructions[20];
    const int lights[6] = {
      RED_A, YELLOW_A, GREEN_A,
      RED_B, YELLOW_B, GREEN_B
    };
    int loopDuration = -1;
    int stepCount = -1;


    void calculateLoopDuration(const instruction (&instructions)[20]) {

      int loopDuration = 0;

      for (int i = 0; i < this->stepCount; i++) {
        loopDuration += instructions[i].duration;
      } // for stepCount

      if (loopDuration > 0) {
        this->loopDuration = loopDuration + 0;
      }

      Serial.begin(9600);
      Serial.println(">>>>>>>> calculateLoopDuration()");
      Serial.println("LOOP DURATION");
      Serial.println(this->loopDuration); // CORRECT
      Serial.println(">>>>>>>>");
      Serial.end();
    } // LightSequence::calculateLoopDuration


    int getSequenceStepNumber() {
      const unsigned long sequenceMillis = millis() % this->loopDuration;
      
      int stepNumber = 0;
      int millis = 0;
      while (millis < sequenceMillis && stepNumber < this->stepCount) {
        millis += this->instructions[stepNumber].duration;
        stepNumber++;
      } // while

      Serial.begin(9600);
      Serial.println(">>>>>>>> getSequenceStepNumber()");
      Serial.println("LOOP DURATION");
      Serial.println(this->loopDuration); // INCORRECT (was correct in calculateLoopDuration())
      Serial.println("STEP COUNT");
      Serial.println(this->stepCount); // INCORRECT (was correct in setInstructions())
      Serial.println("STEP NUMBER");
      Serial.println(stepNumber); // INCORRECT (due to lookDuration being incorrect)
      Serial.println("MILLIS");
      Serial.println(millis); // INCORRECT (due to loopDuration being incorrect)
      Serial.println(">>>>>>>>");
      Serial.end();

      return stepNumber;
    } // LightSequence::getSequenceStepNumber
}; // LightSequence


const instruction BLANK_SEQ[20] = {
  {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}
};
LightSequence LIGHT_SEQUENCE(BLANK_SEQ);

void setup() {
  delay(500);
  initializeButtons();
} // setup

void loop() {
  static bool hasRun = false;

  // cite: forum.arduino.cc/t/is-it-possible-to-add-a-push-button-switch-in-arduino-port-0-or-1/427968/15
  // default state for N.O switch with pullup
  static bool buttonHistory[3] = { true, true, true };
  static bool buttonState = true;
  static bool lastButtonState = true;
  // counts button presses
  static int pressCount = 0;


  if (hasRun == false) {
    setSequenceInstructions(LIGHT_SEQUENCE, pressCount);
    hasRun = true;
  } // if !hasRun


  // read button input state
  debounceReadButton(TX_PIN, buttonState, buttonHistory); 
  if (!lastButtonState && buttonState) {
    ++pressCount;
    setSequenceInstructions(LIGHT_SEQUENCE, pressCount);
  }
  lastButtonState = buttonState;

  // limit loop rate for sake of the debouncer
  delay(5000);

  LIGHT_SEQUENCE.run();
} // loop



void debounceReadButton(byte pin, bool &currentState, bool history[]) {
  history[2] = history[1];
  history[1] = history[0];
  history[0] = digitalRead(pin);
  
  // return new state if history aligns
  // otherwise leave state unchanged
  if (
    (history[0] == history[1]) &&
    (history[1] == history[2])
  ) {
    currentState = history[0];
  } // if
} // debounceReadButton



// START TRACKING BUTTON PRESSES
void initializeButtons() {
  pinMode(RX_PIN, INPUT_PULLUP);
  pinMode(TX_PIN, INPUT_PULLUP);
} // initializeButtons



// CHANGE THE CURRENT LIGHT SEQUENCE
// note: if newSeq is longer than currentSeq, newSeq will be truncated to fit
// TODO: this sucks because trying to pass the new sequence to another method always fucked up and passed nothing for whatever reason
void setSequenceInstructions(LightSequence sequence, int pressCount) {
  // 1. SEQ_BLINK
  // 2. SEQ_LEFT_TO_RIGHT_BLINK
  // 3. SEQ_LEFT_TO_RIGHT_FILL
  // 4. SEQ_TOP_TO_BOTTOM_BLINK
  // 5. SEQ_TOP_TO_BOTTOM_FILL

  const int numberOfSeq = 5;

  if (pressCount % numberOfSeq == 0) {
    const instruction SEQ_LEFT_TO_RIGHT_FILL[20] = {
      {
        { RED_A, RED_B },
        HIGH,
        1250
      }, {
        { YELLOW_A, YELLOW_B },
        HIGH,
        1250
      }, {
        { GREEN_A },
        HIGH,
        1875
      }, {
        { RED_A, RED_B, YELLOW_A, YELLOW_B, GREEN_A },
        LOW,
        1250
      },
      /* PADDING */ {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}
    }; // SEQ_LEFT_TO_RIGHT_FILL
    sequence.setInstructions(SEQ_LEFT_TO_RIGHT_FILL);

  } else if (pressCount % numberOfSeq == 1) {
    const instruction SEQ_LEFT_TO_RIGHT_BLINK[20] = {
      {
        { RED_A, RED_B },
        HIGH,
        1000
      }, {
        { RED_A, RED_B },
        LOW,
        100
      }, {
        { YELLOW_A, YELLOW_B },
        HIGH,
        1000
      }, {
        { YELLOW_A, YELLOW_B },
        LOW,
        100
      }, {
        { GREEN_A },
        HIGH,
        1000
      }, {
        { GREEN_A },
        LOW,
        100
      },    
      /* PADDING */ {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}
    }; // SEQ_LEFT_TO_RIGHT_BLINK
    sequence.setInstructions(SEQ_LEFT_TO_RIGHT_BLINK);

  } else if (pressCount % numberOfSeq == 2) {
    const instruction SEQ_BLINK[20] = {
      {
        { RED_A, RED_B, YELLOW_A, YELLOW_B, GREEN_A },
        HIGH,
        1000
      }, {
        { RED_A, RED_B, YELLOW_A, YELLOW_B, GREEN_A },
        LOW,
        1000
      },
      /* PADDING */ {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}
    }; // SEQ_BLINK
    sequence.setInstructions(SEQ_BLINK);

  } else if (pressCount % numberOfSeq == 3) {
    const instruction SEQ_TOP_TO_BOTTOM_BLINK[20] = {
      {
        { RED_A },
        HIGH,
        1000
      }, {
        { RED_A },
        LOW,
        100
      }, {
        { YELLOW_A },
        HIGH,
        1000
      }, {
        { YELLOW_A },
        LOW,
        100
      }, {
        { GREEN_A },
        HIGH,
        1000
      }, {
        { GREEN_A },
        LOW,
        100
      }, {
        { YELLOW_B },
        HIGH,
        1000
      }, {
        { YELLOW_B },
        LOW,
        100
      }, {
        { RED_B },
        HIGH,
        1000
      }, {
        { RED_B },
        LOW,
        100
      },
      /* PADDING */ {}, {}, {}, {}, {}, {}, {}, {}, {}, {}
    }; // SEQ_TOP_TO_BOTTOM_BLINK
    sequence.setInstructions(SEQ_TOP_TO_BOTTOM_BLINK);

  } else if (pressCount % numberOfSeq == 4) {
    const instruction SEQ_TOP_TO_BOTTOM_FILL[20] = {
      {
        { RED_A },
        HIGH,
        1000
      }, {
        { YELLOW_A },
        HIGH,
        1000
      }, {
        { GREEN_A },
        HIGH,
        1000
      }, {
        { YELLOW_B },
        HIGH,
        1000
      }, {
        { RED_B },
        HIGH,
        1000
      }, {
        { RED_A, RED_B, YELLOW_A, YELLOW_B, GREEN_A },
        LOW,
        1000
      },
      /* PADDING */ {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}
    }; // SEQ_TOP_TO_BOTTOM_FILL
    sequence.setInstructions(SEQ_TOP_TO_BOTTOM_FILL);
  }
} // setSequenceInstructions

Edit: Including console log here as well as the post below where it was initially shared:

>>>>>>>> constructor()
RUNNING...
>>>>>>>>
>>>>>>>> setInstructions()
STEP COUNT
-1
>>>>>>>>
>>>>>>>> calculateLoopDuration()
LOOP DURATION
-1
>>>>>>>>
>>>>>>>> setInstructions()
STEP COUNT
4
>>>>>>>>
>>>>>>>> calculateLoopDuration()
LOOP DURATION
5625
>>>>>>>>
>>>>>>>> getSequenceStepNumber()
LOOP DURATION
-1
STEP COUNT
-1
STEP NUMBER
0
MILLIS
0
>>>>>>>>
>>>>>>>> getSequenceStepNumber()
LOOP DURATION
-1
STEP COUNT
-1
STEP NUMBER
0
MILLIS
0
>>>>>>>>
>>>>>>>> getSequenceStepNumber()
LOOP DURATION
-1
STEP COUNT
-1
STEP NUMBER
0
MILLIS
0
>>>>>>>>

[Grumpy_Mike]

@budfriendguy

That said, I'm not really sure how to debug using TX -- and Serial.write() doesn't seem to magically write back to the Arduino IDE console over USB like I'd hoped.

True, but Serial.print will.

Serial.write sends absolute values of the variable, where as Serial.print will turn a variable value into ASCII that can be read in the serial monitor.

[budfriendguy]

@Grumpy_Mike

I've edited the code sample in my latest post here to include Serial.println() demonstrating where values are correct/incorrect: Having trouble with static variable in loop() "not changing" - #7 by budfriendguy

this->loopDuration and this->stepCount are both correct in the functions where they are initially calculated/set; But by the time they are referenced later by getSequenceStepNumber() both have reverted to their initial values.

[Grumpy_Mike]

@budfriendguy

// counts button presses
  static int pressCount = 0;

In the loop function and

@budfriendguy

int stepCount = -1;

In the private section of your class.

Two variables of different types but with the same name. So it is not difficult to see why there is a confusion about what instance of this variable is being used at any one time.

[budfriendguy]

@Grumpy_Mike

Two variables of different types but with the same name. So it is not difficult to see why there is a confusion about what instance of this variable is being used at any one time.

static int pressCount is stored in the loop() function and passed as an argument to setSequenceInstructions() but not stored on the class.

int stepCount is stored on the class and is calculated within LightSequence::setInstructions() to determine how many legitimate steps there are in the sequence, since sequences may be padded with "empty" steps to meet the array length requirement.

What's at issue here is that within the only method changing the value of a member such as this->stepCount the value of this->stepCount is printed correctly, but for a reason I am not aware of when a subsequent method is called and references the value of this->stepCount it has reverted to its initial value; This happens without the only method that changes its value being called another time (which I have confirmed by Serial.println()-ing each time it runs.

Edit: Here is a copy of the log showing what happens:

>>>>>>>> constructor()
RUNNING...
>>>>>>>>
>>>>>>>> setInstructions()
STEP COUNT
-1
>>>>>>>>
>>>>>>>> calculateLoopDuration()
LOOP DURATION
-1
>>>>>>>>
>>>>>>>> setInstructions()
STEP COUNT
4
>>>>>>>>
>>>>>>>> calculateLoopDuration()
LOOP DURATION
5625
>>>>>>>>
>>>>>>>> getSequenceStepNumber()
LOOP DURATION
-1
STEP COUNT
-1
STEP NUMBER
0
MILLIS
0
>>>>>>>>
>>>>>>>> getSequenceStepNumber()
LOOP DURATION
-1
STEP COUNT
-1
STEP NUMBER
0
MILLIS
0
>>>>>>>>
>>>>>>>> getSequenceStepNumber()
LOOP DURATION
-1
STEP COUNT
-1
STEP NUMBER
0
MILLIS
0
>>>>>>>>

[gfvalvo]

@budfriendguy

this->setInstructions(instructions);

BTW, the 'this->' notation is superfluous. It would only be needed if your class inherits from another class AND the parent class had a variable / function of the same name. In your case, it only adds clutter.

[budfriendguy]

@gfvalvo

BTW, the 'this->' notation is superfluous. It would only be needed if your class inherits from another class AND the parent class had a variable / function of the same name. In your case, it only adds clutter.

Ah thank you. I've removed it. The compiler was complaining earlier for whatever reason but doesn't have a problem with omitting it now.

Edit: Actually it seems that calculations fail (show as 0 where they were previously [temporarily] correct) without this-> present...

[budfriendguy]

Quick update to share that the problem has been diagnosed and resolved!

It turns out that using the syntax of MyClass MY_VAR(constructorArg); would result in the destructor being called after the constructor had run.

Changing to the syntax MyClass *MY_VAR = new MyClass(constructorArg); has resolved the issue.

Fully functioning code below:

#define RX_PIN 0
#define TX_PIN 1

#define RED_A 7
#define YELLOW_A 6
#define GREEN_A 5

#define RED_B 4
#define YELLOW_B 3
#define GREEN_B 2

const int LIGHTS[] = {
  RED_A, YELLOW_A, GREEN_A,
  RED_B, YELLOW_B, GREEN_B
};



/*
  LIGHT MAP
  
  [ ]      RED_A
    [ ]    YELLOW_A
      [ ]  GREEN_A
    [ ]    YELLOW_B
  [ ]      RED_B

  LIGHT SEQUENCE STRUCT
  [0] = lights
  [1] = level
  [2] = duration
*/



struct instruction {
  int lights[6];
  int level;
  int duration;
};



class LightSequence {
  public:
    LightSequence(int instructionIndex) {
      setInstructions(instructionIndex);
    } // LightSequence::LightSequence


    // ~LightSequence() {
    // } // LightSequence::~LightSequence


    void lightsOut() {
      const size_t lightCount = sizeof(lights) / sizeof(*lights); 
      for (int i = 0; i < lightCount; i++) {
        pinMode(lights[i], OUTPUT);
        digitalWrite(lights[i], LOW);
      } // for lightCount
    } // LightSequence::lightsOut


    void run() {
      const int stepNumber = getSequenceStepNumber();

      if (currentStepNumber != stepNumber) {
        currentStepNumber = stepNumber;
        lightsOut();

        const instruction instruction = instructions[stepNumber];

        size_t lightCount = sizeof(instruction.lights) / sizeof(*instruction.lights); 

        for (int j = 0; j < lightCount; j++) {
          // ensure we're not setting TX/RX output due to some mistake
          if (instruction.lights[j] > 1) {
            digitalWrite(
              instruction.lights[j],
              instruction.level
            );
          }
        } // for lightCount
      }
    } // LightSequence::run

    // CHANGE THE CURRENT LIGHT SEQUENCE
    // note: if newSeq is longer than currentSeq, newSeq will be truncated to fit
    void setInstructions(int pressCount) {
      // 1. SEQ_BLINK
      // 2. SEQ_LEFT_TO_RIGHT_BLINK
      // 3. SEQ_LEFT_TO_RIGHT_FILL
      // 4. SEQ_TOP_TO_BOTTOM_BLINK
      // 5. SEQ_TOP_TO_BOTTOM_FILL

      const int numberOfSeq = 5;

      if (pressCount % numberOfSeq == 0) {
        const instruction SEQ_LEFT_TO_RIGHT_FILL[20] = {
          {
            { RED_A, RED_B },
            HIGH,
            1250
          }, {
            { YELLOW_A, YELLOW_B },
            HIGH,
            1250
          }, {
            { GREEN_A },
            HIGH,
            1875
          }, {
            { RED_A, RED_B, YELLOW_A, YELLOW_B, GREEN_A },
            LOW,
            1250
          },
          /* PADDING */ {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}
        }; // SEQ_LEFT_TO_RIGHT_FILL
        writeInstructions(&SEQ_LEFT_TO_RIGHT_FILL);

      } else if (pressCount % numberOfSeq == 1) {
        const instruction SEQ_LEFT_TO_RIGHT_BLINK[20] = {
          {
            { RED_A, RED_B },
            HIGH,
            1000
          }, {
            { RED_A, RED_B },
            LOW,
            100
          }, {
            { YELLOW_A, YELLOW_B },
            HIGH,
            1000
          }, {
            { YELLOW_A, YELLOW_B },
            LOW,
            100
          }, {
            { GREEN_A },
            HIGH,
            1000
          }, {
            { GREEN_A },
            LOW,
            100
          },    
          /* PADDING */ {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}
        }; // SEQ_LEFT_TO_RIGHT_BLINK
        writeInstructions(&SEQ_LEFT_TO_RIGHT_BLINK);

      } else if (pressCount % numberOfSeq == 2) {
        const instruction SEQ_BLINK[20] = {
          {
            { RED_A, RED_B, YELLOW_A, YELLOW_B, GREEN_A },
            HIGH,
            1000
          }, {
            { RED_A, RED_B, YELLOW_A, YELLOW_B, GREEN_A },
            LOW,
            1000
          },
          /* PADDING */ {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}
        }; // SEQ_BLINK
        writeInstructions(&SEQ_BLINK);

      } else if (pressCount % numberOfSeq == 3) {
        const instruction SEQ_TOP_TO_BOTTOM_BLINK[20] = {
          {
            { RED_A },
            HIGH,
            1000
          }, {
            { RED_A },
            LOW,
            100
          }, {
            { YELLOW_A },
            HIGH,
            1000
          }, {
            { YELLOW_A },
            LOW,
            100
          }, {
            { GREEN_A },
            HIGH,
            1000
          }, {
            { GREEN_A },
            LOW,
            100
          }, {
            { YELLOW_B },
            HIGH,
            1000
          }, {
            { YELLOW_B },
            LOW,
            100
          }, {
            { RED_B },
            HIGH,
            1000
          }, {
            { RED_B },
            LOW,
            100
          },
          /* PADDING */ {}, {}, {}, {}, {}, {}, {}, {}, {}, {}
        }; // SEQ_TOP_TO_BOTTOM_BLINK
        writeInstructions(&SEQ_TOP_TO_BOTTOM_BLINK);

      } else if (pressCount % numberOfSeq == 4) {
        const instruction SEQ_TOP_TO_BOTTOM_FILL[20] = {
          {
            { RED_A },
            HIGH,
            1000
          }, {
            { YELLOW_A },
            HIGH,
            1000
          }, {
            { GREEN_A },
            HIGH,
            1000
          }, {
            { YELLOW_B },
            HIGH,
            1000
          }, {
            { RED_B },
            HIGH,
            1000
          }, {
            { RED_A, RED_B, YELLOW_A, YELLOW_B, GREEN_A },
            LOW,
            1000
          },
          /* PADDING */ {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}
        }; // SEQ_TOP_TO_BOTTOM_FILL
        writeInstructions(&SEQ_TOP_TO_BOTTOM_FILL);
      }
    } // LightSequence::setInstructions
    
    
    void setSpeed(int pressCount) {

      switch (pressCount % 6) {
        case 0: speedMultiplier = 1.0; break;
        case 1: speedMultiplier = 0.75; break;
        case 2: speedMultiplier = 0.5; break;
        case 3: speedMultiplier = 0.25; break;
        case 4: speedMultiplier = 2.0; break;
        case 5: speedMultiplier = 1.5; break;
      }
    } // LightSequence::setInstructions


    void writeInstructions(const instruction (*__instructions)[20]) {
      int __stepCount = 0;
      for (int j = 0; j < 20; j++) {
        const int duration = (*__instructions)[j].duration;

        if (duration > 0) {
          instructions[j] = (*__instructions)[j];
          __stepCount++;
        } else {
          instructions[j] = {};
        }
      } // for currentSeqLength

      currentStepNumber = -1;

      if (__stepCount > 0) {
        stepCount = __stepCount;
      }

      calculateLoopDuration(__instructions);
    } // LightSequence::setInstructions


  private:
    int currentStepNumber = -1;
    instruction instructions[20];
    const int lights[6] = {
      RED_A, YELLOW_A, GREEN_A,
      RED_B, YELLOW_B, GREEN_B
    };
    int loopDuration = -1;
    double speedMultiplier = 1.0;
    int stepCount = -1;


    void calculateLoopDuration(const instruction (*__instructions)[20]) {

      int __loopDuration = 0;

      for (int i = 0; i < stepCount; i++) {
        __loopDuration += (*__instructions)[i].duration;
      } // for stepCount

      if (__loopDuration > 0) {
        loopDuration = __loopDuration;
      }
    } // LightSequence::calculateLoopDuration


    int getSequenceStepNumber() {
      const unsigned long sequenceMillis = millis() % (int)(loopDuration * speedMultiplier);
      
      int stepNumber = 0;
      int millis = 0;
      while (millis < sequenceMillis && stepNumber < stepCount) {
        millis += (int)(instructions[stepNumber].duration * speedMultiplier);
        stepNumber++;
      } // while

      return stepNumber - 1;
    } // LightSequence::getSequenceStepNumber
}; // LightSequence


void setup() {
  delay(500);
  initializeButtons();
} // setup

LightSequence *LIGHT_SEQUENCE = new LightSequence(0);

void loop() {

  static bool hasRun = false;

  // cite: forum.arduino.cc/t/is-it-possible-to-add-a-push-button-switch-in-arduino-port-0-or-1/427968/15
  // default state for N.O switch with pullup
  static bool txHistory[3] = { true, true, true };
  static bool txState = true;
  static bool txLastState = true;
  static int txPressCount = 0;
  // default state for N.O switch with pullup
  static bool rxHistory[3] = { true, true, true };
  static bool rxState = true;
  static bool rxLastState = true;
  static int rxPressCount = 0;


  if (hasRun == false) {
    LIGHT_SEQUENCE->setInstructions(txPressCount);
    hasRun = true;
  } // if !hasRun


  // read button input
  debounceReadButton(TX_PIN, txState, txHistory); 
  if (!txLastState && txState) {
    ++txPressCount;
    LIGHT_SEQUENCE->setInstructions(txPressCount);
  }
  txLastState = txState;

  // read button input
  debounceReadButton(RX_PIN, rxState, rxHistory); 
  if (!rxLastState && rxState) {
    ++rxPressCount;
    LIGHT_SEQUENCE->setSpeed(rxPressCount);
  }
  rxLastState = rxState;

  // limit loop rate for sake of the debouncer
  delay(20);

  LIGHT_SEQUENCE->run();
} // loop



void debounceReadButton(byte pin, bool &currentState, bool history[]) {
  history[2] = history[1];
  history[1] = history[0];
  history[0] = digitalRead(pin);
  
  // return new state if history aligns
  // otherwise leave state unchanged
  if (
    (history[0] == history[1]) &&
    (history[1] == history[2])
  ) {
    currentState = history[0];
  } // if
} // debounceReadButton



// START TRACKING BUTTON PRESSES
void initializeButtons() {
  pinMode(RX_PIN, INPUT_PULLUP);
  pinMode(TX_PIN, INPUT_PULLUP);
} // initializeButtons