Random millis

Hi All ,
I am playing around with adding a random LED flicker in a switch case . I have added a random flickering LED effect in Case 3 and I am happy with the effect . My problem happens when I push the button again to go to Case 4 , the LEDs continue doing their effect when I would like them to stop . I think the problem is the delay in Case 3 , how would I replace the delay with a random millis ? Would that stop the effect in Case 4 with next button push ?
Thanks .

[code]
// Experiment with random in switch case


// LEDS
int led1 = 11;      // PWM Dim
int led2 = 10;      // PWM Dim
int led3 = 9;       // PWM Dim
int led4 = 6;       // PWM Dim
int led5 = 5;       // PWM Dim

//Button
int button1Pin = 7;//
int state = 0 ;
int old = 0 ;
int buttonPoll = 0 ;

void setup() {

  pinMode(button1Pin, INPUT_PULLUP);

  pinMode(led1, OUTPUT);
  pinMode(led2, OUTPUT);
  pinMode(led3, OUTPUT);
  pinMode(led4, OUTPUT);
  pinMode(led5, OUTPUT);
}

void loop() {

  // read the state of the pushbutton value:
  buttonPoll = digitalRead(button1Pin);
  if (buttonPoll == 1) {
    delay (50);
    buttonPoll = digitalRead(button1Pin);
    if (buttonPoll == 0) {
      state = old + 1 ;
    }
  }
  else {
    delay (100) ;
  }
  switch (state) {
    case 1 :
      analogWrite(led4, 200);
      old = state ;
      break;
    case 2 :
      analogWrite(led5, 200);
      analogWrite(led4, 0);
      old = state ;
      break;
    case 3 :
      analogWrite(led4, 200);
      analogWrite(led5, 200);

      analogWrite(led1, random(120) + 135);
      analogWrite(led2, random(120) + 135);
      analogWrite(led3, random(120) + 135);
      delay(random(100));
      old = 0 ;
      break;
    case 4 :
      analogWrite(led4, 0);
      analogWrite(led5, 0);
      old = 0 ;
      break;
    default :
      analogWrite(led1, 0);
      analogWrite(led2, 0);
      analogWrite(led3, 0);
      analogWrite(led4, 0);
      analogWrite(led5, 0);
      old = 0 ;
      break;
      //}
      //}
  }
}

[/code]

Use Serial.print to tell about old, state... Theb You can see what's happening, or not happening in the code. That's much faster than asking other people to dry debug Your code

the LEDs continue doing their effect when I would like them to stop

Where does your program instruct them to "stop"?

If using a Uno or Mega:
With this, buttonPoll = digitalRead(button1Pin);, being the only moment that the button is checked for being pressed and with a possibility of over 150mS delay you might want to look at practicing with the millis delay thigny and the millis multiple things thing do, Demonstration code for several things at the same time, which you passed over to make your post.

Once you got the millis thing under your belt, should not take long, doing delay(random(100)); with millis() will be obvious.


Things can be quite a bit different with a STM32, Due, andESP32 with freeRTOS.

Why is there a variable 'old', I don't get it. Can you just use the variable 'state' for everything ?

Suppose you want to do things before or after a state, then you can add an extra state for that. There are called "transitional states" here: The Finite State Machine | Majenko Technologies.

When you use a 'enum' for the states, then it will become more readable. See my millis_and_finite_state_machine.ino.

The button goes through the states, but then your states depends on the increment of the variable 'state'. Those are different things and I want to pull them apart. Suppose you have 100 states, and the button should only activate 5 states. Then you need a list with those 5 states and you should go through that list instead of directly going through the states.

In case 1 and 2 you set case = old; in case 3 and 4 you set case = 0. That may be your problem (I don't understand really what you're trying to do here but that's an oddity).

Why are there commented-out closing curly braces? Doesn't make sense. Just remove those!

Thanks everyone for the replies and suggestions .
I have replaced the delay with millis for the LED random flicker effect , works great . Still working on replacing the delay in the button poll .
I have removed the commented out braces and fixed the case errors as per wvmarle's observation and that allows the button push to go through the cases .
Now I have another question , how would I get the default case to start over again at case 1 ?

Here is the updated code :

[code]

//Take 1 , experiment with random in switch case
//Take 2 , removed commented out closing curly braces , fixed old = state in cases with error
//Take 3 , replaced delay with millis for random flicker LEDs


// LEDS
int led1 = 11;      // PWM Dim
int led2 = 10;      // PWM Dim
int led3 = 9;       // PWM Dim
int led4 = 6;       // PWM Dim
int led5 = 5;       // PWM Dim

//Button
int button1Pin = 7;//
int state = 0 ;
int old = 0 ;
int buttonPoll = 0 ;

unsigned long previousMillis = 0;        // last time LED flickered
const long interval = 100;           // time between LED flicker

void setup() {

  pinMode(button1Pin, INPUT_PULLUP);

  pinMode(led1, OUTPUT);
  pinMode(led2, OUTPUT);
  pinMode(led3, OUTPUT);
  pinMode(led4, OUTPUT);
  pinMode(led5, OUTPUT);
}

void loop() {

  unsigned long currentMillis = millis();

  // read the state of the pushbutton value:
  buttonPoll = digitalRead(button1Pin);
  if (buttonPoll == 1) {
    delay (50);
    buttonPoll = digitalRead(button1Pin);
    if (buttonPoll == 0) {
      state = old + 1 ;
    }
  }
  else {
    delay (100) ;
  }
  switch (state) {
    case 1 :
      analogWrite(led4, 200);
      old = state ;
      break;
    case 2 :
      analogWrite(led5, 200);
      analogWrite(led4, 0);
      old = state ;
      break;
    case 3 :
      analogWrite(led4, 0);
      analogWrite(led5, 0);

      if (currentMillis - previousMillis >= interval) {
        analogWrite(led1, random(120) + 135);
        analogWrite(led2, random(120) + 135);
        analogWrite(led3, random(120) + 135);
        previousMillis = currentMillis;
        old = state ;
        break;
      case 4 :
        analogWrite(led5, 200);
        analogWrite(led4, 200);
        analogWrite(led1, 0);
        analogWrite(led2, 0);
        analogWrite(led3, 0);
        old = state ;
        break;
      default :
        analogWrite(led1, 0);
        analogWrite(led2, 0);
        analogWrite(led3, 0);
        analogWrite(led4, 0);
        analogWrite(led5, 0);
        old = state ;
        break;
      }
  }
}

[/code]

state =1;

The switch-case is the main structure for your sketch.
But case 1, 2 and 3 are not at the level as 4 and the default.

Does this even compile? There's a rather glaring error in the if statement of case 3.

Idahowalker - adding state=1; in default does bring me back to start , thanks
Koepel - sorry , I don’t understand what you mean
wvmarle - it compiles and runs when uploaded to the arduino . Could you tell me the error in the if statement so I could try to fix it ? Thanks for your help .

Could you tell me the error in the if statement so I could try to fix it ?

Take a close look at the curly braces.

Turn on compiler warnings, if they are off.

  if (currentMillis - previousMillis >= interval) 
{
        analogWrite(led1, random(120) + 135);
        analogWrite(led2, random(120) + 135);
        analogWrite(led3, random(120) + 135);
        previousMillis = currentMillis;
        old = state ;
        break;
      case 4 :
        analogWrite(led5, 200);
        analogWrite(led4, 200);
        analogWrite(led1, 0);
        analogWrite(led2, 0);
        analogWrite(led3, 0);
        old = state ;
        break;
      default :
        analogWrite(led1, 0);
        analogWrite(led2, 0);
        analogWrite(led3, 0);
        analogWrite(led4, 0);
        analogWrite(led5, 0);
        old = state ;
        break;
}

Yuppers an issue for case 4 and default being tied to the if statement. Good catch @jremington.

Okay , is this my error :
I have my closing brace for the if statement at the end of the sketch and it should be inside of case 3 ?

You are writing the program. What should the if statement do?

That code compiles without errors or warnings on IDE 1.8.12, which in my view is an egregious error. The switch case statement could in principle jump into the middle of an if block.

The switch-case is the main structure of your sketch.

void loop()
{
  unsigned long currentMillis = millis();

  // get the buttons, sensors, do debounce, and so on.
  ...

  // Process the information and make decisions in the switch-case
  switch (state)
  {
    case 1:
      ...
      break;
    case 2:
      ...
      break;
    case 3:
      ...
      break;
    case 4:
      ...
      break;
    default:
      ...
      break;
  }

  // Other code for in the loop()
  ...
}

You have something else. You don't have that switch-case anymore.

jremington:
You are writing the program. What should the if statement do?

That code compiles without errors or warnings on IDE 1.8.12, which in my view is an egregious error. The switch case statement could in principle jump into the middle of an if block.

Okay , had some time today to get back to this code . Not sure if the egregious error is my code , the compiler didn't catch it or most likely both .
I have added state = 1 ; in the default case to start over .
In the if statement I have moved the "previousMillis = currentMillis ;" line above the random LED task and moved the closing curly brace inside of case 3 . Am I close to what is wrong with the if statement ? Am I looking at the wrong thing ?
Still new to the Arduino stuff , sorry if I am missing something very obvious .
This code compiles and runs on the arduino nano .
Here is the code :

[code]

//Take 1 , experiment with random in switch case
//Take 2 , removed commented out closing curly braces , fixed old = state in cases with error
//Take 3 , replaced delay with millis for random flicker LEDs
//Take 4 , added state = 1 in default case to start over
//Take 5 , updated / hopefully repaired improper if statement 


// LEDS
int led1 = 11;      // PWM Dim
int led2 = 10;      // PWM Dim
int led3 = 9;       // PWM Dim
int led4 = 6;       // PWM Dim
int led5 = 5;       // PWM Dim

//Button
int button1Pin = 7;//
int state = 0 ;
int old = 0 ;
int buttonPoll = 0 ;

unsigned long previousMillis = 0;        // last time LED flickered
const long interval = 100;           // time between LED flicker

void setup() {

  pinMode(button1Pin, INPUT_PULLUP);

  pinMode(led1, OUTPUT);
  pinMode(led2, OUTPUT);
  pinMode(led3, OUTPUT);
  pinMode(led4, OUTPUT);
  pinMode(led5, OUTPUT);
}

void loop() {

  unsigned long currentMillis = millis();

  // read the state of the pushbutton value:
  buttonPoll = digitalRead(button1Pin);
  if (buttonPoll == 1) {
    delay (50);
    buttonPoll = digitalRead(button1Pin);
    if (buttonPoll == 0) {
      state = old + 1 ;
    }
  }
  else {
    delay (100) ;
  }
  switch (state) {
    case 1 :
      analogWrite(led4, 200);
      old = state ;
      break;
    case 2 :
      analogWrite(led5, 200);
      analogWrite(led4, 0);
      old = state ;
      break;
    case 3 :
      analogWrite(led4, 0);
      analogWrite(led5, 0);

      if (currentMillis - previousMillis >= interval) {
        previousMillis = currentMillis;
        analogWrite(led1, random(120) + 135);
        analogWrite(led2, random(120) + 135);
        analogWrite(led3, random(120) + 135);
      }
      old = state ;
      break;
    case 4 :
      analogWrite(led5, 200);
      analogWrite(led4, 200);
      analogWrite(led1, 0);
      analogWrite(led2, 0);
      analogWrite(led3, 0);
      old = state ;
      break;
    default :
      analogWrite(led1, 0);
      analogWrite(led2, 0);
      analogWrite(led3, 0);
      analogWrite(led4, 0);
      analogWrite(led5, 0);
      old = state ;
      state = 1 ;
      break;
  }
}

[/code]

      state = old + 1 ;
...
      old = state ;

Why do you save the state and increment it? Why more than one state variable?

Koepel:
Why is there a variable 'old', I don't get it. Can you just use the variable 'state' for everything ?

TomDuino2017, with a such a Finite State Machine, it is normal to change the state to a new state inside the switch-case. When a state has finished, just set the 'state' to a new state that should be done next time.

TomDuino2017:
Okay , had some time today to get back to this code . Not sure if the egregious error is my code , the compiler didn't catch it or most likely both .

In this case most of the blame is on the compiler.
You created a code block (opening curly braces) within a case: block, which as such is valid. You actually have to do this to create local variables at that point. Then you close the block before the next case: statement and everything is fine.
However then there were more case: statements inside that block - that should have made the compiler throw a syntax error, as it is obviously invalid. Code should never be allowed to jump to a point inside another block. It can of course be that this is one of the allowed behaviours of the infamous goto statement (which I know exists in C++ but never used as such; I've heard though that switch/case is actually a form of goto)??