Controlling 4 flashing LED's with momentry contact switch

I am trying to control 4 LED's, each with a momentry contact switch, so that when you first press the switch the LED starts flashing, until the button is pressed again.

At the moment I have only tried to make the LED flash 10 times when the button is pressed, but even this does not work correctly. LED 1 flashes occasionally and stays on most of the time, LED 2 also flashes if I press the button, but it does not appear to do so to any pattern that I have programmed and seems to only flash when LED 1 is off. the remaining LED's refuse to flash when the button is pressed.

As I am not used to c programming I am not sure where I am going wrong.
Also how do I program press once for ON, press again for OFF?

I learn best by looking at examples, so please contribute ideas that I can use in this project.

Here is the code:

int switch1Pin = 4;              // Button 1 is connected to pin 4
int switch2Pin = 5;              // Button 2 is connected to pin 5
int switch3Pin = 6;              // Button 3 is connected to pin 6
int switch4Pin = 7;              // Button 4 is connected to pin 7


int led1Pin = 12;
int led2Pin = 11;
int led3Pin = 10;
int led4Pin = 9;


int switch1State = 0;                        
int switch2State = 0;                       
int switch3State = 0;                
int switch4State = 0;
int switch5State = 0;   

int timer = 100;           // The higher the number, the slower the timing.

void setup() {
 pinMode(switch1Pin, INPUT);   // Button 1
 pinMode(switch2Pin, INPUT);   // Button 2
 pinMode(switch3Pin, INPUT);   // Button 3
 pinMode(switch4Pin, INPUT);   // Button 4
 
 pinMode(led1Pin, OUTPUT);
 pinMode(led2Pin, OUTPUT);
 pinMode(led3Pin, OUTPUT);
 pinMode(led4Pin, OUTPUT);
 }

void loop() {
 switch1State = digitalRead(switch1Pin);      // read input values
  switch2State = digitalRead(switch2Pin);     
  switch3State = digitalRead(switch3Pin);
  switch4State = digitalRead(switch4Pin);
 
  
  if (switch1State == HIGH)                  // when the first button is pressed
    VID1(); 
   else if (switch2State == HIGH)            // when the next button is pressed
    VID2();
  else if (switch3State == HIGH)             // the next button
    VID3();
  else if (switch4State == HIGH)             // the next button
    VID4();

}

void VID1(){
  for (int i=0; i <10; i++){
digitalWrite(led1Pin, HIGH);
    delay(timer);
    // turn the pin off:
    digitalWrite(led1Pin, LOW);
}
}

void VID2(){
  for (int i=0; i <10; i++){
    digitalWrite(led2Pin, HIGH);
    delay(timer);
    // turn the pin off:
    digitalWrite(led2Pin, LOW);
}
}
void VID3(){
  for (int i=0; i <10; i++){
    digitalWrite(led3Pin, HIGH);
    delay(timer);
    // turn the pin off:
    digitalWrite(led3Pin, LOW);
}
}   
void VID4(){
  for (int i=0; i <10; i++){
    digitalWrite(led4Pin, HIGH);
    delay(timer);
    // turn the pin off:
    digitalWrite(led4Pin, LOW);
}
}

Many thanks!

You need to eliminate the delay() in your code. The Arduino does nothing while in delay, so it will not look at the switch press. REad and understand the BlinkWithoutDelay example.

You also need to detecy when a swicth is pressed (that is, when it goes from off to on, the transition) rather than when it is just on (HIGH). Otherwise you stuff will happen only when you have the switch down. I think there is a StateChange example to show this.

First thing, if you don't have pulldown resistors (10k from button pin to GND) the input pins will be floating and might read HIGH when the button is not pressed, easy fix is connect buttons from pin to GND and use the internal pullups with:

pinMode(switch1Pin,INPUT_PULLUP);

and invert all your input reads:

if (switch1State == LOW)

Thanks for the tips!
The switches work now with one exception!
If I switch off in the ON period the LED stays on, but if I switch off in the OFF period it stays off. I want the lights to stay off if the switch is OFF. How can I do this?

I have looked and incorporated the BlinkWithoutDelay in the code an this works well.

Here is the revised code:

int switch1Pin = 4;              // Button 1 is connected to pin 4
int switch2Pin = 5;              // Button 2 is connected to pin 5
int switch3Pin = 6;              // Button 3 is connected to pin 6
int switch4Pin = 7;              // Button 4 is connected to pin 7


const int led1Pin = 12;
const int led2Pin = 11;
const int led3Pin = 10;
const int led4Pin = 9;


int switch1State = 0;                        
int switch2State = 0;                       
int switch3State = 0;                
int switch4State = 0;
int switch5State = 0;   

//int timer = 100;           // The higher the number, the slower the timing.

// Variables will change:
//led1
int led1State = LOW;             // ledState used to set the LED
long previousMillis = 0;        // will store last time LED was updated

// the follow variables is a long because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long interval;           // interval at which to blink (milliseconds)
long onTime=1000;
long offTime=1000;

//led2

int led2State = LOW;             // ledState used to set the LED
long previousMillis2 = 0;        // will store last time LED was updated

// the follow variables is a long because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long interval2;           // interval at which to blink (milliseconds)
long onTime2=250;
long offTime2=250;

//led3

int led3State = LOW;             // ledState used to set the LED
long previousMillis3 = 0;        // will store last time LED was updated

// the follow variables is a long because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long interval3;           // interval at which to blink (milliseconds)
long onTime3=1000;
long offTime3=1000;

int led4State = LOW;             // ledState used to set the LED
long previousMillis4 = 0;        // will store last time LED was updated

// the follow variables is a long because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long interval4;           // interval at which to blink (milliseconds)
long onTime4=100;
long offTime4=100;


void setup() {
 pinMode(switch1Pin, INPUT_PULLUP);   // Button 1
 pinMode(switch2Pin, INPUT_PULLUP);   // Button 2
 pinMode(switch3Pin, INPUT_PULLUP);   // Button 3
 pinMode(switch4Pin, INPUT_PULLUP);   // Button 4
 
 pinMode(led1Pin, OUTPUT);
 pinMode(led2Pin, OUTPUT);
 pinMode(led3Pin, OUTPUT);
 pinMode(led4Pin, OUTPUT);
 }

void loop() {
  switch1State = digitalRead(switch1Pin);      // read input values
  switch2State = digitalRead(switch2Pin);     
  switch3State = digitalRead(switch3Pin);
  switch4State = digitalRead(switch4Pin);
 
  
  if (switch1State == LOW)                  // when the first button is pressed
    VID1(); 
   else if (switch2State == LOW)            // when the next button is pressed
    VID2();
  else if (switch3State == LOW)             // the next button
    VID3();
  else if (switch4State == LOW)             // the next button
    VID4();

}

void VID1(){

  // check to see if it's time to blink the LED; that is, if the
  // difference between the current time and last time you blinked
  // the LED is bigger than the interval at which you want to
  // blink the LED.
  unsigned long currentMillis = millis();

  if(currentMillis - previousMillis > interval) {
    // save the last time you blinked the LED
    previousMillis = currentMillis;   

    // if the LED is off turn it on and vice-versa:
    if (led1State == LOW)
    {
      led1State = HIGH;
      interval=onTime;
    }
    else
    {
      led1State = LOW;
      interval=offTime;
    }

    // set the LED with the ledState of the variable:
    digitalWrite(led1Pin, led1State);
  }
} //end VID1

void VID2()
{

  // check to see if it's time to blink the LED; that is, if the
  // difference between the current time and last time you blinked
  // the LED is bigger than the interval at which you want to
  // blink the LED.
  unsigned long currentMillis2 = millis();

  if(currentMillis2 - previousMillis2 > interval2) {
    // save the last time you blinked the LED
    previousMillis2 = currentMillis2;   

    // if the LED is off turn it on and vice-versa:
    if (led2State == LOW)
    {
      led2State = HIGH;
      interval2=onTime2;
    }
    else
    {
      led2State = LOW;
      interval2=offTime2;
    }

    // set the LED with the ledState of the variable:
    digitalWrite(led2Pin, led2State);
  }
}//end VID2

void VID3()
{

  // check to see if it's time to blink the LED; that is, if the
  // difference between the current time and last time you blinked
  // the LED is bigger than the interval at which you want to
  // blink the LED.
  unsigned long currentMillis3 = millis();

  if(currentMillis3 - previousMillis3 > interval3) {
    // save the last time you blinked the LED
    previousMillis3 = currentMillis3;   

    // if the LED is off turn it on and vice-versa:
    if (led3State == LOW)
    {
      led3State = HIGH;
      interval3=onTime3;
    }
    else
    {
      led3State = LOW;
      interval3=offTime3;
    }

    // set the LED with the ledState of the variable:
    digitalWrite(led3Pin, led3State);
  }
}//end VID3  

void VID4()
{

  // check to see if it's time to blink the LED; that is, if the
  // difference between the current time and last time you blinked
  // the LED is bigger than the interval at which you want to
  // blink the LED.
  unsigned long currentMillis3 = millis();

  if(currentMillis3 - previousMillis3 > interval3) {
    // save the last time you blinked the LED
    previousMillis3 = currentMillis3;   

    // if the LED is off turn it on and vice-versa:
    if (led4State == LOW)
    {
      led4State = HIGH;
      interval3=onTime3;
    }
    else
    {
      led4State = LOW;
      interval3=offTime3;
    }

    // set the LED with the ledState of the variable:
    digitalWrite(led4Pin, led4State);
  }
}//end VID4

Good job.

Here is the same code rewritten using arrays - this prevents the extensive cut and paste and should make the code more reliable (ie, one edit changes everything). As you can see this makes it much shorter and scalable. Jut by changing the MAX_PIN at the top and filling in the extra data, the software will work.

#define MAX_PIN 4 // max number of buttons

const uint8_t switchPin[MAX_PIN] = { 4, 5, 6, 7 };
const uint8_t ledPin[MAX_PIN] = { 12, 11, 10, 9 };
uint8_t switchState[MAX_PIN] = { 0 };                        
uint8_t ledState[MAX_PIN] = { LOW, LOW, LOW, LOW }; 
uint32_t previousMillis[MAX_PIN] = { 0 };
uint32_t onTime[MAX_PIN] = { 1000, 250, 1000, 100 };
uint32_t offTime[MAX_PIN] = { 1000, 250, 1000, 100 };
uint32_t interval[MAX_PIN] = { 0 };

void setup() 
{
 for (uint8_t i=0; i< MAX_PIN; i++)
 {
  pinMode(switchPin[i], INPUT_PULLUP);
  pinMode(ledPin[i], OUTPUT);
 }
}
 
void loop() 
{
  for (uint8_t i=0; i<MAX_PIN; i++)
  {
    switchState[i] = digitalRead(switchPin[i]);
    if (switchState[i] == LOW)                  // when the first button is pressed
      VID(i); 
  }
}

void VID(uint8_t num)
{
  // check to see if it's time to blink the LED; that is, if the
  // difference between the current time and last time you blinked
  // the LED is bigger than the interval at which you want to
  // blink the LED.
  unsigned long currentMillis = millis();

  if (currentMillis - previousMillis[num] > interval[num])
  {
    // save the last time you blinked the LED
    previousMillis[num] = currentMillis;   

    // if the LED is off turn it on and vice-versa:
    if (ledState[num] == LOW)
    {
      ledState[num] = HIGH;
      interval[num]=onTime[num];
    }
    else
    {
      ledState[num] = LOW;
      interval[num]=offTime[num];
    }

    // set the LED with the ledState of the variable:
    digitalWrite(ledPin[num], ledState[num]);
  }
} //end VID

I still don't see that you are detecting the transition from not-pressed to pressed and vice versa. You are only checking if the switch is low. I would rewrite it like this below (note I have note tested this!). It can be made a lot more efficient but you would lose the logic you have created and not recognise the code. Also you need to look at debouncing your switches - a mechanical switch contact will have a lot of on/off action before it settles down to be on or off.

#define MAX_PIN 4 // max number of buttons

const uint8_t switchPin[MAX_PIN] = { 4, 5, 6, 7 };
const uint8_t ledPin[MAX_PIN] = { 12, 11, 10, 9 };

uint8_t switchState[MAX_PIN] = { 0 };                        
uint8_t ledState[MAX_PIN] = { LOW, LOW, LOW, LOW }; 
boolean ledActive[MAX_PIN] = { false };

uint32_t previousMillis[MAX_PIN] = { 0 };
uint32_t onTime[MAX_PIN] = { 1000, 250, 1000, 100 };
uint32_t offTime[MAX_PIN] = { 1000, 250, 1000, 100 };
uint32_t interval[MAX_PIN] = { 0 };

void setup() 
{
 for (uint8_t i=0; i< MAX_PIN; i++)
 {
  pinMode(switchPin[i], INPUT_PULLUP);
  pinMode(ledPin[i], OUTPUT);
 }
}
 
void loop() 
{
  for (uint8_t i=0; i<MAX_PIN; i++)
    VID(i); 
}

void VID(uint8_t num)
{
  // check to see if it's time to blink the LED; that is, if the
  // difference between the current time and last time you blinked
  // the LED is bigger than the interval at which you want to
  // blink the LED.
  uint8_t state = digitalRead(switchPin[num]);

  // check for switch not pressed to pressed transition
  if (switchState[num] == state) // no transition
    return; // do nothing
  if (switchState[num] == LOW && state == HIGH) // pressed top not pressed transition
  {
    switchState[num] == state;  // remember new state
    return;   // but still do nothing
  }
  //all that is left is a not pressed to prerssed transition
  // if we are currently not flaching this LED, start flashing. Otherwise stop flashing.
  if (!ledActive[num]) // currently not flashing
  {
    ledActive[num] = true;
    previousMillis[num] = millis();
  }
  else
    ledActive[num] = false;
  
  // now do the flashing
  if (ledActive[num] && millis() - previousMillis[num] > interval[num])
  {
    // save the last time you blinked the LED
    previousMillis[num] = millis();   

    // if the LED is off turn it on and vice-versa
    if (ledState[num] == LOW)
    {
      ledState[num] = HIGH;
      interval[num]=onTime[num];
    }
    else
    {
      ledState[num] = LOW;
      interval[num]=offTime[num];
    }

    // set the LED with the ledState of the variable:
    digitalWrite(ledPin[num], ledState[num]);
  }
} //end VID

Thanks Marco!

I have tried the first code and it works as the code I wrote, but when I tried the second code you submitted it did not work (No flashing when any button pressed).
However I am not experienced enough to understand why it is not working!

On my test rig I am using momentry ON/OFF switches, but want to use latching switches for the final version. Will this cause problems with switch bounce?

No surprise that the code did not work 'out of the box' as I had not tested it.

Switch bounce is an issue with all mechanical switches.