help with led loop with in function

First post yay! So I cam from basic stamp 2 so I'm not a complete noob here just with c++. Basically I need to loop a blinking led within a statement forever until a relay is pulled LOW.

void loop() {
   // Get button event and act accordingly
   int b = checkButton();
   if (b == 1) clickEvent();
   if (b == 2) doubleClickEvent();
   if (b == 3) holdEvent();
   if (b == 4) longHoldEvent();

   int b2 = checkButton2();
   if (b2 == 1) clickEvent2();
   if (b2 == 2) doubleClickEvent2();
   if (b2 == 3) holdEvent2();
   if (b2 == 4) longHoldEvent2();
}

//=================================================
// Events to trigger

void clickEvent() {
   digitalWrite(relay1, HIGH);
   blink1();                     // this is where the led blink function is called.
}
void doubleClickEvent() {
   digitalWrite(relay1, LOW);
   digitalWrite(ledpin1, LOW);
}
void holdEvent() {
   digitalWrite(relay1, HIGH);
   digitalWrite(ledpin1, HIGH);
}
void longHoldEvent() {
   digitalWrite(relay1, LOW);
   digitalWrite(ledpin1, LOW);
}

This is where the function called blink is called in my loop. I need this function below to blink the led until relay1 is low.

int blink1() {
    while (digitalRead(relay1) == HIGH) {
    // this is where it needs to loop forever until relay 1 is LOW
}


int blink2() {
    digitalWrite(ledpin2, HIGH);   // turn the LED on (HIGH is the voltage level)
  delay(1000);                       // wait for a second
  digitalWrite(ledpin2, LOW);    // turn the LED off by making the voltage LOW
  delay(1000);  
}

I started with "while" and I'm a bit stumped from here. Just to be clear much of this isn't my code is if from Jeff Saltzman from these forums. Any help would be appreciated guys!

Ok so I got it to blink while relay1 was HIGH. My problem now is no matter what I do after it goes into loop it won't respond to any buttons.

int blink1() {
    while (digitalRead(relay1 == HIGH)) {
      digitalWrite(ledpin1, HIGH);  
      delay(1000);                       
      digitalWrite(ledpin1, LOW);   
      delay(1000);
    }
  }
while (digitalRead(relay1) == HIGH)

You have a ')' in the wrong place; compare above with your code.

sterretje:

while (digitalRead(relay1) == HIGH)

You have a ‘)’ in the wrong place; compare above with your code.

Well you we’re right but I’m still stuck in the loop.

Hi,
Welcome to the forum.

Because you ave not posted your entire code, no void setup() info, we cannot see how you have configured your "relay" input.

If your "relay" contacts pull arduino input to 5V, have you got a 10K resistor from the arduino input pin to gnd?

If your "relay" contacts pull arduino input to gnd, have you got a 10K resistor from the arduino input pin to 5V, or internal_ pullup resistor turned ON?

The inputs are high impedance.

What model arduino are you using?

Thanks.. Tom.. :slight_smile:

/* 4-Way Button:  Click, Double-Click, Press+Hold, and Press+Long-Hold Test Sketch

By Jeff Saltzman
Oct. 13, 2009

To keep a physical interface as simple as possible, this sketch demonstrates generating four output events from a single push-button.
1) Click:  rapid press and release
2) Double-Click:  two clicks in quick succession
3) Press and Hold:  holding the button down
4) Long Press and Hold:  holding the button for a long time
*/

#define buttonPin 9
#define buttonpin2 8
#define ledpin1 7          // digital output pin for LED 1
#define ledpin2 6
#define relay1 12
#define relay2 11
 



void setup() {
   // Set button input pin
   pinMode(buttonPin, INPUT);
   pinMode(buttonpin2, INPUT);
   pinMode(relay1, OUTPUT);
   pinMode(relay2, OUTPUT);
   pinMode(ledpin1, OUTPUT);  
   pinMode(ledpin2, OUTPUT);  

   

}
 

void loop() {
   // Get button event and act accordingly
   int b = checkButton();
   if (b == 1) clickEvent();
   if (b == 2) doubleClickEvent();
   if (b == 3) holdEvent();
   if (b == 4) longHoldEvent();

   int b2 = checkButton2();
   if (b2 == 1) clickEvent2();
   if (b2 == 2) doubleClickEvent2();
   if (b2 == 3) holdEvent2();
   if (b2 == 4) longHoldEvent2();
}

//=================================================
// Events to trigger

void clickEvent() {
   digitalWrite(relay1, HIGH);
   blink1();                     // this is where the led blink function is called.
}
void doubleClickEvent() {
   digitalWrite(relay1, LOW);
   digitalWrite(ledpin1, LOW);
}
void holdEvent() {
   digitalWrite(relay1, HIGH);
   digitalWrite(ledpin1, HIGH);
}
void longHoldEvent() {
   digitalWrite(relay1, LOW);
   digitalWrite(ledpin1, LOW);
}


void clickEvent2() {       //click event 2 for second button
   digitalWrite(relay2, HIGH);
   blink2();
}
void doubleClickEvent2() {
   digitalWrite(relay2, LOW);
   digitalWrite(ledpin2, LOW);
}
void holdEvent2() {
   digitalWrite(relay2, HIGH);
   digitalWrite(ledpin2, HIGH);
}
void longHoldEvent2() {
   digitalWrite(relay2, LOW);
   digitalWrite(ledpin2, LOW);
}
//=================================================
//  MULTI-CLICK:  One Button, Multiple Events

// Button timing variables
int debounce = 20;          // ms debounce period to prevent flickering when pressing or releasing the button
int DCgap = 250;            // max ms between clicks for a double click event
int holdTime = 1000;        // ms hold period: how long to wait for press+hold event
int longHoldTime = 3000;    // ms long hold period: how long to wait for press+hold event

// Button variables
boolean buttonVal = HIGH;   // value read from button
boolean buttonLast = HIGH;  // buffered value of the button's previous state
boolean DCwaiting = false;  // whether we're waiting for a double click (down)
boolean DConUp = false;     // whether to register a double click on next release, or whether to wait and click
boolean singleOK = true;    // whether it's OK to do a single click
long downTime = -1;         // time the button was pressed down
long upTime = -1;           // time the button was released
boolean ignoreUp = false;   // whether to ignore the button release because the click+hold was triggered
boolean waitForUp = false;        // when held, whether to wait for the up event
boolean holdEventPast = false;    // whether or not the hold event happened already
boolean longHoldEventPast = false;   // whether or not the long hold event happened already

//second set of button vars

boolean buttonVal2 = HIGH;   // value read from button
boolean buttonLast2 = HIGH;  // buffered value of the button's previous state
boolean DCwaiting2 = false;  // whether we're waiting for a double click (down)
boolean DConUp2 = false;     // whether to register a double click on next release, or whether to wait and click
boolean singleOK2 = true;    // whether it's OK to do a single click
long downTime2 = -1;         // time the button was pressed down
long upTime2 = -1;           // time the button was released
boolean ignoreUp2 = false;   // whether to ignore the button release because the click+hold was triggered
boolean waitForUp2 = false;        // when held, whether to wait for the up event
boolean holdEventPast2 = false;    // whether or not the hold event happened already
boolean longHoldEventPast2 = false;

// blink led for 10 on and 5 off mode


int blink1() {
    while ((digitalRead(buttonPin) == HIGH)&&(digitalRead(relay1) == HIGH)) {
      digitalWrite(ledpin1, HIGH);  
      delay(1000);                       
      digitalWrite(ledpin1, LOW);   
      delay(1000);
    }
  }



int blink2() {
    digitalWrite(ledpin2, HIGH);  
  delay(1000);                       
  digitalWrite(ledpin2, LOW);   
  delay(1000);  
}




int checkButton() {   
   int event = 0;
   buttonVal = digitalRead(buttonPin);
   // Button pressed down
   if (buttonVal == LOW && buttonLast == HIGH && (millis() - upTime) > debounce)
   {
       downTime = millis();
       ignoreUp = false;
       waitForUp = false;
       singleOK = true;
       holdEventPast = false;
       longHoldEventPast = false;
       if ((millis()-upTime) < DCgap && DConUp == false && DCwaiting == true)  DConUp = true;
       else  DConUp = false;
       DCwaiting = false;
   }
   // Button released
   else if (buttonVal == HIGH && buttonLast == LOW && (millis() - downTime) > debounce)
   {       
       if (not ignoreUp)
       {
           upTime = millis();
           if (DConUp == false) DCwaiting = true;
           else
           {
               event = 2;
               DConUp = false;
               DCwaiting = false;
               singleOK = false;
           }
       }
   }
   // Test for normal click event: DCgap expired
   if ( buttonVal == HIGH && (millis()-upTime) >= DCgap && DCwaiting == true && DConUp == false && singleOK == true && event != 2)
   {
       event = 1;
       DCwaiting = false;
   }
   // Test for hold
   if (buttonVal == LOW && (millis() - downTime) >= holdTime) {
       // Trigger "normal" hold
       if (not holdEventPast)
       {
           event = 3;
           waitForUp = true;
           ignoreUp = true;
           DConUp = false;
           DCwaiting = false;
           //downTime = millis();
           holdEventPast = true;
       }
       // Trigger "long" hold
       if ((millis() - downTime) >= longHoldTime)
       {
           if (not longHoldEventPast)
           {
               event = 4;
               longHoldEventPast = true;
           }
       }
   }
   buttonLast = buttonVal;
   return event;
}
   //second checkbutton

int checkButton2() {   
   int event2 = 0;
   buttonVal2 = digitalRead(buttonpin2);
   // Button pressed down
   if (buttonVal2 == LOW && buttonLast2 == HIGH && (millis() - upTime2) > debounce)
   {
       downTime2 = millis();
       ignoreUp2 = false;
       waitForUp2 = false;
       singleOK2 = true;
       holdEventPast2 = false;
       longHoldEventPast2 = false;
       if ((millis()-upTime2) < DCgap && DConUp2 == false && DCwaiting2 == true)  DConUp2 = true;
       else  DConUp2 = false;
       DCwaiting2 = false;
   }
   // Button released
   else if (buttonVal2 == HIGH && buttonLast2 == LOW && (millis() - downTime2) > debounce)
   {       
       if (not ignoreUp2)
       {
           upTime2 = millis();
           if (DConUp2 == false) DCwaiting2 = true;
           else
           {
               event2 = 2;
               DConUp2 = false;
               DCwaiting2 = false;
               singleOK2 = false;
           }
       }
   }
   // Test for normal click event: DCgap expired
   if ( buttonVal2 == HIGH && (millis()-upTime2) >= DCgap && DCwaiting2 == true && DConUp2 == false && singleOK2 == true && event2 != 2)
   {
       event2 = 1;
       DCwaiting2 = false;
   }
   // Test for hold
   if (buttonVal2 == LOW && (millis() - downTime2) >= holdTime) {
       // Trigger "normal" hold
       if (not holdEventPast2)
       {
           event2 = 3;
           waitForUp2 = true;
           ignoreUp2 = true;
           DConUp2 = false;
           DCwaiting2 = false;
           //downTime2 = millis();
           holdEventPast2 = true;
       }
       // Trigger "long" hold
       if ((millis() - downTime2) >= longHoldTime)
       {
           if (not longHoldEventPast2)
           {
               event2 = 4;
               longHoldEventPast2 = true;
           }
       }
   }
   buttonLast2 = buttonVal2;
   return event2;
}

It's an uno by inland. Also my relay1 and relay2 are outputs. My buttonPin is a pull down with a 10k on it. And my second button is a pull up. I have modified the code a bit in blink1 to allow me to end that particular loop, however while in that loop I cannot activate the second button at any time.

Hi,
So your buttonPin switches to gnd, and you have a 10K to 5v?
Your second button is a pull up switch, so has a pull down 10K?
Can you please post a copy of your circuit, in CAD or a picture of a hand drawn circuit in jpg, png?

Thanks.. Tom... :slight_smile:

Your code will become unstuck if you replace the "while loop" with an "if" condition. Then, if you use millis() for LED blink timing rather than delay(), your code will become much more responsive.

Sorry theyre both pull up circuits I did end up fixing that. I know its a bit rough but heres my drawing.

print screen

dlloyd:
Your code will become unstuck if you replace the "while loop" with an "if" condition. Then, if you use millis() for LED blink timing rather than delay(), your code will become much more responsive.

How do I go about replacing delay with millis. I looked it up and I don't quit understand.

Hi,
Your circuit diagram is a bit hard to make out.
Can you draw your buttons like this;
Button.jpg
And LED like this;
Diode2016.jpg

Your two relays are only connected by two wires, where is the gnd and/or Vcc connections?
Also link to data/spec of your relays

Can you post a picture of your project please?

Thanks… Tom… :slight_smile:

How do I go about replacing delay with millis. I looked it up and I don't quit understand.

A good example is in the IDE ... [File] [Examples] [02.Digital] BlinkWithoutDelay

I’m fairly certain its not my circuit, I can make this same circuit do anything I want with different code. This is definitely a coding issue. As per advice I used the IF statement instead of WHILE and MILLIS instead of DELAY. At this point I cannot get my loop to start at all. My relay will activate but my status led never goes HIGH.

#define buttonPin 9
#define buttonpin2 8
#define ledpin1 7          // digital output pin for LED 1
#define ledpin2 6
#define relay1 12
#define relay2 11

boolean ledState1 = LOW;
int ledState2 = LOW;

unsigned long previousMillis1 = 0; 
unsigned long previousMillis2 = 0; 

const long interval = 1000;



void setup() {
   // Set button input pin
   pinMode(buttonPin, INPUT);
   pinMode(buttonpin2, INPUT);
   pinMode(relay1, OUTPUT);
   pinMode(relay2, OUTPUT);
   pinMode(ledpin1, OUTPUT);  
   pinMode(ledpin2, OUTPUT);

   

   

   

}
 

void loop() {
   unsigned long currentMillis = millis();
   // Get button event and act accordingly
   int b = checkButton();
   if (b == 1) clickEvent();
   if (b == 2) doubleClickEvent();
   if (b == 3) holdEvent();
   if (b == 4) longHoldEvent();

   int b2 = checkButton2();
   if (b2 == 1) clickEvent2();
   if (b2 == 2) doubleClickEvent2();
   if (b2 == 3) holdEvent2();
   if (b2 == 4) longHoldEvent2();
}

//=================================================
// Events to trigger

void clickEvent() {
   digitalWrite(relay1, HIGH);
   blink1();                     // this is where the led blink function is called.
}
void doubleClickEvent() {
   digitalWrite(relay1, LOW);
   digitalWrite(ledpin1, LOW);
}
void holdEvent() {
   digitalWrite(relay1, HIGH);
   digitalWrite(ledpin1, HIGH);
}
void longHoldEvent() {
   digitalWrite(relay1, LOW);
   digitalWrite(ledpin1, LOW);
}


void clickEvent2() {       //click event 2 for second button
   digitalWrite(relay2, HIGH);
   blink2();
}
void doubleClickEvent2() {
   digitalWrite(relay2, LOW);
   digitalWrite(ledpin2, LOW);
}
void holdEvent2() {
   digitalWrite(relay2, HIGH);
   digitalWrite(ledpin2, HIGH);
}
void longHoldEvent2() {
   digitalWrite(relay2, LOW);
   digitalWrite(ledpin2, LOW);
}
//=================================================
//  MULTI-CLICK:  One Button, Multiple Events

// Button timing variables
int debounce = 20;          // ms debounce period to prevent flickering when pressing or releasing the button
int DCgap = 250;            // max ms between clicks for a double click event
int holdTime = 1000;        // ms hold period: how long to wait for press+hold event
int longHoldTime = 3000;    // ms long hold period: how long to wait for press+hold event

// Button variables
boolean buttonVal = HIGH;   // value read from button
boolean buttonLast = HIGH;  // buffered value of the button's previous state
boolean DCwaiting = false;  // whether we're waiting for a double click (down)
boolean DConUp = false;     // whether to register a double click on next release, or whether to wait and click
boolean singleOK = true;    // whether it's OK to do a single click
long downTime = -1;         // time the button was pressed down
long upTime = -1;           // time the button was released
boolean ignoreUp = false;   // whether to ignore the button release because the click+hold was triggered
boolean waitForUp = false;        // when held, whether to wait for the up event
boolean holdEventPast = false;    // whether or not the hold event happened already
boolean longHoldEventPast = false;   // whether or not the long hold event happened already

//second set of button vars

boolean buttonVal2 = HIGH;   // value read from button
boolean buttonLast2 = HIGH;  // buffered value of the button's previous state
boolean DCwaiting2 = false;  // whether we're waiting for a double click (down)
boolean DConUp2 = false;     // whether to register a double click on next release, or whether to wait and click
boolean singleOK2 = true;    // whether it's OK to do a single click
long downTime2 = -1;         // time the button was pressed down
long upTime2 = -1;           // time the button was released
boolean ignoreUp2 = false;   // whether to ignore the button release because the click+hold was triggered
boolean waitForUp2 = false;        // when held, whether to wait for the up event
boolean holdEventPast2 = false;    // whether or not the hold event happened already
boolean longHoldEventPast2 = false;

// blink led for 10 on and 5 off mode

unsigned long currentMillis = millis();
    


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

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

    // set the LED with the ledState of the variable:
    digitalWrite(ledpin1, ledState1);
  }
}
  



int blink2() {
    digitalWrite(ledpin2, HIGH);  
  delay(1000);                       
  digitalWrite(ledpin2, LOW);   
  delay(1000);  
}




int checkButton() {   
   int event = 0;
   buttonVal = digitalRead(buttonPin);
   // Button pressed down
   if (buttonVal == LOW && buttonLast == HIGH && (millis() - upTime) > debounce)
   {
       downTime = millis();
       ignoreUp = false;
       waitForUp = false;
       singleOK = true;
       holdEventPast = false;
       longHoldEventPast = false;
       if ((millis()-upTime) < DCgap && DConUp == false && DCwaiting == true)  DConUp = true;
       else  DConUp = false;
       DCwaiting = false;
   }
   // Button released
   else if (buttonVal == HIGH && buttonLast == LOW && (millis() - downTime) > debounce)
   {

I have redacted parts of code to stay under the post limit.

You probably mean this ...

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

   // Get button event and act accordingly
   int b = checkButton();
   if (b == 1) clickEvent();
   if (b == 2) doubleClickEvent();
   if (b == 3) holdEvent();
   if (b == 4) longHoldEvent();

   int b2 = checkButton2();
   if (b2 == 1) clickEvent2();
   if (b2 == 2) doubleClickEvent2();
   if (b2 == 3) holdEvent2();
   if (b2 == 4) longHoldEvent2();
}

... to be ...

void loop() {
   currentMillis = millis();

   // Get button event and act accordingly
   const uint8_t b = checkButton();
   if      (b == 1) clickEvent();
   else if (b == 2) doubleClickEvent();
   else if (b == 3) holdEvent();
   else if (b == 4) longHoldEvent();

   const uint8_t b2 = checkButton2();
   if      (b2 == 1) clickEvent2();
   else if (b2 == 2) doubleClickEvent2();
   else if (b2 == 3) holdEvent2();
   else if (b2 == 4) longHoldEvent2();
}

Unfortunately that made no difference. Guys I have got to be missing something serious here, something is breaking my code or isn’t setup right. I’ve been at this for four hours and have made no progress.

Post EVERYTHING you currently have!

Personal opinion, but easier for me to read at least -

void loop()
{
    // NOTE: write to global, not local, as per your last posted fragment
    currentMillis = millis();

    switch ( checkButton() )
    {
        case 1: clickEvent();           break;
        case 2: doubleClickEvent();     break;
        case 3: holdEvent();            break;
        case 4: longHoldEvent();        break;
    }

    switch ( checkButton2() )
    {
        casw 1: clickEvent2();          break;
        casw 2: doubleClickEvent2();    break;
        casw 3: holdEvent2();           break;
        casw 4: longHoldEvent2();       break;
    }
}

lloyddean:
Personal opinion, but easier for me to read at least -

void loop()

{
    // NOTE: write to global, not local, as per your last posted fragment
    currentMillis = millis();

switch ( checkButton() )
    {
        case 1: clickEvent();          break;
        case 2: doubleClickEvent();    break;
        case 3: holdEvent();            break;
        case 4: longHoldEvent();        break;
    }

switch ( checkButton2() )
    {
        casw 1: clickEvent2();          break;
        casw 2: doubleClickEvent2();    break;
        casw 3: holdEvent2();          break;
        casw 4: longHoldEvent2();      break;
    }
}

I like that what is this called?

Hi,
Its called "switch case" helps fix problems with lots of if.. ..else..

https://www.arduino.cc/en/Reference/SwitchCase

Tom... :slight_smile: