Go Down

Topic: LED x2 controlled by push buttons x2 (Read 1 time) previous topic - next topic

cklkaren

Mar 12, 2015, 06:54 pm Last Edit: Mar 12, 2015, 09:04 pm by cklkaren
Hello, I have some problem with my code that involves 2 push buttons and 2 RGB LED pins where the LED pin should respond to a push button each.

This is what my project would be :

1) 2 RGB LED Pins would flash one after another (as a sequence)
2) When a push button that associated with the RGB LED pin is hit, the associated LED would stay light up (like a switch) and the other one would have light off
3) When u press the same button again, the button's light will turn off and the 2 LED pins would flash as a sequence again

I got the (1) and (2) working alone. But when I tried to put the code together, it doesn't work, only the LED light sequence is playing and it doesn't respond to any push button pressed . Could someone please suggest where did it go wrong please?

Thanks :)))

Code: [Select]
int switchPin = 7;
int switchPin2 = 8;

int LED1 = 13;
int LED2 = 12;

int ledState = LOW;
int buttonState;
int lastButtonState = HIGH;

long lastDebounceTime =0;
long debounceDelay = 50;

int lastButtonState2 = HIGH;
long lastDebounceTime2 =0;
long debounceDelay2 = 50;

void setup() {
 pinMode (switchPin, INPUT);
 pinMode (switchPin2, INPUT);
 pinMode (LED1, OUTPUT);
 pinMode (LED2, OUTPUT);
 Serial.begin(9600);
 
 digitalWrite (switchPin, ledState);
}

void loop() {
 
      //LED blink before any buttons are being pressed.
      digitalWrite(LED1, HIGH);
      delay(500);
      digitalWrite(LED1, LOW);
      delay(500);
      digitalWrite(LED2, HIGH);
      delay(500);
      digitalWrite(LED2, LOW);
      delay(500);
 
  //when pressed, do this.   
  int reading = digitalRead(switchPin);
  int reading2 = digitalRead(switchPin2);

    if (reading != lastButtonState) {
    lastDebounceTime = millis();
  }
 
      if (reading2 != lastButtonState2) {
    lastDebounceTime2 = millis();
  }
 
  if ((millis() - lastDebounceTime) > debounceDelay) {
    if (reading != buttonState) {
      buttonState = reading;

      if (buttonState == LOW) {
        ledState = !ledState;
      }
    }
  }
 
    if ((millis() - lastDebounceTime2) > debounceDelay2) {
      if (reading2 != buttonState) {
        buttonState = reading2;
 
        if (buttonState == LOW) {
          ledState = !ledState;
      }
    }
  }
 
   if (digitalRead(switchPin) == LOW) {
     Serial.write(0);
     digitalWrite(LED1, ledState);
   } else {
     Serial.write(1);
     digitalWrite(LED1, ledState);
   }
   
   if (digitalRead(switchPin2) == LOW) {
     Serial.write(2);
     digitalWrite(LED2, ledState);
   } else {
     Serial.write(3);
     digitalWrite(LED2, ledState);   
   }
   delay(100);
 
 lastButtonState = reading;
}

Grumpy_Mike

#1
Mar 12, 2015, 10:50 pm Last Edit: Mar 12, 2015, 10:51 pm by Grumpy_Mike
Quote
Could someone please suggest where did it go wrong please?
Code: [Select]
delay(500);
During a delay nothing else can happen. You need to structure your code as a timer based state machine. See these two links.
My:-
http://www.thebox.myzen.co.uk/Tutorial/State_Machine.html
And / or Robin2's several things at once
http://forum.arduino.cc/index.php?topic=223286.0

Forget debouncing until you know you need it.

cklkaren

During a delay nothing else can happen. You need to structure your code as a timer based state machine. See these two links.
My:-
http://www.thebox.myzen.co.uk/Tutorial/State_Machine.html
And / or Robin2's several things at once
http://forum.arduino.cc/index.php?topic=223286.0

Forget debouncing until you know you need it.
Thanks Mike for your suggestions. I have now modified my code as the tutorials suggested and also to use 2 LEDs associating with a switch each.

I am very new to coding and now my problem with the code is that if i should set the functions to boolean instead?

My loop is playing everything together and so even when I am pressing the button, the LEDs are still flashing in sequence, which my desired outcome is that:
1) When button is pressed, the associated LED would stay on and light up and my LEDs that flash in sequence would stop flashing
2) When the button is pressed again the associated LED would turn off and my LEDs will flash in sequence again.

Cheers,
Karen

Code: [Select]
int switchPin = 7;
int switchPin2 = 8;

int LED1 = 13;
int LED2 = 12;
int LED1LightUp = 11;
//int LED2LightUp = 10;

const int led1Interval = 500;
const int led2Interval = 1000;
const int blinkDuration = 500;
const int buttonInterval = 300;

byte led1State = LOW;           //   LOW = off
byte led2State = LOW;
byte switchPin_State = LOW;

int ledState = HIGH;         // the current state of the output pin
int buttonState;             // the current reading from the input pin
int lastButtonState = LOW;   // the previous reading from the input pin

// the following variables are long's because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long lastDebounceTime = 0;  // the last time the output pin was toggled
long debounceDelay = 50;

unsigned long currentMillis = 0;    // stores the value of millis() in each iteration of loop()
unsigned long previousLed1Millis = 0;
unsigned long previousLed2Millis = 0;

unsigned long previousButtonMillis = 0; // time when button press last checked


//=============================================
void setup() {

  Serial.begin(9600);  
  
  // set the button pin as input with a pullup resistor to ensure it defaults to HIGH
   pinMode (switchPin, INPUT_PULLUP);
   pinMode (switchPin2, INPUT_PULLUP);
   pinMode (LED1, OUTPUT);
   pinMode (LED2, OUTPUT);
   pinMode (LED1LightUp, OUTPUT);
//   pinMode (LED2LightUp, OUTPUT);
}

//=============================================

void loop(){
  currentMillis = millis();
  
  readButton1();
  updateLed1FlashState();
  updateLed2FlashState();
  switchLeds();
}

//=============================================
void updateLed1FlashState() {

  if (led1State == LOW) {
    if (currentMillis - previousLed1Millis >= led1Interval) {
       led1State = HIGH;
       previousLed1Millis += led1Interval;
    }
  }
  else {
    if (currentMillis - previousLed1Millis >= blinkDuration) {
       led1State = LOW;
       previousLed1Millis += blinkDuration;
    }
  }    
}

//========================================

void updateLed2FlashState() {

  if (led2State == LOW) {
    if (currentMillis - previousLed2Millis >= led2Interval) {
       led2State = HIGH;
       previousLed2Millis += led2Interval;
    }
  }
  else {
    if (currentMillis - previousLed2Millis >= blinkDuration) {
       led2State = LOW;
       previousLed2Millis += blinkDuration;
    }
  }    
}

//=============================================

void switchLeds(){  
      // this is the code that actually switches the LEDs on and off
  digitalWrite(LED1, led1State);
  digitalWrite(LED2, led2State);
  digitalWrite(LED1LightUp, switchPin_State);  
}

//=============================================

void readButton1(){

    int reading = digitalRead(switchPin);

  if (reading != lastButtonState) {
    lastDebounceTime = millis();
  }
  
  if ((millis() - lastDebounceTime) > debounceDelay) {
    if (reading != buttonState) {
      buttonState = reading;

      // only toggle the LED if the new button state is HIGH
      if (buttonState == HIGH) {
        ledState = !ledState;
      }
    }
  }
  
  // set the LED:
  digitalWrite(LED1LightUp, ledState);

  // save the reading.  Next time through the loop,
  // it'll be the lastButtonState:
  lastButtonState = reading;
  
}

Grumpy_Mike

Quote
now my problem with the code is that if i should set the functions to boolean instead?
Not too sure what you mean by that.
All those functions are void that is they do not return a value, if they returned a Boolean variable you would have to set them the boolean. That is the answer to what you asked but I think that was not the question you wanted to ask.

cklkaren

#4
Mar 15, 2015, 05:01 pm Last Edit: Mar 15, 2015, 05:05 pm by cklkaren
Not too sure what you mean by that.
All those functions are void that is they do not return a value, if they returned a Boolean variable you would have to set them the boolean. That is the answer to what you asked but I think that was not the question you wanted to ask.
I have added a boolean function whenever the button is being pressed.

The code worked perfectly when I was dealing with one button; however when I added it to 3 buttons, they do not function as desired (the flashing sequence do not stop when button is being pressed):

1) LED pin 11, 12 and 13 will flash as a sequence when the code starts running
2) When a button is pressed, it will light up and stay on; and LED pin 11, 12 and 13 will stop flashing as a sequence.
3) When a button is pressed again, the light will turn off and LED pin 11, 12 and 13 will start flashing as a sequence again.

I also have a problem that the my LED pin 11 is not being read at all.
Sorry to bother you again but are there any suggestions you could give me please? Are there logic wrong in my code please?

Cheers,
Karen

Code: [Select]
int switchPin = 9;
int switchPin2 = 8;
int switchPin3 = 7;

int LED1 = 13;
int LED2 = 12;
int LED3 = 11;
int LED1LightUp = 5;
int LED2LightUp = 4;
int LED3LightUp = 3;

const int led1Interval = 500;
const int led2Interval = 1000;
const int led3Interval = 1500;
const int blinkDuration = 500;
const int buttonInterval = 300;

byte led1State = LOW;           //   LOW = off
byte led2State = LOW;
byte led3State = LOW;
//byte switchPin_State = LOW;

int ledState = LOW;        // the current state of the output pin
int buttonState;             // the current reading from the input pin
int lastButtonState = LOW;   // the previous reading from the input pin

int ledState2 = LOW;       
int buttonState2;             
int lastButtonState2 = LOW;

int ledState3 = LOW;       
int buttonState3;             
int lastButtonState3 = LOW;

long lastDebounceTime = 0;  // the last time the output pin was toggled
long debounceDelay = 50;

unsigned long currentMillis = 0;    // stores the value of millis() in each iteration of loop()
unsigned long previousLed1Millis = 0;
unsigned long previousLed2Millis = 0;
unsigned long previousLed3Millis = 0;

unsigned long previousButtonMillis = 0; // time when button press last checked

boolean LEDFlashSequence = false;

//=====================================================================

void setup() {

  Serial.begin(9600); 
 
  // set the button pin as input with a pullup resistor to ensure it defaults to HIGH
   pinMode (switchPin, INPUT_PULLUP);
   pinMode (switchPin2, INPUT_PULLUP);
   pinMode (switchPin3, INPUT_PULLUP);
   pinMode (LED1, OUTPUT);
   pinMode (LED2, OUTPUT);
   pinMode (LED3, OUTPUT);
   pinMode (LED1LightUp, OUTPUT);
   pinMode (LED2LightUp, OUTPUT);
   pinMode (LED3LightUp, OUTPUT);
}

//=============================================

void loop(){
  currentMillis = millis();
 
  readButton1();
  readButton2();
  readButton3();   
  updateLed1FlashState();
  updateLed2FlashState();
  updateLed3FlashState();
  switchLeds();
}

//=============================================

void updateLed1FlashState() {

  if (led1State == LOW) {
    if (currentMillis - previousLed1Millis >= led1Interval) {
       led1State = HIGH;
       previousLed1Millis += led1Interval;
    }
  }
  else {
    if (currentMillis - previousLed1Millis >= blinkDuration) {
       led1State = LOW;
       previousLed1Millis += blinkDuration;
    }
  }   
}

//========================================

void updateLed2FlashState() {

  if (led2State == LOW) {
    if (currentMillis - previousLed2Millis >= led2Interval) {
       led2State = HIGH;
       previousLed2Millis += led2Interval;
    }
  }
  else {
    if (currentMillis - previousLed2Millis >= blinkDuration) {
       led2State = LOW;
       previousLed2Millis += blinkDuration;
    }
  }   
}

//========================================

void updateLed3FlashState() {

  if (led3State == LOW) {
    if (currentMillis - previousLed2Millis >= led3Interval) {
       led3State = HIGH;
       previousLed3Millis += led3Interval;
    }
  }
  else {
    if (currentMillis - previousLed3Millis >= blinkDuration) {
       led3State = LOW;
       previousLed3Millis += blinkDuration;
    }
  }   
}


//=============================================

void switchLeds(){ 
      // this is the code that actually switches the LEDs on and off
  if (readButton1()==true){
    LEDFlashSequence = false;
//    readButton2 == false;
//    readButton3 == false;
  } else {
    LEDFlashSequence = true;
//    readButton2 == false;
//    readButton3 == false;
  }
 
    if (readButton2()==true){
    LEDFlashSequence = false;
//    readButton1 == false;
//    readButton3 == false;
  } else {
    LEDFlashSequence = true;
//    readButton1 == false;
//    readButton3 == false;
  }
 
    if (readButton3()==true){
    LEDFlashSequence = false;
//    readButton1 == false;
//    readButton2 == false;   
  } else {
    LEDFlashSequence = true;
//    readButton1 == false;
//    readButton2 == false;
  }
 
  if (LEDFlashSequence == true){ 
  digitalWrite(LED1, led1State);
  digitalWrite(LED2, led2State);
  digitalWrite(LED3, led3State);
  } else {
  digitalWrite(LED1, LOW);
  digitalWrite(LED2, LOW);   
  digitalWrite(LED3, LOW);   
//  digitalWrite(LED1LightUp, switchPin_State); 
  }
}

//=============================================

boolean readButton1(){

    int reading = digitalRead(switchPin);

  // check to see if you just pressed the button
  // (i.e. the input went from LOW to HIGH),  and you've waited
  // long enough since the last press to ignore any noise: 

  // If the switch changed, due to noise or pressing:
  if (reading != lastButtonState) {
    // reset the debouncing timer
    lastDebounceTime = millis();
  }
 
  if ((millis() - lastDebounceTime) > debounceDelay) {
    // whatever the reading is at, it's been there for longer
    // than the debounce delay, so take it as the actual current state:

    // if the button state has changed:
    if (reading != buttonState) {
      buttonState = reading;

      // only toggle the LED if the new button state is HIGH
      if (buttonState == LOW) {
        ledState = !ledState;
      }
    }
    return ledState;
  }
 
  // set the LED:
  digitalWrite(LED1LightUp, ledState);

  // save the reading.  Next time through the loop,
  // it'll be the lastButtonState:
  lastButtonState = reading;
 
}

//=============================================

boolean readButton2(){

    int reading2 = digitalRead(switchPin2);

  if (reading2 != lastButtonState2) {
    lastDebounceTime = millis();
  }
 
  if ((millis() - lastDebounceTime) > debounceDelay) {

    if (reading2 != buttonState2) {
      buttonState2 = reading2;

      if (buttonState2 == LOW) {
        ledState2 = !ledState2;
      }
    }
    return ledState2;
  }

  digitalWrite(LED2LightUp, ledState2);

  lastButtonState2 = reading2;
 
}

//=============================================

boolean readButton3(){

    int reading3 = digitalRead(switchPin3);

  if (reading3 != lastButtonState3) {
    // reset the debouncing timer
    lastDebounceTime = millis();
  }
 
  if ((millis() - lastDebounceTime) > debounceDelay) {

    if (reading3 != buttonState3) {
      buttonState3 = reading3;

      if (buttonState3 == LOW) {
        ledState3 = !ledState3;
      }
    }
    return ledState3;
  }

  digitalWrite(LED3LightUp, ledState3);

  lastButtonState3 = reading3;
}

Grumpy_Mike

That code is very mixed up and way too long for what it has to do.

First of all forget debouncing you don't need it.

You say what you want to do is:-
Quote
1) When button is pressed, the associated LED would stay on and light up and my LEDs that flash in sequence would stop flashing
So there is nothing in the code that will stop an LED from flashing.
Your read functions only return the state of an LED. They do not stop any existing flashing nor start a new sequence.

You need a variable to indicate that each state should be flashing or not flashing. Then in each of the update LED functions if that variable is not true then exit the function. If it is true then go on and :-
1) First see if it is time to change the LED state.
2) Only if it is you then decide what state to change it to.
The way you have it at the moment is inside out.

Now when you detect a button is pressed, you immediately clear all the variables that say sequences are flashing and turn off all the LEDs. Then set the variable to indicate the sequence associated with the pressed button is to be done. All this can be done in one function not three.

In fact if you use arrays to hold your led interval then you can do all the update LED flash states in one function.

Remember if chunks of code look near identical then there is a way of reducing it to just one set of instructions not many.

cklkaren

#6
Mar 15, 2015, 09:32 pm Last Edit: Mar 15, 2015, 09:33 pm by cklkaren
Quote
In fact if you use arrays to hold your led interval then you can do all the update LED flash states in one function.
Hi Mike,

Thanks for your reply.
I am taking your suggestions and modifying my codes gradually.
I tried to use an array to hold my LED interval, however, my code does not react to any button press now...

Cheers,
Karen

Grumpy_Mike

Quote
I tried to use an array to hold my LED interval,
Where? In that code you posted you only used an array to hold LED pin numbers. And you used delay which is what all the millis is supposed to avoid. You still have all the debounce nonsense in.

When something doesn't work then the way to tackle it is to use print statements to see when and if sections of code have been reached and what value variables are at that stage. In that way you can track down what is happening and try and spot your error.

Go Up