Break your thinking into 3 stages. LEDOn, LEDFading, LEDOff. To help with this.. "enum" is your friend!
enum states { LEDOn, LEDFading, LEDOff };
states ourState = LEDOff;
Here is an example..
enum states { LEDOn, LEDFading, LEDOff }; // The possible things we can do doing.
states ourState; // We save our current state here.
// In setup we set our initial state.
void setup(void) {
//.. typical setup things then..
ourState = LEDOff; // We start with the LED off.. note it.
}
// In loop we FIRST look at our state then from there we know what we should be doing.
void loop(void) {
switch (ourState) {
case LEDOff : doStuffWeDoWhenItsOff(); break;
case LEDOn : doStuffWeDoWhenItsOn(); break;
case LEDFading : doStuffWeDoWhenItsFading(); break;
}
}
This splits up the problem. You can use your button prresses to change the state and keep things from getting mixed up.
jimLee is right, you have to make the 'state' the most important variable of the sketch.
In the loop(), first do the State Change Detection without the led state. I use a 'bool' variable that I make 'true' when a button was pressed.
I think you have only two states: "FADING" and "OFF", each have their own block of code.
Please press Ctrl+T in the Arduino IDE, and make the text layout look good. Put every space, every comma, every new line, every indent in the right place. Use one format for the curly brackets.
Give the variables better names.
The name 'ledState' is used for the brightness. Put 'brightness' or 'luminosity' or 'pwm' in the name.
The name 'ledIsOn' is used for the timing. Put 'time' or 'elapsed time' or 'millis' in the name.
You can remove the 'volatile' keyword, that is only used in certain situations when interrupts are used.
The pin numbers are preferred as this (instead of the #define):
Hello ! Thanks Koepel and jim ! I was able to achieve my goal as you can see with the code below even if i wasn't able to use the switch case I'm a beginner and even after checking the doc, i wasn't confortable enough to work with it.
@Koepel : Thanks a lot for the tips : Cmd +T and State change detection, also for the tips about naming (you can see in my code below that i have changed the name)
i had so trouble to understand the way you use enum :
enum states { LEDOn, LEDFading, LEDOff }; // The possible things we can do doing.[color=#222222][/color]
states ourState; // We save our current state here
I've check some documentation but can you confirm what is written above is the same as below because i wasn't able to confirm this.
int states;
int LEDOn;
int LEDFading;
int LEDOff;[color=#222222][/color]
states = ourState;
Is that the intended way to use the ourState variable with switch
switch (ourState) {
case LEDOff:
digitalWrite(led_pin, LOW);
break;
case LEDFading :
digitalWrite(led_pin, HIGH); // its not fading but just test it with a simple light on
break;
}
Should i change the value of the ourState var when the button is pressed ?
For example :
int anInt; // This can hold integers. Typically -30k something to +30k something.
states ourState; // This can only hold.. LEDOn, LEDFading, LEDOff.
Why? Because that was the list I put into the emum. Its handy because
A) Your list can have names that mean something.
and..
B) The compiler knows about it, so you can't "cheat" and tell it to store something that's NOT on the list. Things like switch(ourState) statements get all fussy if you don't give cases for the entire set. And, they just won't compile of you try to put in something that's NOT on the list.
So enums are VERY popular for breaking up your code into different states.
Sometimes its trivial to change a state sometimes it gets complcated. Lets pretend your is complex. (They all start out that way. Then you get used to it.)
When something happens and you need to change states, you can go into a routine that is basically"I'm in state A and need to change to state B".
In your case the change is that a button was hit. Ok..
switch (ourState) {
case LEDOff :
...
steps you take if its in LEDOff state and the button was hit.
ourState = whatever state happens when the button was hit in this state.
break;
case LEDOn :
...
steps you take if its in LEDOn state and the button was hit.
ourState = whatever state happens when the button was hit in this state.
break;
case LEDFading :
...
steps you take if its in LEDFading state and the button was hit.
ourState = whatever state happens when the button was hit in this state.
break;
}
This is different than the switch (ourState) in your loop() that one is used for "I have some time, I'm doing X, what needs doing right now?".
The switch(ourState) above is where you "shift gears" from state A to state B.
Thanks for the explanation @jill ! Its clearer now even if i'll need some time to really digest your explanations i guess
Here the code with a simple led on (instead of fading) for now because i try to keep thing simple before moving forward. Is it a right way to use switch or can i improve something ?
Thanks
const int buttonPin = 2; // the pin that the pushbutton is attached to
const int ledPin = 11; // the pin that the LED is attached to
// Variables will change:
int buttonPushCounter = 0; // counter for the number of button presses
int buttonState = 1; // current state of the button
int lastButtonState = 1; // previous state of the button
enum states { LEDFading, LEDOff }; // The possible things we can do doing.[color=#222222][/color]
states ourState; // We save our current state here
void setup() {
// initialize the button pin as a input:
pinMode(buttonPin, INPUT_PULLUP);
// initialize the LED as an output:
pinMode(ledPin, OUTPUT);
// initialize serial communication:
Serial.begin(9600);
//ourState = LEDOff; // We start with the LED off.. note it.
}
void loop() {
// read the pushbutton input pin:
buttonState = digitalRead(buttonPin);
if (buttonState != lastButtonState) {
// if the state has changed, increment the counter
if (buttonState == LOW) {
// if the current state is HIGH then the button went from off to on:
buttonPushCounter++;
Serial.println("on");
//Serial.print("number of button pushes: ");
// Serial.println(buttonPushCounter);
} else {
// if the current state is LOW then the button went from on to off:
Serial.println("off");
}
// Delay a little bit to avoid bouncing
delay(50);
}
// save the current state as the last state, for next time through the loop
lastButtonState = buttonState;
switch (ourState) {
case LEDOff :
if (buttonPushCounter % 2 == 0 && buttonState==0) {
ourState = LEDFading;
Serial.println("I'm in LEDOff");
break;
}
case LEDFading :
if (buttonPushCounter % 2 == 0 && buttonState==0) {
ourState = LEDOff;
Serial.println("I'm in LEDFading");
break;
}
}
digitalWrite(ledPin, ourState);
Serial.println(ourState);
delay(200);
}
You kinda need 2 switch statements. One in loop() that lets you decide what to do given the state you are in, And the other being called when the switch gets clicked to let to decide how to change states.
I'd write a switchCheck() function that can contain all the logic of watching your button etc. Then, if clicked, do the state change action. You can call it from loop(). This will get a lot of the complexity out of your loop() and should make the code easier to do.
@jim ! Sorry for late answer, here is the new code with your previous recommandation. I think its cleaner, what do you think ? Next step for me will be to make the lade fade and add the timer
const int buttonPin = 2; // the pin that the pushbutton is attached to
const int ledPin = 11; // the pin that the LED is attached to
// Variables will change:
int buttonPushCounter = 0; // counter for the number of button presses
int buttonState = 1; // current state of the button
int lastButtonState = 1; // previous state of the button
enum states { LEDFading, LEDOff };
states ourState; // We save our current state here
void setup() {
// initialize the button pin as a input:
pinMode(buttonPin, INPUT_PULLUP);
// initialize the LED as an output:
pinMode(ledPin, OUTPUT);
// initialize serial communication:
Serial.begin(9600);
//ourState = LEDOff; // We start with the LED off.. note it.
}
void loop() {
// read the pushbutton input pin:
buttonState = digitalRead(buttonPin);
if (buttonState != lastButtonState) {
// if the state has changed, increment the counter
if (buttonState == LOW) {
buttonPushCounter++;
Serial.println(buttonPushCounter);
//Serial.println("Appuyer");
} else {
//Serial.println("Relacher");
}
// Delay a little bit to avoid bouncing
delay(50);
}
// save the current state as the last state, for next time through the loop
lastButtonState = buttonState;
switch (buttonPushCounter % 2) {
case 0:
Serial.println("i'm in case 0");
myLedFading ();
delay(50);
break;
default:
// if nothing else matches, do the default
// default is optional
Serial.println("i'm in case default");
myLEDTurnOff();
delay(50);
break;
}
digitalWrite(ledPin, ourState);
delay(50);
}
void myLedFading () {
// This function WILL make the LED Fade
ourState = LEDFading;
}
void myLEDTurnOff() {
// This function turn off the LED
ourState = LEDOff;
}
buttonState = digitalRead(buttonPin);
if (buttonState != lastButtonState) {
// if the state has changed, increment the counter
if (buttonState == LOW) {
buttonPushCounter++;
...
delay(50);
}
// save the current state as the last state, for next time through the loop
lastButtonState = buttonState;
I believe this is wrong. This code means, the last line ensures that lastButtonState is always compared in the ‘if’ statement, with the last value of the input pin, regardless of whether it was determined to be valid or not. If the ‘if’ statement is found false, the delay(50) is skipped and so that determination is not debounced. You should only store a ‘lastButtonState’ that has passed the checks for a valid state change, like this:
buttonState = digitalRead(buttonPin);
if (buttonState != lastButtonState) {
// save the current state as the last state, for next time through the loop
lastButtonState = buttonState;
// if the state has changed, increment the counter
if (buttonState == LOW) {
buttonPushCounter++;
...
// Delay a little bit to avoid bouncing
delay(50);
}
Also, to be really “clean” to use your word, you should eliminate the delay() and use millis() for the debounce interval.
I'm a beginner and even after checking the doc, i wasn't confortable enough to work with it.