Optimizing buttonPress code

Using Arduino LEDs and Multiplexing
1

Hello,

I'm quite fresh Arduino user and I have this one problem / question.

Below is code that is used to change 2 LEDs states (ON / OFF) , change time interval with potentiometer and "turn program ON/OFF" with pushButton. LEDs are blinking with millis(); function.

const int greenPin =  8;  // pin 8 to greedPin const
int greenState = LOW;     // green LED LOW (0)
const int redPin =  4;    // pin 4 to redPin const
int redState = LOW;       // red LED LOW (0)

const int btn = 11;     // pin 11 to btn const

int btnCounter = 0;     // counter of button pressed states
int btnState = 0;       // button state
int lastBtnState = 0;   // last button state

unsigned long greenPreviousTime = 0;   // last time for green LED
unsigned long redPreviousTime = 0;     // last time for red LED


void setup() {
 Serial.begin(9600);          //Serial for debug
 pinMode(greenPin, OUTPUT);   
 pinMode(redPin, OUTPUT); 
 pinMode(btn, INPUT);
}


void loop() {
 
 int btnState = digitalRead(btn);     // reads current button state 0 or 1 and saves it to variable

 if (btnState != lastBtnState) {
   if(btnCounter==2){              // if counter = 2 reset to 0
       btnCounter=0;
       Serial.println("Program state: RUNNING --> Press button to stop...");
   }
   if (btnState == LOW) {                // if btn pushed down (state is LOW -> 0)
     btnCounter++;                       // add 1 to counter
   }
   if(btnCounter==1 && btnState==HIGH){    // without btnState at HIGH Serial
     greenState = 0;                                   // message was printing 2 times
     redState = 0;                   // turn off LEDs
     digitalWrite(greenPin, greenState);
     digitalWrite(redPin, redState);
     Serial.println("Program state: STOP --> Press button to start...");
   }                              // delay for avoiding errors ??? how to avoid this ???
 delay(50);
 }
 lastBtnState = btnState;            // changes last btn state variable to current state
 

 if(btnCounter==0){                // if counter 0 main code part runs, if is TRUE
   int timeInterval = analogRead(A5);       // TimeInterval from potentiometer (~ 0 -> 1023)

   /*
   Serial.print("Time interval: ");         // uncomment to see current interval in Serial Monitor
   Serial.println(TimeInterval);
   */

   unsigned long greenCurrentTime = millis();    // add millis(); to RED and GREEN LED time variables
   unsigned long redCurrentTime = millis();

   // GREEN LED CODE START **********************
     
   if (greenCurrentTime - greenPreviousTime >= timeInterval) {   // if passed time is higher that interval
     greenPreviousTime = greenCurrentTime;                       // previous time to current time variable
     if (greenState == LOW) {                                    // simple dioda state change
       greenState = HIGH;
     }else {
       greenState = LOW;
     }  
   digitalWrite(greenPin, greenState);                           // on - off LED
   }

   // RED LED CODE START **********************
 
   if (redCurrentTime - redPreviousTime >= timeInterval) {
     redPreviousTime = redCurrentTime;
     if (redState == LOW) {
       redState = HIGH;
     }else {
       redState = LOW;
     } 
   digitalWrite(redPin, redState);
   }
 }
}

Photo of prototype:

I'd like to see some opinion about this code, is it good approach or maybe better is to use some library?

Is there a better way to control state of pushButton?
Is there a better solution to make program / loop run or stop with button?

Right now when I press button program stops, but It can stop for e.g. on 560 ms and when I push it again it will run from 560 ms and after another 440 ms change state of diodes. I want it to run from 0 ms after button is pressed for the second time. Is it possible?

PS: Sorry for English, hope it's ok :slight_smile:

2 
digitalWrite(redPin, greenState);

This does not look right. Perhaps you mean redState?

If you want to reset the time, then you need to set the PreviousTime variables time to millis() whenever the time needs to be reset (ie, when you decide to restart the timer.

The approach is not the most efficient but it should work if you are not doing anything else. If this is part of a larger program then I would simplify it by using structures and arrays to manage the data better.

3

marco_c:

digitalWrite(redPin, greenState);

This does not look right. Perhaps you mean redState?

Ups, already fixed it in my Sketch before posting, but copied the wrong one :wink:

marco_c:
If you want to reset the time, then you need to set the PreviousTime variables time to millis() whenever the time needs to be reset (ie, when you decide to restart the timer.

OK, I'll try to change it like this, and post it here later.

marco_c:
The approach is not the most efficient but it should work if you are not doing anything else. If this is part of a larger program then I would simplify it by using structures and arrays to manage the data better.

Right now I'll try to learn solid basics with Arduino, when I go to more complex code I'll use structures and arrays. Thanks for the hint :wink: