Hello! I am an undergrad mechanical engineering student, and I’m trying to write what seems to be a simple code with a simple task, but I cannot get it to work properly.

I have 3 buttons and 2 LEDs.
Button 1: makes the yellow LED blink on for 3 seconds and then off
Button 2: makes the red LED blink on for 1 second, off for 1 second, on for 1 second, and then off
Button 3: makes both the yellow and red LEDs do their designated blinking pattern

I can get this code to work by simply waiting for the LED to finish blinking before pushing another button (using delays), but I am having trouble figuring out how to write it so that while the yellow LED is blinking, Button 2 can be pressed and the red LED can do its pattern immediately without having to wait for the yellow LED to finish.

I am trying to write the code using previousMillis and currentMillis commands, as I am familiar with using millis() as a substitute for delays.

My attempt so far is attached below, but the LEDs only turn on while the buttons are being pressed, and they only turn on steadily without doing their blinking patterns.

three_buttons_nonworking.txt (2.82 KB)

You need a Finite State Machine.

Google: “FSM arduino” for many tutorials and examples, like: State Machine

``````void loop(){
// read the state of the pushbutton value:

// check if the pushbutton is pressed.
// if it is, the buttonState is HIGH:
if (button1State == HIGH) {
previousMillis = millis();
//button1Output();
}
if (button2State == HIGH) {
previousMillis = millis();
}
if (button3State == HIGH) {
// previousMillis = millis();
}
else {
ledOff();
}
}
``````

Every time through the loop, if the button is still pushed then you get previousMillis again. But if the button isn't pushed you don't call the ledxBlink routine.

You need to think about state machines and you need to think about state change. Don't start the routine if the buttonState IS HIGH, stert it when it BECOMES HIGH. So you need to keep up with the states of the buttons from the last time through loop to compare. A button that was LOW last time and is now HIGH just got pressed.

Then, instead of calling your blink routine from inside the if statement, you need to have a state variable that tracks which one of those functions you want to call. Then in another part of loop, run through the possible values of that and call the appropriate function.

Much of what you want is likely to be in the demo several things at a time

...R

andi37:

Use the IPO programming model and seperate into logical units:

• Input
• Processing
• Output

Add data structures and algorithms as needed.

Try that:

``````enum {BUTTON_1, BUTTON_2, BUTTON_3}; // index 0, 1, 2 of buttons in the button-arrays

#define INPUTMODE INPUT_PULLUP    // INPUT or INPUT_PULLUP
#define BOUNCETIME 5              // bouncing time in milliseconds
byte buttonPins[]={12, 11, 10};// pin numbers of all buttons
#define NUMBUTTONS sizeof(buttonPins) // number of buttons (automatically calculated)
byte buttonState[NUMBUTTONS];  // array holds the actual HIGH/LOW states
byte buttonChange[NUMBUTTONS]; // array holds the state changes when button is pressed or released
enum{UNCHANGED,BUTTONUP,BUTTONDOWN};

long nowMillis;

void input(){
nowMillis=millis(); // read the current time
// read the input state and state changes of all buttons
static unsigned long lastButtonTime; // time stamp for remembering the time when the button states were last read
memset(buttonChange,0,sizeof(buttonChange)); // reset all old state changes
if (nowMillis-lastButtonTime<BOUNCETIME) return;  // within bounce time: leave the function
lastButtonTime=nowMillis; // remember the current time
for (int i=0;i<NUMBUTTONS;i++)
{
byte curState=digitalRead(buttonPins[i]);        // current button state
if (INPUTMODE==INPUT_PULLUP) curState=!curState; // logic is inverted with INPUT_PULLUP
if (curState!=buttonState[i])                    // state change detected
{
if (curState==HIGH) buttonChange[i]=BUTTONDOWN;
else buttonChange[i]=BUTTONUP;
}
buttonState[i]=curState;  // save the current button state
}
}

struct led_t {byte pin; int time; boolean isOn; unsigned long isOnSince;};

led_t leds[]={
{3, 3000}, // pin-3, 3000ms ON time
{2, 1000}, // pin-2, 1000ms ON time
};

void processing()
{
// switch first led on if BUTTON_1 or BUTTON_3 pressed
if (buttonChange[BUTTON_1]==BUTTONDOWN || buttonChange[BUTTON_3]==BUTTONDOWN)
{
leds[0].isOn=true;
leds[0].isOnSince=nowMillis;
}
// switch the other led on if BUTTON_2 or BUTTON_3 pressed
if (buttonChange[BUTTON_2]==BUTTONDOWN || buttonChange[BUTTON_3]==BUTTONDOWN)
{
leds[1].isOn=true;
leds[1].isOnSince=nowMillis;
}
// switch first LED off if time is due
if (nowMillis-leds[0].isOnSince > leds[0].time) leds[0].isOn=false;
// switch the other LED off if time is due
if (nowMillis-leds[1].isOnSince > leds[1].time) leds[1].isOn=false;
}

void output()
{
digitalWrite(leds[0].pin, leds[0].isOn);
digitalWrite(leds[1].pin, leds[1].isOn);
}

void setup()
{
pinMode(leds[0].pin,OUTPUT);
pinMode(leds[1].pin,OUTPUT);
for(int i=0;i<NUMBUTTONS;i++) pinMode(buttonPins[i],INPUTMODE);
}

void loop()
{
input();
processing();
output();
}
``````

jurs:
Use the IPO programming model and seperate into logical units:

• Input
• Processing
• Output

In case of confusion, that is the style in my examples in several things at a time and planning and implementing a program etc

...R

Robin2:
In case of confusion, that is the style in my examples in several things at a time and planning and implementing a program etc

But one thing is different:

In your examples the loop function never looks so short and clean as my loop function for multitasking programs:

``````void loop()
{
input();
processing();
output();
}
``````

In each and every example I try to make clear, that it is always the same programming logic:

• Input: Take all the input from the outer world and read into the Arduino
• Processing: Do all the logical stuff in RAM, keeping it in the Arduino
• Output: And as a final step send the results to the world outside the controller

Just three simple steps. Always.

Although you can split each steps: If you have input from buttons, input from potentiometers, input from I2C sensors, input from SD card, input from RFID readers and so on, maybe the input() function itself calls different sub-intput functions. Or some input is not done each time, but only from time to time.

But the logical sequence is always: Input-Processing-Output
That's what I propagate.

But most people do it the wrong way, they are mixing it up: They read one single button input and then want to make something out of it: "if (buttonOnePressed()) doSomethingForMe();". They do not understand that this is the wrong way if they have more than one button in their circuit and the actual function of the button may depend on the state of other buttons.

So they have first to read ALL buttons, then do the processing logic.
And the final step after all processing is done, would be sending/setting the output to somewhere.

Easy to remember: IPO programming model stands for

• Input
• Processing
• Output
Just as easy as that. All the time. Every program.

One issue with your IPO model is that, as implemented, all your variables are global. Not a disaster in a short arduino sketch, but not a good thing either.

Anything you do involving an input controlling an output obviously has to be in IPO order, but I'm not convinced that there's virtue in doing all inputs/outputs together irrespective as to whether they're related. I'd prefer to be able to encapsulate those things that belong together to be able to make more independent reusable parts. YMMV of course.

wildbill:
One issue with your IPO model is that, as implemented, all your variables are global. Not a disaster in a short arduino sketch, but not a good thing either.

Not really 'all' variables.

Only those state variables that must be accessed by at least two of the functions input(), processing(), output() must be global.

All variables that are just used in one function, can be made local variables.

If you don't like global variables at all, you could put everything into object classes. For example if you would have a ButtonInput-class, your program could create a ButtonInput-object, than perhaps the input() for the buttons would be handled with a public method like:

``````  buttonInput.updateInput();
``````

and instead of accessing global arrays to find out if a button was pressed, you would call a method like

``````  if (buttonInput.changeState(buttonIndex)==BUTTONDOWN) ...
``````

Then you easily could access public methods in objects instead of global variables.

Access to global variables is not necessary for using the IPO-programming model.
Although accessing functions instead of values may cost some performance: So if encapsulating everything in objects, maybe the loop function will not run 50000 times per seconds, but only 10000 times maybe. Would make no difference for most programs, I think.

This was made sing my state machine library which supports concurrent state machines.
Time to draw the state diagrams: 6 min
Time to write code from state diagram: 21min
Time for debugging(missed to change some cut and paste): 4 min
Total: 31min

``````#include <SM.h>

SM red(rIdle);
SM yellow(yIdle);

enum {yPin = 2, rPin, b1Pin, b2Pin, b3Pin} ioPins;

void setup(){
Serial.begin(115200);
pinMode(yPin, OUTPUT);
pinMode(rPin, OUTPUT);
for(int i = b1Pin; i<=b3Pin; i++) pinMode(i, INPUT_PULLUP);
}//setup()

void loop(){
EXEC(red);
EXEC(yellow);
}//loop()

State rIdle(){
Serial.println("red on");
digitalWrite(rPin, 1);
red.Set(rOn1);//change state on next scan cycle
}//if(b1||b3)
}//rIdle()

State rOn1(){
if(red.Timeout(1000)){
Serial.println("red off");
digitalWrite(rPin, 0);
red.Set(rOff);//change state on next scancycle
}//if(timeout)
}//rOn1()

State rOff(){
if(red.Timeout(1000)){
Serial.println("red on");
digitalWrite(rPin, 1);
red.Set(rOn2);//change state on next scancycle
}//if(timeout)
}//rOff()

State rOn2(){
if(red.Timeout(1000)){
Serial.println("red off");
digitalWrite(rPin, 0);
red.Set(rIdle);//change state on next scancycle
}//if(timeout)
}//rOn2()

State yIdle(){
Serial.println("yellow on");
digitalWrite(yPin, 1);
yellow.Set(yOn);//change state on next scan cycle
}//if(b2||b3)
}//yIdle()

State yOn(){
if(yellow.Timeout(1000)){
Serial.println("yellow off");
digitalWrite(yPin, 0);
yellow.Set(yIdle);//change state on next scancycle
}//if(timeout)
}//yOn()
``````

nilton61:
This was made sing my state machine library which supports concurrent state machines.
Time to draw the state diagrams: 6 min
Time to write code from state diagram: 21min
Time for debugging(missed to change some cut and paste): 4 min
Total: 31min

OK, then one more solution.
Simpler than my code in reply #4 or the code in reply #10

``````byte buttonPins[]={12, 11, 10};// pin numbers of all buttons
#define NUMBUTTONS sizeof(buttonPins) // number of buttons (automatically calculated)
struct led_t {byte pin; unsigned long time; boolean isOn; unsigned long isOnSince;};
led_t leds[]={
{3, 3000}, // pin-3, 3000ms ON time
{2, 1000}, // pin-2, 1000ms ON time
};

void setup()
{
pinMode(leds[0].pin,OUTPUT);
pinMode(leds[1].pin,OUTPUT);
for(int i=0;i<NUMBUTTONS;i++) pinMode(buttonPins[i],INPUT_PULLUP);
}

void loop()
{
static long nowMillis;
nowMillis=millis();
{ // switch on LED1
leds[0].isOn=true;
leds[0].isOnSince=nowMillis;
}
{ // switch on LED1
leds[1].isOn=true;
leds[1].isOnSince=nowMillis;
}
// switch off if time is due
if (leds[0].isOn && nowMillis-leds[0].isOnSince >= leds[0].time) leds[0].isOn=false;
if (leds[1].isOn && nowMillis-leds[1].isOnSince >= leds[1].time) leds[1].isOn=false;
digitalWrite(leds[0].pin,leds[0].isOn);
digitalWrite(leds[1].pin,leds[1].isOn);
}
``````

jurs:

``````void loop()
``````

{
input();
processing();
output();
}

``````

Just three simple steps. Always.
``````

Yes, I see what you are saying.

I much prefer to have a series of function calls in loop() that have names that tell you what the program is all about - like reading a summary of a book.

I'm sure our systems can live happily alongside each other.

...R

Thank you for all of your responses! I've learned a lot about different methods of writing code, and you've helped me a great deal. I'm still trying to fully understand some conventions that were used, but once I do hopefully I'll be able to write similar code but for different applications.

Robin2:
I'm sure our systems can live happily alongside each other.

Sometimes they get so happy they seem to light up a cigarrete and puff some smoke!

Happened to me today: I was so sleepy I plugged my 20x4 LCD display on a 12V line. Needless to say its backside smoked. And I didn't need a single line of code to do that!

AlxDroidDev:
Sometimes they get so happy they seem to light up a cigarrete and puff some smoke!

Please don’t associate cigarettes with me or me with cigarettes.

…R

Robin2:
Please don't associate cigarettes with me or me with cigarettes.

I was making a reference to systems - software and specially hardware. Sometimes they fail and when hardware fail they smoke. It was a joke regarding components burning, not meant to offend anyone. I had no way to guess you would be offended by references to smoke and cigarretes.

AlxDroidDev:
I had no way to guess you would be offended by references to smoke and cigarretes.

No problem with smoke. Only with cigarettes and tobacco and their ilk. In my opinion they cause too much harm to be used in jest.

…R

Robin2:
No problem with smoke. Only with cigarettes and tobacco and their ilk. In my opinion they cause too much harm to be used in jest.

FYI, I agree with your opinion. I also find cigarettes disgusting (for a lack of a more harsh word)