Static variable changing in Switch/Case loop

Hello -
Need a little help. Have been programming for many years, and trying to write a simple traffic light sequencer.
I have a static unsigned int called OperationState which is toggled by two input buttons. This part of the code, and the setting to 1 in Setup() work just fine.
OperationState is being set back to 1 somewhere in the Switch/Case loop and I just cannot see how or where. I thought that as a static it was a global variable?? Have used Serial.print to do some debugging. Any help much appreciated ....
Here is the code:

/*
**************************************
Traffic Light software - version 4
FINAL VERSION
uses the Case statement and millis for
timing functions and not delay()
Transitions between Case statements have
unused transitions removed
The stopping/restarting of the light
sequence button actions are added
**************************************
*/

//Define the traffic light output pin numbers
//Traffic Light A
#define TLARED 2
#define TLAAMBER 3
#define TLAGREEN 4
//Traffic Light B
#define TLBRED 5
#define TLBAMBER 6
#define TLBGREEN 7

//Define the input push button pin numbers
#define STOPSEQUENCE 8
#define RESET_RESTART 9

//Define the state of operation from pushbutton inputs
//Set to 1 if in normal operation, set to 2 if in special operation
static unsigned int OperationState;

//Define the sequence state of the lights - 1 through to 8
static unsigned int state;

//Define the timing variables
static unsigned long previousMillis;  // stored previous value of millis
static unsigned long currentMillis;   // current value of millis
static unsigned long SEQUENCEDELAY1;  //30 second delay timer
static unsigned long SEQUENCEDELAY2;  // 2 second delay timer
static unsigned long SEQUENCEDELAY3;  // 5 second reduced special mode delay timer

void setup() {
  // code in the section of the program runs only once at startup:

  // initialize the output pins:
  pinMode(TLARED, OUTPUT);
  pinMode(TLAAMBER, OUTPUT);
  pinMode(TLAGREEN, OUTPUT);
  pinMode(TLBRED, OUTPUT);
  pinMode(TLBAMBER, OUTPUT);
  pinMode(TLBGREEN, OUTPUT);

  // initialize the input pins:
  pinMode(STOPSEQUENCE, INPUT);
  pinMode(RESET_RESTART, INPUT);

  // initialise the traffic light sequence state variable
  state = 1;

  // initialise the operation state variable
  OperationState = 1;

  //initialise the time delay variables
  previousMillis = 0;     // set previousMillis to zero
  currentMillis = 0;      // set currentMillis to zero
  SEQUENCEDELAY1 = 5000;  // set SEQUENCEDELAY1 to 30 seconds
  SEQUENCEDELAY2 = 2000;  // set SEQUENCEDELAY2 to 2 seconds
  SEQUENCEDELAY3 = 3000;  // set SEQUENCEDELAY3 to 5 seconds

  Serial.begin(9600);
}

void loop() {
  // traffic light sequence - with Case statements - loops runs continuously:


  //check to see which operating mode the lights should be in
  if (digitalRead(STOPSEQUENCE) == 0) {
    //  // stopping of sequence switch pressed - special mode
    // need to seqence to red light in both directions safely
    OperationState = 2;
  }
  if (digitalRead(RESET_RESTART) == 0) {
    // reset/restart of sequence switch pressed
    // resume normal operation
    OperationState = 1;
  }

  Serial.print(OperationState);
  Serial.print("\n");

  // gets the current time value
  currentMillis = millis();

  switch (state) {
    case 1:
      //sequence step 1
      digitalWrite(TLARED, HIGH);
      digitalWrite(TLAAMBER, LOW);
      digitalWrite(TLAGREEN, LOW);
      digitalWrite(TLBRED, HIGH);
      digitalWrite(TLBAMBER, LOW);
      digitalWrite(TLBGREEN, LOW);

      if (OperationState = 1)
      // working in normal sequence mode
      {
        //check the time delay to see if it has expired
        if (currentMillis - previousMillis >= SEQUENCEDELAY1) {
          state = state + 1;
          previousMillis = currentMillis;
        }
      }
      break;

    case 2:
      //sequence step 2
      digitalWrite(TLBAMBER, HIGH);

      if (OperationState = 1)
      // working in normal sequence mode
      {
        //check the time delay to see if it has expired
        if (currentMillis - previousMillis >= SEQUENCEDELAY2) {
          state = state + 1;
          previousMillis = currentMillis;
        }
      }
      break;

    case 3:
      //sequence step 3
      digitalWrite(TLBRED, LOW);
      digitalWrite(TLBAMBER, LOW);
      digitalWrite(TLBGREEN, HIGH);

      if (OperationState = 1)
      // working in normal sequence mode
      {
        //check the time delay to see if it has expired
        if (currentMillis - previousMillis >= SEQUENCEDELAY1) {
          state = state + 1;
          previousMillis = currentMillis;
        }
      }
      break;

    case 4:
      //sequence step 4
      digitalWrite(TLBAMBER, HIGH);
      digitalWrite(TLBGREEN, LOW);

      if (OperationState = 1)
      // working in normal sequence mode
      {
        //check the time delay to see if it has expired
        if (currentMillis - previousMillis >= SEQUENCEDELAY2) {
          state = state + 1;
          previousMillis = currentMillis;
        }
      }
      break;

    case 5:
      //sequence step 5
      digitalWrite(TLBRED, HIGH);
      digitalWrite(TLBAMBER, LOW);

      if (OperationState = 1)
      // working in normal sequence mode
      {
        //check the time delay to see if it has expired
        if (currentMillis - previousMillis >= SEQUENCEDELAY1) {
          state = state + 1;
          previousMillis = currentMillis;
        }
      }
      break;

    case 6:
      //sequence step 6
      digitalWrite(TLAAMBER, HIGH);

      if (OperationState = 1)
      // working in normal sequence mode
      {
        //check the time delay to see if it has expired
        if (currentMillis - previousMillis >= SEQUENCEDELAY2) {
          state = state + 1;
          previousMillis = currentMillis;
        }
      }
      break;

    case 7:
      //sequence step 7
      digitalWrite(TLARED, LOW);
      digitalWrite(TLAAMBER, LOW);
      digitalWrite(TLAGREEN, HIGH);

      if (OperationState = 1)
      // working in normal sequence mode
      {
        //check the time delay to see if it has expired
        if (currentMillis - previousMillis >= SEQUENCEDELAY1) {
          state = state + 1;
          previousMillis = currentMillis;
        }
      }
      break;

    case 8:
      //sequence step 8
      digitalWrite(TLAAMBER, HIGH);
      digitalWrite(TLAGREEN, LOW);

      if (OperationState = 1)
      // working in normal sequence mode
      {
        //check the time delay to see if it has expired
        if (currentMillis - previousMillis >= SEQUENCEDELAY1) {
          state = 1;  //go back to state 1 - start of sequence
          previousMillis = currentMillis;
        }
      }
      break;
  }
}

First of all: compliments for your very neat sketch!
Second:

Classic error... (== vs. =)
(fix all of them...)

1 Like

And if you turn on warnings in preferences it will tell you about them.

2 Likes

Why are you making all those static anyway? They're already global so their lifetime will last the entire program. The only thing making them static does is restrict their access to the current source file and make them unavailable to other .cpp files via the extern keyword. But, you don't mention any other .cpp files in your post.

Your static guys are declared outside of all your functions. So they are already global.

-jim lee

Ok, you type faster..

If instead of

//Define the state of operation from pushbutton inputs
//Set to 1 if in normal operation, set to 2 if in special operation
static unsigned int OperationState;

you used a boolean variable

//Define the state of operation from pushbutton inputs
// true is normal operation false means special
static bool isNormalOperation = true;

All those tests would read

  if (isNormalOperation) {

but name it something you like, I am no good at naming things.

Neatly sidesteps the issue, and sometimes makes code easier to read. Here, for example, I never have to know, or remember or look again to see what means 1 versus whatever other values the unsigned int might have taken on.

a7

I built your project in the wokwi simulator after fixing the = vs == thing.

I added some printing; you can change what it prints to be more meaningful the one, two, three and so forth.

I sped it up. I old.

You might like the simulator. One thing for sure is that you can't burn anything out and don't have to worry about broken components. Or buy them, for that matter.

a7

Thank you everyone for all your help and suggestions. It has been a while since I have written a sketch and had forgotten about using == (which does the test) instead of = (which reassigns the value!).
The suggestion for using a bool is a great one, and something I did consider.
Also, I don't really need static as I only have the one sketch file.
I have taken a look at Wokwi and will start using this. I had built the circuit on an Uno and breadboard - the Wokwi simulator looks excellent.
Thank you all once again!

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.