button non responsive

hi, i have this code but the button is not very responsive, apparently it has something to do with the delays but i would not know how to change that, please help :slight_smile:

/*
* Police Light Flasher using two arrays of WHITE and blue LEDs.
*/

#define SWHITE 7 // WHITE LEDs on the slave module
#define SBLUE 8 // Blue LEDs on the slave module
#define MWHITE 2 // WHITE LEDs on the main module
#define MBLUE 4 // Blue LEDs on the main module
int button = 12; // button is connected to pin 12
int val; // variable for reading the button status
int state; // variable to hold the last button state
int presses = 0; // how many times the button was pressed
int mode = 0; // what mode the flasher is in
void setup(){ 
pinMode(SWHITE,OUTPUT); //sets WHITE LEDs as output
pinMode(SBLUE,OUTPUT); //sets Blue LEDs as output
pinMode(MWHITE,OUTPUT); //sets WHITE LEDs as output
pinMode(MBLUE,OUTPUT); //sets Blue LEDs as output
pinMode(button,INPUT); //sets the button as an input

state = digitalRead(button); //reads buttons start position
} 

void loop(){ 
val = digitalRead(button);  //sets val to the state of the button press
delay(10);  //debounce multiple button presses
if (val != state){  //compares button press to current state 
if (val == LOW){ 
if (mode == 0){ //if mode is zero and button was pressed, set mode to 1
mode = 1; 
} else { 
if (mode == 1) {  //increment mode to 2 
mode = 2;
} else { 
if (mode == 2){ 
mode = 0; 
} 
} 
} 
} 
state = val; 
} 

if (mode == 0){ //ALL OFF //mode 0 turns all LEDs off
digitalWrite(SWHITE,LOW);
digitalWrite(MWHITE,LOW);
digitalWrite(SBLUE,HIGH);
digitalWrite(MBLUE,HIGH);
}
if (mode == 1){ // strobe both sides, alternating color
byte count = 0; //sets a counter for flashing
byte number = 0; while (count < 3){ //counter loop to flash WHITE LEDs 5 times
digitalWrite(SWHITE,HIGH);
digitalWrite(MWHITE,HIGH);
delay(60);
digitalWrite(SWHITE,LOW);
digitalWrite(MWHITE,LOW);
delay(60);
count++;
}
while (number < 3){ //counter loop to flash blue LEDs 5 times
digitalWrite(SBLUE,HIGH);
digitalWrite(MBLUE,HIGH);
delay(60);
digitalWrite(SBLUE,LOW);
digitalWrite(MBLUE,LOW);
delay(60);
number++;
}
}
if (mode == 2){ // WHITE flasher, alternating sides
digitalWrite(SWHITE,HIGH);  //slave WHITE on
digitalWrite(MWHITE,HIGH);  //main WHITE on
delay(100); //wait 100 ms
digitalWrite(SWHITE,LOW); //slave WHITE off
digitalWrite(MWHITE,LOW); //main WHITE off
digitalWrite(SBLUE,HIGH);  //slave BLUE on
digitalWrite(MBLUE,HIGH);  //main BLUE on
delay(100); //wait 100 ms
digitalWrite(SBLUE,LOW); //slave BLUE off
digitalWrite(MBLUE,LOW); //main BLUE off
}
}

p.s I am new to coding

Next time, before you post your code, use the auto formatter (on the Tools menu) to properly indent your code. It's difficult to be sure what this does because it's hard to find the loops.

Looking at mode 1, your comment says "flash 5 times" but then there's a loop that appears to flash them 3 times. There doesn't seem to be any way of leaving mode 1 except if the button is pushed, so it will flash 3 times 3 times 3 times 3 times... Is this the same as just flashing 1,1,1,1,1,1... and checking the button between every flash?

This is possibly a candidate for an interrupt. You want it to do a complex flashing cycle but if the button is pushed at any time during that cycle, it will switch to the next pattern after it has finished the current cycle. Is that what you want or do you want it to immediately switch to the next pattern as soon as the button is pushed?

This is possibly a candidate for an interrupt.

Perhaps but unnecessary. It would be better if the code was restructured to not have any blocking sections. However, that also takes more coding knowledge and I see the OP is new to coding.
If you want to show him how to use an interrupt, go ahead.

something like this untested code (it compiles)

/*
* Police Light Flasher using two arrays of WHITE and blue LEDs.
*/

int state = 0;
int lastState;
bool lastVal;
byte flashes = 10;

#define SWHITE 7 // WHITE LEDs on the slave module
#define SBLUE 8 // Blue LEDs on the slave module
#define MWHITE 2 // WHITE LEDs on the main module
#define MBLUE 4 // Blue LEDs on the main module
int button = 12; // button is connected to pin 12
void setup()
{
  pinMode(SWHITE, OUTPUT); //sets WHITE LEDs as output
  pinMode(SBLUE, OUTPUT); //sets Blue LEDs as output
  pinMode(MWHITE, OUTPUT); //sets WHITE LEDs as output
  pinMode(MBLUE, OUTPUT); //sets Blue LEDs as output
  pinMode(button, INPUT); //sets the button as an input
}

void loop()
{
  bool val = digitalRead(button);
  delay(30);
  if (val != lastVal && lastVal == false) //capture one side of the state change
  {
    allLedsOff();
    state ++;
    if (state > 3)
      state = 0;
  }
  if (state == 0)
  {
    //nothing to do here
  }
  else if (state == 1)
  {
    strobeBoth();
  }
  else if (state == 2)
  {
    if (state != lastState)
    {
      flashes = 10;
    }
    strobeWhite();
  }
  else if (state == 3)
  {
    if (state != lastState)
    {
      flashes = 10;
    }
    strobeBlue();
  }
  lastVal = val;
  lastState = state;
}

void allLedsOff()
{
  digitalWrite(SWHITE, LOW);
  digitalWrite(MWHITE, LOW);
  digitalWrite(SBLUE, HIGH);
  digitalWrite(MBLUE, HIGH);
}

void strobeBoth()
{
  static unsigned long lastTime = millis();
  static bool myToggle = false;
  if ( millis() - lastTime > 100UL)
  {
    digitalWrite(SWHITE, myToggle);
    digitalWrite(MWHITE, myToggle);
    digitalWrite(SBLUE, !myToggle);
    digitalWrite(MBLUE, !myToggle);
    lastTime = millis();
    myToggle = !myToggle;
  }

}
void strobeWhite()
{
  static unsigned long lastTime = millis();
  if ( millis() - lastTime > 100UL && flashes > 0)
  {
    digitalWrite(SWHITE, !digitalRead(SWHITE));
    digitalWrite(MWHITE, !digitalRead(MWHITE));
    lastTime = millis();
    flashes = max(0, flashes--);
  }
}
void strobeBlue()
{
  static unsigned long lastTime = millis();
  if ( millis() - lastTime > 100UL && flashes > 0)
  {
    digitalWrite(SBLUE, !digitalRead(SBLUE));
    digitalWrite(MBLUE, !digitalRead(MBLUE));
    lastTime = millis();
    flashes = max(0, flashes--);
  }
}

Hi,
How have you got your buttons wired?
Are you using pull up or pull down resistors?

Tom..... :slight_smile:

this isn’t a finished program it just shows a different way to write the code and also does away with the delays. it should work for testing.

/*
* Police Light Flasher using two arrays of WHITE and blue LEDs.
*/

#define SWHITE 7 // WHITE LEDs on the slave module
#define SBLUE 8 // Blue LEDs on the slave module
#define MWHITE 2 // WHITE LEDs on the main module
#define MBLUE 4 // Blue LEDs on the main module
#define button 12 // button is connected to pin 12

int state; // variable to hold the last button state
byte prev_mode = 0;
byte mode = 0; // what mode the flasher is in
byte count = 0; //sets a counter for flashing
byte button_state = 0;
byte prev_button_state = 0;
unsigned long previousMillis = 0;
unsigned long interval = 6; //this adjusts the delay
unsigned int timer = 0;
unsigned int timer_2 = 0;


void setup() {
  pinMode(SWHITE, OUTPUT); //sets WHITE LEDs as output
  pinMode(SBLUE, OUTPUT); //sets Blue LEDs as output
  pinMode(MWHITE, OUTPUT); //sets WHITE LEDs as output
  pinMode(MBLUE, OUTPUT); //sets Blue LEDs as output
  pinMode(button, INPUT); //sets the button as an input
}

void loop() {
  button_state = digitalRead(button);  //sets val to the state of the button press

  //compares button press to current state
  if ((prev_button_state != button_state) && (button_state == LOW)) {
    mode++;//change mode
  }
  prev_button_state = button_state;

  if (mode > 2) {
    mode = 0;//roll back to 0
  }

  unsigned long currentMillis = millis();//one second into timers
  if (currentMillis - previousMillis > interval) {
    timer++;
    timer_2++;
    previousMillis = currentMillis;
  }
  //next lines are to detect mode changes to reset timers one time
  if (prev_mode != mode) {
    timer = 0;
    timer_2 = 0;
  }
  prev_mode = mode;

  switch (mode) {

    case 0:  //ALL OFF //mode 0 turns all LEDs off
      digitalWrite(SWHITE, LOW);
      digitalWrite(MWHITE, LOW);
      digitalWrite(SBLUE, HIGH);
      digitalWrite(MBLUE, HIGH);
      break;

    case 1: // strobe both sides, alternating color
      if (count < 5) { //counter loop to flash WHITE LEDs 5 times
        if ((timer > 0) && (timer < 10)) {
          digitalWrite(SWHITE, HIGH);
          digitalWrite(MWHITE, HIGH);
        }
        if ((timer > 10) && (timer < 20)) {
          digitalWrite(SWHITE, LOW);
          digitalWrite(MWHITE, LOW);
        }
        if ((timer > 20) && (timer < 30)) {
          digitalWrite(SBLUE, HIGH);
          digitalWrite(MBLUE, HIGH);
        }
        if ((timer > 30) && (timer < 40)) {
          digitalWrite(SBLUE, LOW);
          digitalWrite(MBLUE, LOW);
        }
        if (timer > 41) {
          count++;
          timer = 0;
        }
      } else {
        count = 0;
        mode = 0;//not sure if this was the plan as it just turns the lights off
      }
      break;

    case 2: // WHITE flasher, alternating sides
      if ((timer_2 > 20) && (timer_2 < 40)) {
        digitalWrite(SWHITE, HIGH); //slave WHITE on
        digitalWrite(MWHITE, HIGH); //main WHITE on
      }
      if ((timer_2 > 20) && (timer_2 < 40)) {
        digitalWrite(SWHITE, LOW); //slave WHITE off
        digitalWrite(MWHITE, LOW); //main WHITE off
        digitalWrite(SBLUE, HIGH); //slave BLUE on
        digitalWrite(MBLUE, HIGH); //main BLUE on}
      }
      if ((timer_2 > 40) && (timer_2 < 60)) {
        digitalWrite(SBLUE, LOW); //slave BLUE off
        digitalWrite(MBLUE, LOW); //main BLUE off
      }
      if (timer_2 > 60) {
        mode = 0;
        timer_2 = 0;
      }
      break;
  }
}