Go Down

Topic: Consistent flash on button press (Button + FlashWithoutDelay) (Read 888 times) previous topic - next topic

Jebimasta

Jun 02, 2013, 10:37 pm Last Edit: Jun 02, 2013, 11:21 pm by Jebimasta Reason: 1
Hey guys,

I'm attempting to build a strobe controller using the arduino uno.
I've got 10 buttons all wired up in a pullup fashion onto 10 of the digital inputs, and I'm using pin 13 as the output.

I've combined the button and flash without delay examples and can get the buttons to cause the LED to flash at the rates I need.
Below is a simplified code for only 1 button.

Code: [Select]
/* Button */

// constants won't change. They're used here to
// set pin numbers:
const int buttonPin = 2;     // the number of the pushbutton pin
const int ledPin =  13;      // the number of the LED pin

// variables will change:
int buttonState = 0;         // variable for reading the pushbutton status
int ledState = LOW;             // ledState used to set the LED
long previousMillis = 0;
long interval = 1000;
void setup() {
 // initialize the LED pin as an output:
 pinMode(ledPin, OUTPUT);      
 // initialize the pushbutton pin as an input:
 pinMode(buttonPin, INPUT);    
}

void loop(){
 // read the state of the pushbutton value:
 buttonState = digitalRead(buttonPin);
 // check if the pushbutton is pressed.
 // if it is, the buttonState is HIGH:
 if (buttonState == HIGH) {
 unsigned long currentMillis = millis();    
   // turn LED on:    
     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 (ledState == LOW)
     ledState = HIGH;
   else
     ledState = LOW;

   // set the LED with the ledState of the variable:
   digitalWrite(ledPin, ledState);
 }
     
 }
 else {
   // turn LED off:
   digitalWrite(ledPin, LOW);
 }
}


This code seems to rely on When during the cycle you press the button. There can be a delay between hitting the button and seeing a flash, especially with the longer intervals.

I've never done any coding before and I'm trying to work out a way to reset the timer when a button is released so the next time a button is pressed it will always start the cycle again.

I'm taking it step by step but my ultimate goal is to make a momentary button press make the LED on pin 13 flash a set number of times. Also the possibility of using the 10th button as a modifier so I can also have the other 9 act as hold to flash as well.

Thanks.

PaulS

Code: [Select]
    // if the LED is off turn it on and vice-versa:
    if (ledState == LOW)
      ledState = HIGH;
    else
      ledState = LOW;

Or,

Quote
ledState = !ledState; // Toggle the state

Same great taste; less filling.

Quote
This code seems to rely on When during the cycle you press the button.

Because it Does. Why did you write it That way?

Random capitals don't add much, do they?

It's hard to tell what that code is supposed to be doing, since it is not indented correctly. Use Tools + Auto Format to fix that. But, it appears that the else clause is hung off the wrong if.

Jebimasta

#2
Jun 03, 2013, 12:18 am Last Edit: Jun 03, 2013, 01:08 am by Jebimasta Reason: 1
Thanks for your reply  :)

I'm only just beginning to learn how to code. Firstly I used "delay", which did exactly what I wanted it to do in the most basic sense.

Everything I read suggested that using millis() was a better way of coding timed events, and pointed to the BlinkWithoutDelay example.
So what I did was combine the button and BlinkWithoutDelay examples only to find that it doesn't give the response I was after.
What I would like is when a button is held, the LED will flash at an assigned interval. When the button is released the flashing stops, and if it or another button is pressed at some time (less than the interval assigned to that button, I think this will have something to do with how I write the code, but I'm still working on that) after release, the LED would instantaneously begin to flash.

UPDATED CODE, included the state toggle and tidied it up a bit. The else clause seems to be functioning correctly because the LED is off when no buttons are pressed.

Code: [Select]
/* Strobe controller */

// set pin numbers:
const int buttonPin = 2;     // the number of the pushbutton pin
const int ledPin =  13;      // the number of the LED pin
// variables
int buttonState = 0;         // variable for reading the pushbutton status
int ledState = LOW;          // ledState used to set the LED on or off
long previousMillis = 0;     // last time LED was updated
long interval = 250;         // half of 1 on/off cycle time
void setup() {
  pinMode(ledPin, OUTPUT);   // initialize the LED pin as an output   
  pinMode(buttonPin, INPUT); // initialize the pushbutton pin as an input   
}
void loop(){
  buttonState = digitalRead(buttonPin);              // read the state of the pushbutton value
  if (buttonState == HIGH) {                         // check if the pushbutton is pressed, if it is, the buttonState is HIGH
    unsigned long currentMillis = millis();   
    if(currentMillis - previousMillis > interval) {  // check if it's time to change LED state     
      previousMillis = currentMillis;                // save the time you blinked the LED
      ledState = !ledState;                          // if the LED is off turn it on and vice-versa:
      digitalWrite(ledPin, ledState);                // set the LED with the ledState of the variable:
    }
  }
  else {                         // if no button is pressed
    digitalWrite(ledPin, LOW);   // turn LED off
  }
}


I would like to learn how to accomplish this without using delays, and I'm still trying to work out the logic of what I want, but I'm stumped for now.

Thanks again

Arrch


I would like to learn how to accomplish this without using delays, and I'm still trying to work out the logic of what I want, but I'm stumped for now.


To get multiple switches working, separate the logic. The switch logic should only update the interval variable (and maybe a boolean that is set when at least one button is pressed).

Then, independent of the switch logic, you should have the BlinkWithoutDelay logic, almost exactly as it is in the example. Perhaps the only minor difference would be a check for the aforementioned boolean.

Jebimasta

#4
Jun 03, 2013, 02:48 am Last Edit: Jun 03, 2013, 02:50 am by Jebimasta Reason: 1
Thanks for your reply,

having the different switches change the interval is a good call and will cut the code down considerably.

I've currently got it in the loop function like:

Code: [Select]
if (digitalRead(buttonPin1) == HIGH)
 {
   interval = 250;
 }
 else if (digitalRead(buttonPin2) == HIGH)
 {
   interval = 125;
 }


I don't understand what you mean by seperating the logic though, or how to relate the boolean to the buttons.

My housemate has a book on C, think I'd better get reading.

Arrch


I don't understand what you mean by seperating the logic though


Different if/else if/else blocks

Quote
or how to relate the boolean to the buttons.


One way would be to assume something is pressed and if you don't fine one pressed, set the boolean to false.

Jebimasta

Ok so I think I've made the suggested changes, pressing a button will cause the LED to flash at a given rate, however, to begin with the LED is on, not off, and when no button is pressed it keeps flashing.

Code: [Select]
//constants
const int ledPin =  13;     
const int buttonPin1 = 2;
const int buttonPin2 = 3;
// Variables
boolean pressed = false;     
long previousMillis = 0;
long interval = 0;
int strobeState = LOW;
void setup() {
  // assign pinmodes
  pinMode(ledPin, OUTPUT);
  pinMode(buttonPin1, INPUT);
  pinMode(buttonPin2, INPUT);
  // start with the LED off
  digitalWrite(ledPin, LOW);
}

void loop() {
  unsigned long currentMillis = millis();
  // Check if a button is pressed
  if (digitalRead(buttonPin1) == HIGH)
  {
    pressed = true;
    interval = 250;
  }
  else if (digitalRead(buttonPin2) == HIGH)
  {
    pressed = true;
    interval = 125;
  }
  else
  {
    pressed = false;     // if no button is pressed set boolean to false
  }
  // if a button is pressed, begin flashing
  if (pressed = true)
  {
    if(currentMillis - previousMillis > interval) {
      // save the last time you blinked the LED
      previousMillis = currentMillis;
      strobeState = !strobeState;     //toggle
      digitalWrite(ledPin, strobeState);
    }
  }
  else
  {
    digitalWrite(ledPin, LOW);
  }
}



I think this code should:

  • start with the LED off, and assuming no button is pressed (wasn't sure about assuming the opposite)

  • Checks if a button is pressed

  • If a button is pressed sets the boolean to true and assigns an inverval value

  • if none of the above are valid then it sets the boolean to false

  • when boolean is true it starts flashing

  • otherwise the boolean is false and the LED turns off



I presume there is an error in this logic, or it could be a problem with the wiring, I certainly don't see how the LED is on before any buttons are pressed.  :~

PaulS

Quote
I presume there is an error in this logic

At least one:
Code: [Select]
  if (pressed = true)
Assigning the value of true to pressed in this statement doesn't seem to be what you want to do.

Jebimasta

ahhh yes!

Code: [Select]
if (pressed == true)

I'm getting there, I've got multiple buttons all working and flashing at the desired rates, but I'm still getting an unwanted delay before the flashing starts on presses after the first press.
The delay varies and I know it's a product of the blink without delay code and how it counts. I really can't get my head around either altering this code or just simply using a different method of flashing.


PaulS

Quote
I really can't get my head around either altering this code or just simply using a different method of flashing.

Try resetting previousMillis to 0 when pressed is set to true. No sense waiting to start a new sequence.

Jebimasta

Now the LED doesn't flash but just turn on, and it's not at full brightness. I'm guessing the interval value is now being ignored because currentMillis - previousMillis is always greater than interval, so the LED is being switched as fast as the code is running (presenting like PWM).
I think the problem is telling the LED to be on when the button is pushed, and to start the flashing cycle, and to effectively "reset" the flashing cycle when the button is released. I've tried putting previousMillis = 0; in the else clause so it's set when there is no button pressed, but that seems to have no effect. There is also another problem, if the button is released whilst the LED is on and another pressed promptly, then the next cycle will switch it off before switching it on.

Code: [Select]
//constants
const int ledPin =  13;     
const int buttonPin1 = 2;
const int buttonPin2 = 3;
// Variables
boolean pressed = false;     
long previousMillis = 0;
long interval = 0;
int strobeState = LOW;
void setup() {
  // assign pinmodes
  pinMode(ledPin, OUTPUT);
  pinMode(buttonPin1, INPUT);
  pinMode(buttonPin2, INPUT);
  // start with the LED off
  digitalWrite(ledPin, LOW);
}

void loop() {
  unsigned long currentMillis = millis();
  // Check if a button is pressed
  if (digitalRead(buttonPin1) == HIGH)
  {
    pressed = true;
    interval = 1000;
  }
  else if (digitalRead(buttonPin2) == HIGH)
  {
    pressed = true;
    interval = 125;
  }
  else
  {
    pressed = false;     // if no button is pressed set boolean to false
  }
  // if a button is pressed, begin flashing
  if (pressed == true)
  {
    if(currentMillis - previousMillis > interval) {
      // save the last time you blinked the LED
      previousMillis = currentMillis;
      strobeState = !strobeState;     //toggle
      digitalWrite(ledPin, strobeState);
    }
  }
  else
  {
    digitalWrite(ledPin, LOW);
    previousMillis = 0;
  }
}

PaulS

Quote
I'm guessing the interval value is now being ignored because currentMillis - previousMillis is always greater than interval

But, only once, when the switch is pressed. I don't you see you resetting previousMillis when pressed is set to true.

Jebimasta

Code: [Select]
//constants
const int ledPin =  13;     
const int buttonPin1 = 2;
const int buttonPin2 = 3;
// Variables
boolean pressed = false;     
long previousMillis = 0;
long interval = 0;
int strobeState = LOW;
void setup() {
  // assign pinmodes
  pinMode(ledPin, OUTPUT);
  pinMode(buttonPin1, INPUT);
  pinMode(buttonPin2, INPUT);
  // start with the LED off
  digitalWrite(ledPin, LOW);
}

void loop() {
  unsigned long currentMillis = millis();
  // Check if a button is pressed
  if (digitalRead(buttonPin1) == HIGH)
  {
    pressed = true;
    interval = 1000;
    previousMillis = 0;
  }
  else if (digitalRead(buttonPin2) == HIGH)
  {
    pressed = true;
    interval = 125;
    previousMillis = 0;
  }
  else
  {
    pressed = false;     // if no button is pressed set boolean to false
  }
  // if a button is pressed, begin flashing
  if (pressed == true)
  {
    if(currentMillis - previousMillis > interval) {
      // save the last time you blinked the LED
      previousMillis = currentMillis;
      strobeState = !strobeState;     //toggle
      digitalWrite(ledPin, strobeState);
    }
  }
  else
  {
    digitalWrite(ledPin, LOW);
  }
}


Have I put it in the right place?

When the button checks have previousMillis in like this, the LED is constantly at half brightness when a button is held.

Thanks once again for your help and patience.

PaulS

Quote
When the button checks have previousMillis in like this, the LED is constantly at half brightness when a button is held.

Do you want the press of a switch to initiate an action, or the release of the switch to initiate the action, or do you want the action to happen while the switch is pressed (or released)? My view is that pressing the switch (transition from released to pressed) is what should trigger the action, but your view is what matters.

I was thinking that pressing the switch should trigger the action, which was why I had you add the previousMillis reset in the blocks where pressed is set to true. If that is not what you want, and it doesn't sound like it is, you need to remove those. They mean, effectively, that blinking doesn't happen, since previousMillis is reset on every pass through loop.


Jebimasta

Because this is a controller which will send the signal to a strobe light, the timing is important, I need the flashing to be on beat with music. The different intervals represent different "flashes per beat" at given beats per minute. Any delay between a button being pressed and +5V at pin 13 will mean the flashes are out of sync.


  • Pressing a button should initiate flashing

  • Flashing continues for as long as the button is held

  • Releasing the button stops flashing



This code using delay is effective for shorter delays because I'm not likely to release a button and press another one before the delay ends, however with longer delays it's limited and will restrict any expansion of functionality in the future, so I don't want to use it.
The extra buttons are just extra "else if" clauses, but I've removed them for clarity.

Code: [Select]
// constants won't change. They're used here to
// set pin numbers:
const int button1 = 2;     // the number of the pushbutton pins
const int Output =  13;      // the number of the LED pin

// variables will change:
int buttonState1 = 0;         // variable for reading the pushbutton status

void setup() {
 // initialize the output pin as an output:
 pinMode(Output, OUTPUT);      
 // initialize the pushbutton pins as inputs:
 pinMode(button1, INPUT);
}

void loop(){
 // read the state of the pushbutton values:
 buttonState1 = digitalRead(button1);

 // check if the pushbuttons are pressed.
 // if one is, the buttonState is HIGH:
 if (buttonState1 == HIGH) {    
   digitalWrite(Output, HIGH);
 delay(250);
 digitalWrite(Output, LOW);
 delay(250);
 }
 else {
   // turn LED off:
   digitalWrite(Output, LOW);
 }
}


If somehow it is possible that a momentary button press triggering a series of 16/32/64 flashes is an easier task, then perhaps I should investigate that avenue instead.

Go Up