In this code I made, a pushbutton when on will turn on a led. This Led after the button is pushed, has to stay on for 3 seconds regardless if the button is pushed or not and has to stay off for 3 seconds regardless if the button is pushed or not. After these 6 seconds. The push button can turn on the light again. The code I made seems to work but a few thing seem odd. Is it normal to have 4 paranthesis on line 21? I just feel this is pretty messy and that there must be another way!
thank you everybody
const int buttonPin = 2; // the number of the pushbutton pin
const int ledPin = 3; // the number of the LED pin
const int interval = 3000; // when the button is pushed the led will stay on for ''interval'' millis regardless if the button is pushed or not
int restingled= 6000;// when the led turns off after ''interval '' millis, the led will be off for ''restigled millis regarless if the button is pushed or not
unsigned long previousMillis= restingled; // I think this line is necessary for the led not to be opened when I start the code.
int buttonState = 0; // variable for reading the pushbutton status
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);
unsigned long currentMillis = millis ();
// check if the pushbutton is pressed. If it is, the buttonState is HIGH:
if ((((unsigned long) currentMillis - previousMillis) >= interval) && ((unsigned long)currentMillis - previousMillis) <=restingled) {
digitalWrite(ledPin, LOW);
}
else if ((unsigned long)(currentMillis - previousMillis) <= interval) {
digitalWrite (ledPin, HIGH);
}
else if (buttonState == HIGH) {
digitalWrite(ledPin, HIGH);
previousMillis = millis ();
}
else{
digitalWrite(ledPin, LOW);
}
}
The multiple paranthesis is not necessary if you can remember the order of precedence of operators and interpret the resulting test. The use of unsigned long is not necessary if the correct data types are used so one immediate improvement would be
You don’t need the unsigned long casts for currentMillis and previousMillis, they are already unsigned long.
All of the other brackets in the if statements (bar the outer ones) aren’t needed. Some people add them for clarity. Personally I only use them to override maths precedence (BODMAS) and when I have && and || mixed in the same if.
If you want...
Unsigned long elapsed = currentMillis - previousMillis:
if (elapsed >= interval && elapsed <= restingled) {
digitalWrite(ledPin, LOW);
}
jubukraa:
That all said, interval and restingled are the ones that need the casts in this case, since they're int.
Even if they are int the compiler will work it all out for you so no need to cast them. Having said that, I prefer variables that I am calculating with/comparing to be of the same type as it makes more sense to me
I don't like the IDE to give me warnings so I always do those casts.
And in a world where all and sundry will bog on anyone who uses int for a pin instead of byte, to waste an unsigned long on an interval that would fit in an int, seems to be asking for a shoeing.
in a world where all and sundry will bog on anyone who uses int for a pin instead of byte, to waste an unsigned long on an interval that would fit in an int, seems to be asking for a shoeing.
const int buttonPin = 2; // the number of the pushbutton pin
const int ledPin = 3; // the number of the LED pin