I am trying to have an LED flash every second and when a push button is pressed the intensity of the LED decreases. I have managed to have this working to some extent. The problem I am having is that the push button is not registering every press, the LED will change intensity every few presses or if the button is held rather than pressed. I do not think it is a circuit issue as I have used the same circuit to do other experiments i.e turn the LED on/off every 4 presses and that has worked alright. So I imagine it has something to do with the code instead. If anyone could see any errors with the code below that would be appreciated.
/*
Blink without Delay
Turns on and off a light emitting diode (LED) connected to a digital pin,
without using the delay() function. This means that other code can run at the
same time without being interrupted by the LED code.
08-05-2022
LED flashes every 1000 ms (interval2)
When the button is pressed the intesnity of the LED changes
BUG: the push button counter does not increase every time the push button is pressed.
The amount of times the push button is pressed until the counter increases is inconsistent would
indicate that it is a circuit error rather than a coding error.
*/
// constants won't change
const int buttonPin = 2; //the number of the button pin
const int ledPin = 9; // the number of the LED pin
// Variables will change:
int ledValue = 255; // ledValue used to set the LED intensity
int ledState = LOW; //determined the state of the LED
// Generally, you should use "unsigned long" for variables that hold time
// The value will quickly become too large for an int to store
static unsigned long timer = 0;
unsigned long interval = 20;
const long interval2 = 1000; // interval at which to blink (milliseconds)
int buttonPushCounter = 0; // counter for the number of button presses
boolean buttonState = 0; // current state of the button
boolean lastButtonState = 0; // previous state of the button
void setup()
{
// set the digital pin as output:
pinMode(buttonPin, INPUT_PULLUP);
pinMode(ledPin, OUTPUT);
Serial.begin(9600);
}
void loop()
{
if (millis() - timer >= interval) //ensures that the code has been running longer than 0.02s
{
if (millis() - timer >= interval2) //ensures that the code has been running longer than 1s
{
timer = millis(); //redefine timer to the current amount of time code has been running
buttonState = digitalRead(buttonPin); // read the pushbutton input pin:
Serial.print("LED starting value: "); //prints the LED value
Serial.println(ledValue);
// compare the buttonState to its previous state
if (buttonState != lastButtonState)
{
if (buttonState == LOW)
{
// if the current state is LOW then the button went from off to on:
buttonPushCounter++; // if the state has changed, increment the counter
ledValue = ledValue-250; //if the button is pressed the LED intensity will be redefined
//debugging print statements
Serial.println("on");
Serial.print("number of button pushes: ");
Serial.println(buttonPushCounter);
Serial.print("LED PWM value: ");
Serial.println(ledValue);
}
else
{
// if the current state is HIGH then the button went from on to off:
Serial.println("off");
}
// save the current state as the last state, for next time through the loop
lastButtonState = buttonState;
}
if (ledState == LOW)
{
//this if statement is used to keep the LED flashing
//ledState keeps track of whether the LED is on/off
//Serial.print("IN LOOP \\"); //debugging print statement
ledState = HIGH; //redefine the state to ensure that it enters the else loop
if (buttonPushCounter % 1 == 0)
{
analogWrite(ledPin, ledValue); //if the button is pressed the LED will light at the new intensity value
}
}
else
{
//Serial.print("IN LOOP 2 \\"); //debugging print statement
ledState = LOW;
if (buttonPushCounter % 1 == 0)
{
digitalWrite(ledPin, LOW);
}
}
}
}
}
First, congrats on using millis() instead of the blocking delay().
I believe the reason it sometimes miss your button press is because the buttonState is only checked once per second (interval2). If you happen to release/press it before it goes into the second if statement, then it would miss your button press or release.
I think the best way to solve this is to use interrupts.
That is, your loop could do whatever, but as soon as the button is pressed, your routine that handles the button would be called no matter what the loop is doing at that time.
I respectfully disagree. A button press by a human lasts an eternity in computer time. A well written loop will never miss a button press. See the state change for active low inputs tutorial. If it is done right you can poll the switch very often (20-50 times a second), you won't miss a press and the stuff that you want to happen on a high to low transition will only happen once when the button is pressed.
I have watched the serial output and have noticed that the LED value becomes negative. analogWrite seems to just write the last previous positive value. I do plan to implement an element on what to do when ledValue becomes negative but just wanted to focus on the button error at the moment.
At the moment interval2 defines the length of time of the flash and time between flashes. Do I need to introduce another variable so that I can reduce the time between readings of the button presses?
Yes, take the button out of the flashing logic. That sections looks like it will be executed every second, just turn the LED on and off as you do… no depend on the button, no mod % operator. Anyhting mod 1 is 0 anyway, so…
I removed your comments which became or were misleading/wrong and in the way of just reading the code. This pattern is idiomatic, after you've written it a few hundred times, you won't need comment anymore that most of us do.
And where you change the intensity, just subtract say 25, that wil at least work for 10 buttons presses getting dimmer and dimmer… we can worry about turning around and making it brighter and bright later, or even checking to make sure it doesn't go negative as has been pointed out.
As for
I disrespectfully disagree. I do not respect you for suggesting throwing mud into the vodka at this point.
I notice now that you've munged your two millis() regulated sections. The structure might better be
void loop()
{
if (millis() - timer >= interval) //ensures that the code has been running longer than 0.02s
{
// do stuff you want to do 50 times a second - check the button and, whatever
}
if (millis() - timer2 >= interval2) //ensures that the code has been running longer than 1s
{
// stuff you wanna do once a second - toggle the LED on/off
}
}
And you need another timer variable, I just called it timer2. You can figure it out I bet.
HTH
This code blocks the button from being read for 1 second after being pressed.
The button should have a function in void loop() that reads the pin and updates a status variable that a different function will read and act on.
And once your code unblocks, button bounce makes 1 press look like many which is why debouncing buttons is an Arduino Topic and rite of passage code.
I've just tried implementing this using interrupt.
You are all correct. I was wrong!
I ended up still checking millis() to prevent the button from responding too aggressively.
I retract my suggestion. Using interrupt for this is overkill. I was wrong. Sorry...
I was able to get the desired effect without interrupt.
I will share code if someone requests it, but otherwise, I presume we want OP to learn, rather than just solving his/her problem.
Again, I apologize for overcomplicating the matter.
Let me apologize for slamming your suggestion. Too, I cerainyl should not not respect you, ever.
I have found very few cases for preferring interrupts over other methods. Too often in these fora it is used (noobs) or proposed for use. In some of those cases it can be made to work or even work well.
And some of us react, at times poorly.
But when there is still confusion over whether to use an if loop or a void, it is probably better to keep interrupts off the table.
Even if using an interrupt was the best techniwue in a given circumstance, it might still be of value to explore and possibly solve the problem without doing.
It's kinda chicken and eggish. By the time someone knows enough to competently use interrupts, she'll know what kinds of things not to trot them out for.
Interrupts have 84 cycles of jiust overhead, not counting what the interrupt does.
You can poll maybe 10-20 pins or the same pin 20 times in that interval to catch one pin change event.
