Pages: 1 [2] 3   Go Down
Author Topic: "blink without delay" style routine almost working  (Read 2436 times)
0 Members and 1 Guest are viewing this topic.
Leighton Buzzard, UK
Offline Offline
Edison Member
*
Karma: 21
Posts: 1339
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

bill is right though
scrap the states just set the LEDs
and you do need a pullup on the switch pin

I'll glare at your "requirements spec"
Logged

there are only 10 types of people
them that understands binary
and them that doesn't

Wales
Offline Offline
Full Member
***
Karma: 0
Posts: 246
Don't take things too seriously
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

bill is right though
scrap the states just set the LEDs
and you do need a pullup on the switch pin

I'll glare at your "requirements spec"

Here's my current code

Code:
#include <Bounce.h>

#define ledPin 8      // the number of the LED pin
#define buttonPin 2 // the number of the button pin
#define ledPin2 9 // the number of the button pin

int buttonState = LOW; // used to set the button state
int LedStateConstantfor10s = LOW; // used to set the LED state
int LedStateFlash = LOW;         // used to set the LED state if LED was already on
long interval1 = 900; // time to keep the LED on for (5s)
long interval2 = 5000; // time to keep the LED on for (0.9s)
long previousMillis = 0; // will store last time LED was updated
long previousMillis2 = 0;       // will store last time LED was updated

// Instantiate a Bounce object with a 50 millisecond debounce time
Bounce button = Bounce( buttonPin,50 );

void setup()
{
  pinMode(buttonPin,INPUT);     
  pinMode(ledPin,OUTPUT);     
  pinMode(ledPin2,OUTPUT);     
}

void loop()
{
  button.update ( ); // Update the debouncer
  int buttonvalue = button.read(); // Get the update value

  if (buttonvalue == LOW && LedStateConstantfor10s == LOW) // If the button's pressed and LED is not on
  {
    LedStateConstantfor10s = HIGH;
    LedStateFlash = LOW;
    previousMillis = millis();
  }

  else if (buttonvalue == LOW && LedStateConstantfor10s == HIGH) // if the button's pressed and LED is on
  {
    LedStateConstantfor10s = LOW;
    LedStateFlash = HIGH;
    previousMillis2 = millis();
  }

  if      (LedStateConstantfor10s == HIGH)  // if the LED status has been set to on
  {                        
    digitalWrite(ledPin, HIGH);
  }

  if      (LedStateFlash == HIGH)  // if the LED status has been set to flash
  {                
    digitalWrite(ledPin2, HIGH); // Blink Code to go here.
  }

  if (LedStateConstantfor10s==LOW && LedStateFlash==LOW)   
  {
    digitalWrite(ledPin, LOW);
    digitalWrite(ledPin2, LOW);
  }

  if      (previousMillis + interval1 < millis()) // If the predefined interval has elapsed for the first button press
  {
    LedStateConstantfor10s = LOW;
  }

  if      (previousMillis2 + interval2 < millis()) // If the predefined interval has elapsed for the second button press
  {       
    LedStateFlash = LOW;
  }
}

What do you mean you'll glare at my spec? You've lost me here mate smiley

I do have a pull up on the switch pin. And software debouncing. There's never any issue with the device reading false positives or negatives from the button.

I'll try to scrap the states. I implemented them because I thought I was advised it was the better way to do it. I think I was anyway!
Logged

New Jersey
Offline Offline
Faraday Member
**
Karma: 72
Posts: 3761
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I'd suggest you keep the states - you'll need at least one to control the flashing. You can vary it between HIGH and LOW according to your timing logic and use it directly - digitalWrite(ledstate2). As to the names, I'm not aware of any naming convention docs in this context, just try to name it to represent what it is - StateOfFlashingLed perhaps. In a more real world sketch, it might be MashOvertempAlarmLed for example.
Logged

Leighton Buzzard, UK
Offline Offline
Edison Member
*
Karma: 21
Posts: 1339
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

sorry I meant "I will study with interest in hope of seeing a light at the end of the tunnel, hopefully without a train attached to it!"
Logged

there are only 10 types of people
them that understands binary
and them that doesn't

Wales
Offline Offline
Full Member
***
Karma: 0
Posts: 246
Don't take things too seriously
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

sorry I meant "I will study with interest in hope of seeing a light at the end of the tunnel, hopefully without a train attached to it!"

Cheers smiley

It's obviously me who has cocked up somewhere. God knows where. Who'd have thought I could cause so much chaos in such supposedly simple code!

Appreciate it.
Logged

Wales
Offline Offline
Full Member
***
Karma: 0
Posts: 246
Don't take things too seriously
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I'd suggest you keep the states - you'll need at least one to control the flashing. You can vary it between HIGH and LOW according to your timing logic and use it directly - digitalWrite(ledstate2). As to the names, I'm not aware of any naming convention docs in this context, just try to name it to represent what it is - StateOfFlashingLed perhaps. In a more real world sketch, it might be MashOvertempAlarmLed for example.

I've amended my code to this:

Code:
#include <Bounce.h>

#define ledPin 8          // the number of the LED pin
#define ledPin2 9     // the number of the button pin
#define buttonPin 2     // the number of the button pin

int buttonState = LOW;              // used to set the button state
int LedStateConstantfor10s = LOW;   // used to set the LED state
int LedStateFlash = LOW;     // used to set the LED state if LED was already on
long interval1 = 5000;     // time to keep the LED on for (5s)
long interval2 = 1000;     // time to keep the LED on for (1s)
long previousMillis = 0;     // will store last time LED was updated
long previousMillis2 = 0;           // will store last time LED was updated

// Instantiate a Bounce object with a 50 millisecond debounce time
Bounce button = Bounce( buttonPin,50 );

void setup()
{
  pinMode(buttonPin,INPUT);     
  pinMode(ledPin,OUTPUT);     
  pinMode(ledPin2,OUTPUT);     
}

void loop()
{
  button.update ( ); // Update the debouncer
  int buttonvalue = button.read(); // Get the update value

  if (buttonvalue == LOW && LedStateConstantfor10s == LOW) // If the button's pressed and LED is not on
  {
    LedStateConstantfor10s = HIGH;
    LedStateFlash = LOW;
    previousMillis = millis();
  }

  else if (buttonvalue == LOW && LedStateConstantfor10s == HIGH) // if the button's pressed and LED is on
  {
    LedStateConstantfor10s = LOW;
    LedStateFlash = HIGH;
    previousMillis2 = millis();
  }

  if      (LedStateConstantfor10s == HIGH)  // if the LED status has been set to on
  {                        
    digitalWrite(ledPin, LedStateConstantfor10s);
  }

  if      (LedStateFlash == HIGH)  // if the LED status has been set to flash
  {                
    digitalWrite(ledPin2, LedStateFlash); // Blink Code to go here.
  }

  if (LedStateConstantfor10s==LOW && LedStateFlash==LOW)   
  {
    digitalWrite(ledPin, LedStateConstantfor10s);
    digitalWrite(ledPin2, LedStateFlash);
  }

  if      (previousMillis + interval1 < millis()) // If the predefined interval has elapsed for the first button press
  {
    LedStateConstantfor10s = LOW;
  }

  if      (previousMillis2 + interval2 < millis()) // If the predefined interval has elapsed for the second button press
  {       
    LedStateFlash = LOW;
  }
}

More sensible naming, I hope. Or at least in part. And I've changed to digitalwrite. Still doesn't work, mind!!
Logged

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 653
Posts: 50881
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I'd recommend that you stop trying to combine the switch logic and the LED state logic in one if statement, like this:
Code:
 if (nurseryvalue == LOW && LedState1 == LOW)
The reason I say this is because your requirement says that you want the LED to turn on for 10 seconds if the switch is pressed.. Then, if the switch is pressed again, the LED should start flashing for the remaining time. What to do with the LED depends on the switch being pressed. So first decide if the switch is pressed, then, decide what to do. Then, completely independently of the switch state, implement the "what to do" stuff.

Try something like this:
Code:
enum LedStates { Off, On, Blinking };

unsigned long onTime = 0;
unsigned long onInterval = 10000; // On for 10 seconds
unsigned long blinkTime = 0;
unsigned long blinkInterval = 500; // Blink on/off time

LedStates ledState = Off;

int ledPinState = LOW;

void setup()
{
   // Do whatever...
}

void loop()
{
   button.update ( ); // Update the debouncer
   int buttonvalue = button.read(); // Get the update value

   // Determine if a state change is needed,
   // and, if so, what the new state should be
   if(nurseryvalue == LOW)
   {
      // A state change is needed
      if(ledState == Off) // If not on, turn on and record when
      {
         ledState = On;
         onTime = millis();
         ledPinState = HIGH;
         digitalWrite(ledPin, ledPinState);
      }
      else if(ledState == On)
      {
         ledState = Blinking;
         blinkTime = currTime;
         ledPinState = LOW;
         digitalWrite(ledPin, ledPinState);
      }
   }

   // What should we do now? Depends on the state and the time
   unsigned long currTime = millis();
   switch(ledState)
   {
      case On:
         if(currTime - onTime >= onInterval) // If the LED has been on long enough, turn it off
         {
            onTime = 0;
            digitalWrite(ledPin, LOW);
            ledState = Off;
         }
         break;

      case Blinking:
         if(currTime - onTime >= onInterval) // If the LED has been blinking long enough, turn it off
         {
            onTime = 0;
            digitalWrite(ledPin, LOW);
            ledState = Off;
         }

         if(ledState == Blinking) // If LED should still be blinking
         {
             if(currTine - blinkTime >= blinkInterval) // Is it time to toggle the LED?
             {
                ledPinState = !ledPinState; // Toggle it
                digitalWrite(ledPin, ledPinState);
                blinkTime = currTime;
             }
         }
         break;
   }
}
Logged

Leighton Buzzard, UK
Offline Offline
Edison Member
*
Karma: 21
Posts: 1339
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

ok I tried this
Code:
#include <Bounce.h>

#define ledPin1 8      // the number of the LED pin
#define buttonPin 2 // the number of the button pin
#define ledPin2 9 // the number of the button pin

int buttonState = LOW; // used to set the button state
int LedState1 = LOW; // used to set the LED state
int LedState2 = LOW;         // used to set the LED state if LED was already on
long interval1 = 5000; // time to keep the LED on for (2s)
long interval2 = 900; // time to keep the LED on for (0.9s)
long previousMillis1 = 0; // will store last time LED was updated
long previousMillis2 = 0;       // will store last time LED was updated

// Instantiate a Bounce object with a 20 millisecond debounce time
Bounce nursery = Bounce( buttonPin, 100);
int nurseryvalue;

enum {OFF, ON, FLASH};

void setup()
{
  pinMode(buttonPin,INPUT);     
  pinMode(ledPin1,OUTPUT);     
  pinMode(ledPin2,OUTPUT);     
  digitalWrite(ledPin1, OFF);
  digitalWrite(ledPin2, OFF);
  LedState1 = OFF;
  LedState2 = OFF;
 
  Serial.begin(19200);
}

void loop()
{
  nursery.update ( ); // Update the debouncer
  nurseryvalue = nursery.read(); // Get the update value

  unsigned long currentMillis = millis();

  if (nurseryvalue == LOW)
  {
    // button pressed
    //===============
    Serial.println("button");
    if (LedState1 == OFF)
    {
      // the button's pressed and LED is OFF
      // turn it ON
      //====================================
      digitalWrite(ledPin1, ON);
      LedState1 = ON;
      digitalWrite(ledPin2, OFF);
      LedState2 = OFF;
      previousMillis1 = currentMillis;
    }
    else if (LedState1 == ON)
    {
      // if the button's pressed and LED is ON
      // start FLASHing
      //======================================
      digitalWrite(ledPin1, OFF);
      LedState1 = FLASH;
      digitalWrite(ledPin2, ON);
      LedState2 = ON;
      previousMillis2 = currentMillis;
    }
  }

  if (previousMillis1 + interval1 < currentMillis)
  {
    // If the predefined interval has elapsed for the first button press
    // turn led OFF
    //==================================================================
    digitalWrite(ledPin1, OFF);
    LedState1 = OFF;
    digitalWrite(ledPin2, OFF);
    LedState2 = OFF;
  }

  if (LedState1 == FLASH && previousMillis2 + interval2 < currentMillis)
  {       
    // If the predefined interval has elapsed for the second button press
    // toggle flash
    //===================================================================
    digitalWrite(ledPin1, !digitalRead(ledPin1));
    previousMillis2 = currentMillis;
  }
}

it goes straight to blink
serial monitor show multiple button presses???
Logged

there are only 10 types of people
them that understands binary
and them that doesn't

Leighton Buzzard, UK
Offline Offline
Edison Member
*
Karma: 21
Posts: 1339
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

aha cracked it
Code:
#include <Bounce.h>

#define ledPin1 8      // the number of the LED pin
#define buttonPin 2 // the number of the button pin
#define ledPin2 9 // the number of the button pin

int buttonState = LOW; // used to set the button state
int LedState1 = LOW; // used to set the LED state
int LedState2 = LOW;         // used to set the LED state if LED was already on
long interval1 = 5000; // time to keep the LED on for (2s)
long interval2 = 900; // time to keep the LED on for (0.9s)
long previousMillis1 = 0; // will store last time LED was updated
long previousMillis2 = 0;       // will store last time LED was updated

// Instantiate a Bounce object with a 20 millisecond debounce time
Bounce nursery = Bounce( buttonPin, 100);
int nurseryvalue;

enum {OFF, ON, FLASH};

void setup()
{
  pinMode(buttonPin,INPUT);     
  pinMode(ledPin1,OUTPUT);     
  pinMode(ledPin2,OUTPUT);     
  digitalWrite(ledPin1, OFF);
  digitalWrite(ledPin2, OFF);
  LedState1 = OFF;
  LedState2 = OFF;
 
  Serial.begin(19200);
}

void loop()
{
  nursery.update ( ); // Update the debouncer
  nurseryvalue = nursery.read(); // Get the update value

  unsigned long currentMillis = millis();

  if (nurseryvalue == LOW)
  {
    if (buttonState == OFF)
    {
      // button pressed
      //===============
      Serial.println("button");
      buttonState = ON;
      if (LedState1 == OFF)
      {
        // the button's pressed and LED is OFF
        // turn it ON
        //====================================
        digitalWrite(ledPin1, ON);
        LedState1 = ON;
        digitalWrite(ledPin2, OFF);
        LedState2 = OFF;
        previousMillis1 = currentMillis;
      }
      else if (LedState1 == ON)
      {
        // if the button's pressed and LED is ON
        // start FLASHing
        //======================================
        digitalWrite(ledPin1, OFF);
        LedState1 = FLASH;
        digitalWrite(ledPin2, ON);
        LedState2 = ON;
        previousMillis2 = currentMillis;
      }
    }
  }
  else
  {
    buttonState = OFF;
  }

  if (previousMillis1 + interval1 < currentMillis)
  {
    // If the predefined interval has elapsed for the first button press
    // turn led OFF
    //==================================================================
    digitalWrite(ledPin1, OFF);
    LedState1 = OFF;
    digitalWrite(ledPin2, OFF);
    LedState2 = OFF;
  }

  if (LedState1 == FLASH && previousMillis2 + interval2 < currentMillis)
  {       
    // If the predefined interval has elapsed for the second button press
    // toggle flash
    //===================================================================
    digitalWrite(ledPin1, !digitalRead(ledPin1));
    previousMillis2 = currentMillis;
  }
}
Logged

there are only 10 types of people
them that understands binary
and them that doesn't

Wales
Offline Offline
Full Member
***
Karma: 0
Posts: 246
Don't take things too seriously
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Many, many MANY thanks to all of who helped with this!! The code is at least now finally working in a 90% "to spec" (hehe!) way.

I have made some small alterations (mainly naming):-

Code:
#include <Bounce.h>

#define ledPin1 8      // the number of the LED pin
#define buttonPin 2 // the number of the button pin

int buttonState = LOW; // used to set the button state
int LedState1 = LOW; // used to set the LED state
int LedState2 = LOW;         // used to set the LED state if LED was already on
long LedOnDuration = 10000; // time to keep the LED on for (10s)
long FlashRate = 1000; // time to keep the LED on for (1s)
long previousMillis1 = 0; // will store last time LED was updated for press 1
long previousMillis2 = 0;       // will store last time LED was updated for press 2

// Instantiate a Bounce object with a 20 millisecond debounce time
Bounce nursery = Bounce( buttonPin, 100);
int nurseryvalue;

enum {
  OFF, ON, FLASH};

void setup()
{
  pinMode(buttonPin,INPUT);     
  pinMode(ledPin1,OUTPUT);     
  digitalWrite(ledPin1, OFF);
  LedState1 = OFF;
  LedState2 = OFF;
  Serial.begin(19200);
}

void loop()
{
  nursery.update ( ); // Update the debouncer
  nurseryvalue = nursery.read(); // Get the update value

  unsigned long currentMillis = millis();

  if (nurseryvalue == HIGH)
  {
    if (buttonState == OFF)
    {
      // button pressed
      //===============
      Serial.println("button");
      buttonState = ON;
      if (LedState1 == OFF)
      {
        // the button's pressed and LED is OFF
        // turn it ON
        //====================================
        digitalWrite(ledPin1, ON);
        LedState1 = ON;
        previousMillis1 = currentMillis;
      }
      else if (LedState1 == ON)
      {
        // if the button's pressed and LED is ON
        // start FLASHing
        //======================================
        digitalWrite(ledPin1, OFF);
        LedState1 = FLASH;
        previousMillis2 = currentMillis;
      }
    }
  }
  else
  {
    buttonState = OFF;
  }

  if (previousMillis1 + LedOnDuration < currentMillis)
  {
    // If the predefined interval has elapsed for the first button press
    // turn led OFF
    //==================================================================
    digitalWrite(ledPin1, OFF);
    LedState1 = OFF;
  }

  if (LedState1 == FLASH && previousMillis2 + FlashRate < currentMillis)
  {       
    // If the predefined interval has elapsed for the second button press
    // toggle flash
    //===================================================================
    digitalWrite(ledPin1, !digitalRead(ledPin1));
    previousMillis2 = currentMillis;
  }
}

I have two questions

1) If I wanted the flashing to continue for 10s from the button being pressed for a second time, how would I do that? I tried to update currentmillis, but this didn't affect anything
2) is adding a second LED and a second button child's play?

THANKS again  smiley-razz  smiley-grin  smiley-mr-green
Logged

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 653
Posts: 50881
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
If I wanted the flashing to continue for 10s from the button being pressed for a second time, how would I do that?
What variable holds the time that the LED was turned on? It isn't currentmillis. It is that variable that needs a new value (the time the switch is pressed again).

Quote
is adding a second LED and a second button child's play?
For some, yes. Once you understand exactly what the code is doing, and can make the change request above, dealing with a second switch/pin, a second LED/pin, and a second set of times is easy.

Perhaps you might even consider making a function that took the 4 values listed above as input...
Logged

Leighton Buzzard, UK
Offline Offline
Edison Member
*
Karma: 21
Posts: 1339
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

well your homework is to propose a solution to each of those problems
the committee will give you marks for style and accuracy!

hint1 you need to update previousMillis not currentMillis
hint2 you could just repeat everything for the 2nd LED/button, but there must be a neater way
hint3 maybe a function with a parameter or two

now go do your homework or no supper for you smiley
Logged

there are only 10 types of people
them that understands binary
and them that doesn't

Wales
Offline Offline
Full Member
***
Karma: 0
Posts: 246
Don't take things too seriously
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
is adding a second LED and a second button child's play?
For some, yes. Once you understand exactly what the code is doing, and can make the change request above, dealing with a second switch/pin, a second LED/pin, and a second set of times is easy.

Perhaps you might even consider making a function that took the 4 values listed above as input...

Ok, good. Because I do understand 90% of it. So I should easily be able to add the variables and change the code to make it work for more buttons/LEDs.

As for making a function, that would be fantastic. But it's beyond me. Maybe one exists? I haven't found it in all my Googling. This code is making a mess out of my loop, but not as badly as if you hadn't pointed out autoformat. I wish I could split the button stuff onto seperate tabs, but I gather that isn't what they're for.
Logged

Wales
Offline Offline
Full Member
***
Karma: 0
Posts: 246
Don't take things too seriously
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

well your homework is to propose a solution to each of those problems

hint1 you need to update previousMillis not currentMillis
hint2 you could just repeat everything for the 2nd LED/button, but there must be a neater way
hint3 maybe a function with a parameter or two

I got it to work. Thanks smiley

I wish there was a function for this, but I haven't found one - and I surely can't write one.

Thanks again.
Logged

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 653
Posts: 50881
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
I wish I could split the button stuff onto seperate tabs, but I gather that isn't what they're for.
No, that's what functions are for.

A function is really nothing more than a named block of code, taking some input, and producing, maybe, some output. You define the output type, the name of the function, and the input type and name to refer to the data, for each bit of input data.

Code:
void monitorSwitch(Bounce &switchObj, int LEDPin, int &prevTime, int &interval)
{
   switchObj.update();
   int state = switchObj.read();

   // Do some more stuff
}

Then, call the function:
Code:
void loop()
{
   monitorSwitch(nursery, ledPin1, previousMillis1, LedOnDuration);
}
Logged

Pages: 1 [2] 3   Go Up
Jump to: