Reminder device

Using Arduino Programming Questions
1

Hi all this is my first project from scratch, I have made a few kits but decide it was about time I built something myself, the circuit is built and working, but I am having trouble with the code.

I am making a small device to help me at work, the device has 4 leds and 4 buttons, what I am trying to do is light up certain leds when I press a button which I have managed to do so far.

press button 1 and 2 yellow leds light up
press button 2 and 1 yellow led lights up
press button 3 and 1 red led lights up
and button 4 a green led lights up

I am using pushbuttons with 10k pulldown resistors.
what I am trying to do is have button 4 switch on the green led and then flash on and off every second until I cancel it by pressing button 4 again, also I want it to stay on when I press any of the other button. here is the code I have so far

// set pin numbers:
const int buttonPin1 = 2;     // the number of the buttonPin1 pin
const int buttonPin2 = 3;     // the number of the buttonPin2 pin
const int buttonPin3 = 4;     // the number of the buttonPin3 pin
const int buttonPin4 = 5;     // the number of the buttonPin4 pin
const int ledPin1 =  9;      // the number of the ledPin1 pin
const int ledPin2 =  10;      // the number of the ledPin2 pin
const int ledPin3 =  11;      // the number of the ledPin3 pin
const int ledPin4 =  12;      // the number of the ledPin4 pin

// variables will change:
int buttonState1 = 0;         // variable for reading the buttonPin1 status
int buttonState2 = 0;         // variable for reading the buttonPin2 status
int buttonState3 = 0;         // variable for reading the buttonPin3 status
int buttonState4 = 0;         // variable for reading the buttonPin4 status

void setup() {
  // initialize the LED pin as an output:
  pinMode(ledPin1, OUTPUT); 
pinMode(ledPin2, OUTPUT); 
pinMode(ledPin3, OUTPUT); 
pinMode(ledPin4, OUTPUT); 
  // initialize the buttonPin pin as an input:
  pinMode(buttonPin1, INPUT);
pinMode(buttonPin2, INPUT);
pinMode(buttonPin3, INPUT);
pinMode(buttonPin4, INPUT);
}

void loop(){
  // read the state of the buttonPin value:
  buttonState1 = digitalRead(buttonPin1);

  // switch on 2 yellow leds
  // check if the pushbutton is pressed.
  // if it is, the buttonState is HIGH:
  if (buttonState1 == HIGH) {     
    // turn LED on:    
    digitalWrite(ledPin1, HIGH);
    digitalWrite(ledPin3, HIGH); 
  digitalWrite(ledPin2, LOW);
 digitalWrite(ledPin4, LOW); 
  } 
  
   // read the state of the buttonPin value:
  buttonState2 = digitalRead(buttonPin2);

  // switch on 1 yellow led
  // check if the pushbutton is pressed.
  // if it is, the buttonState is HIGH:
  if (buttonState2 == HIGH) {     
    // turn LED on:  
  digitalWrite(ledPin3, HIGH);  
    digitalWrite(ledPin2, LOW); 
   digitalWrite(ledPin1, LOW); 
   digitalWrite(ledPin4, LOW);
  } 
  
  // read the state of the buttonPin value:
  buttonState3 = digitalRead(buttonPin3);

  // switch on red led
  // check if the pushbutton is pressed.
  // if it is, the buttonState is HIGH:
  if (buttonState3 == HIGH) {     
    // turn LED on: 
    digitalWrite(ledPin4, HIGH);
 digitalWrite(ledPin3, LOW);   
    digitalWrite(ledPin2, LOW); 
   digitalWrite(ledPin1, LOW); 
  } 
  
  // read the state of the buttonPin value:
  buttonState4 = digitalRead(buttonPin4);

  //switch on green led
  // check if the pushbutton is pressed.
  // if it is, the buttonState is HIGH:
  if (buttonState4 == HIGH) {     
    // turn LED on: 
    digitalWrite(ledPin4, LOW);
 digitalWrite(ledPin3, LOW);   
    digitalWrite(ledPin2, HIGH); 
   digitalWrite(ledPin1, LOW); 
  } 
}

I would be grateful of any help

2

Please use code tags around your code (The # button)

It makes the code much easier to read, and you are more likely to get a answer to your question

// set pin numbers:
const int buttonPin1 = 2;     // the number of the buttonPin1 pin
const int buttonPin2 = 3;     // the number of the buttonPin2 pin
const int buttonPin3 = 4;     // the number of the buttonPin3 pin
const int buttonPin4 = 5;     // the number of the buttonPin4 pin
const int ledPin1 =  9;      // the number of the ledPin1 pin
const int ledPin2 =  10;      // the number of the ledPin2 pin
const int ledPin3 =  11;      // the number of the ledPin3 pin
const int ledPin4 =  12;      // the number of the ledPin4 pin

// variables will change:
int buttonState1 = 0;         // variable for reading the buttonPin1 status
int buttonState2 = 0;         // variable for reading the buttonPin2 status
int buttonState3 = 0;         // variable for reading the buttonPin3 status
int buttonState4 = 0;         // variable for reading the buttonPin4 status

void setup() {
  // initialize the LED pin as an output:
  pinMode(ledPin1, OUTPUT);
pinMode(ledPin2, OUTPUT);
pinMode(ledPin3, OUTPUT);
pinMode(ledPin4, OUTPUT);
  // initialize the buttonPin pin as an input:
  pinMode(buttonPin1, INPUT);
pinMode(buttonPin2, INPUT);
pinMode(buttonPin3, INPUT);
pinMode(buttonPin4, INPUT);
}

void loop(){
  // read the state of the buttonPin value:
  buttonState1 = digitalRead(buttonPin1);

  // switch on 2 yellow leds
  // check if the pushbutton is pressed.
  // if it is, the buttonState is HIGH:
  if (buttonState1 == HIGH) {     
    // turn LED on:   
    digitalWrite(ledPin1, HIGH);
    digitalWrite(ledPin3, HIGH);
  digitalWrite(ledPin2, LOW);
 digitalWrite(ledPin4, LOW);
  }
 
   // read the state of the buttonPin value:
  buttonState2 = digitalRead(buttonPin2);

  // switch on 1 yellow led
  // check if the pushbutton is pressed.
  // if it is, the buttonState is HIGH:
  if (buttonState2 == HIGH) {     
    // turn LED on: 
  digitalWrite(ledPin3, HIGH); 
    digitalWrite(ledPin2, LOW);
   digitalWrite(ledPin1, LOW);
   digitalWrite(ledPin4, LOW);
  }
 
  // read the state of the buttonPin value:
  buttonState3 = digitalRead(buttonPin3);

  // switch on red led
  // check if the pushbutton is pressed.
  // if it is, the buttonState is HIGH:
  if (buttonState3 == HIGH) {     
    // turn LED on:
    digitalWrite(ledPin4, HIGH);
 digitalWrite(ledPin3, LOW);   
    digitalWrite(ledPin2, LOW);
   digitalWrite(ledPin1, LOW);
  }
 
  // read the state of the buttonPin value:
  buttonState4 = digitalRead(buttonPin4);

  //switch on green led
  // check if the pushbutton is pressed.
  // if it is, the buttonState is HIGH:
  if (buttonState4 == HIGH) {     
    // turn LED on:
    digitalWrite(ledPin4, LOW);
 digitalWrite(ledPin3, LOW);   
    digitalWrite(ledPin2, HIGH);
   digitalWrite(ledPin1, LOW);
  }
}
3

Sorry, have use the code tag now
thank you

4

The demo code in the first post of this Thread might give you some ideas.

...R

5

Robin2:
The demo code in the first post of this Thread might give you some ideas.

Or, take a look at this, which I just help a fellow finish... very similar.

try something like this:

// set pin numbers:
const int buttonPin1 = 2;     // the number of the buttonPin1 pin
const int buttonPin2 = 3;     // the number of the buttonPin2 pin
const int buttonPin3 = 4;     // the number of the buttonPin3 pin
const int buttonPin4 = 5;     // the number of the buttonPin4 pin
const int ledPin1 =  9;       // yellow?
const int ledPin2 =  10;      // green?
const int ledPin3 =  11;      // yellow?
const int ledPin4 =  12;      // red?
//
// variables will change:
//int buttonState1 = 0;         // variable for reading the buttonPin1 status
//int buttonState2 = 0;         // variable for reading the buttonPin2 status
//int buttonState3 = 0;         // variable for reading the buttonPin3 status
//int buttonState4 = 0;         // variable for reading the buttonPin4 status
//
unsigned long startTime = millis();
unsigned long blinkInterval = 250UL;// change this for slower/faster flashing of green LED
boolean blinkToggle = false;
//
void setup() 
{
  // initialize the LED pin as an output:
  pinMode(ledPin1, OUTPUT); 
  pinMode(ledPin2, OUTPUT); 
  pinMode(ledPin3, OUTPUT); 
  pinMode(ledPin4, OUTPUT); 
  // initialize the buttonPin pin as an input:
  pinMode(buttonPin1, INPUT);
  pinMode(buttonPin2, INPUT);
  pinMode(buttonPin3, INPUT);
  pinMode(buttonPin4, INPUT);
}

void loop()
{
  // read the state of the buttonPin1 value:
  if (digitalRead(buttonPin1) == HIGH) 
  {      
    digitalWrite(ledPin1, HIGH);
    digitalWrite(ledPin3, HIGH);
    digitalWrite(ledPin4, LOW);
    if (blinkToggle)
    {
      digitalWrite(ledPin2, HIGH);
    }
    else
    {
      digitalWrite(ledPin2, LOW);
    }  
  } 
  else if (digitalRead(buttonPin2) == HIGH) 
  {     
    digitalWrite(ledPin1, LOW);
    digitalWrite(ledPin3, HIGH); 
    digitalWrite(ledPin4, LOW);  
    if (blinkToggle)
    {
      digitalWrite(ledPin2, HIGH);
    }
    else
    {
      digitalWrite(ledPin2, LOW);
    }  
  } 
  else if (digitalRead(buttonPin3) == HIGH) 
  {   
    digitalWrite(ledPin1, LOW);  
    digitalWrite(ledPin3, HIGH);
    digitalWrite(ledPin4, LOW);   
    if (blinkToggle)
    {
      digitalWrite(ledPin2, HIGH);
    }
    else
    {
      digitalWrite(ledPin2, LOW);
    }  
  } 
  else if (digitalRead(buttonPin4) == HIGH) 
  {
    blinkToggle = (true ? false: true);
    digitalWrite(ledPin4, LOW);
    digitalWrite(ledPin3, LOW);    
    digitalWrite(ledPin1, LOW);
    if (blinkToggle)
    {
      blinkGreenLed();
    } 
    else
    {
      digitalWrite(ledPin2, LOW);
    }
  } 
  else
  {
    // nothing to do here
    // you could add compound if's
    // if (digitalRead(buttonPin3) == HIGH && digitalRead(buttonPin2) == HIGH)
    // do something else
    // for example
  }
}

void blinkGreenLed()
{
  if (millis() - startTime >= blinkInterval)
  {
    digitalRead(ledPin2) == HIGH ? digitalWrite(ledPin2, LOW): digitalWrite(ledPin2, HIGH);
    startTime=millis();
  }
}
6

BulldogLowell:
try something like this:

or perhaps not.

How about rewriting it to use arrays rather than encourage silly long-winded, repetitive and error prone code.

...R

7

Robin2:

BulldogLowell:
try something like this:

or perhaps not.

How about rewriting it to use arrays rather than encourage silly long-winded, repetitive and error prone code.

...R

I worked an improvement off of what was posted, and pointed out a few of the things that the OP may have done to block his solution. There is nothing wrong with people seeing the progression of improvement, even if it doesn't bring them to nirvana.

How about being constructive and helping the OP and the rest of the forum members rather than just pointing out what you might have done more perfectly?

I didn't think you were one of those tough-guy code bullies beating up newbies for silly things like code tags. I saw you as trying in earnest to help. You may even have helped me once or twice... I am sure.

8

BulldogLowell:
How about being constructive and helping the OP and the rest of the forum members rather than just pointing out what you might have done more perfectly?

Robin2's post was constructive. It pointed out a substantial problem with the code making it four times as long as it needed to be and encouraging copy-and-paste errors, and then suggested how to correct that problem. He also referred to another thread where there are code examples demonstrating exactly how to do what adrwebs is trying to do. As if that wasn't helpful enough, he is the author of the code on the other thread that he linked to and spent a considerable amount of effort developing that example and explaining it.

By the way, it would benefit you to go learn how to use arrays, too - it will simplify your code considerably.

9

PeterH:

BulldogLowell:
How about being constructive and helping the OP and the rest of the forum members rather than just pointing out what you might have done more perfectly?

Robin2's post was constructive. It pointed out a substantial problem with the code making it four times as long as it needed to be and encouraging copy-and-paste errors, and then suggested how to correct that problem. He also referred to another thread where there are code examples demonstrating exactly how to do what adrwebs is trying to do. As if that wasn't helpful enough, he is the author of the code on the other thread that he linked to and spent a considerable amount of effort developing that example and explaining it.

Well, if in your view his last comment was constructive, in my view yours certainly was not. But, this is a FORUM, so I guess like everyone here I have to tolerate the concept of differing approaches, suggestions, opinions, proposals, ideas, etc. Likewise, I must sit by and watch folks get lambasted for making an error in their efforts to assist as if those who cast judgment are immune to making errors.

By the way, no one suggested that the OP should overlook Robin2's advice, particularly me. It is good stuff that I have used myself.

10

Thanks for the help and guidance, I do appreciate all your constructive comments.
I have looked at arrays and for some reason I am having trouble with it, that why I decided to write the code this way, I know its not perfect but it is a learning curve and I have to start somewhere.

11

adrwebs:
Thanks for the help and guidance, I do appreciate all your constructive comments.
I have looked at arrays and for some reason I am having trouble with it, that why I decided to write the code this way, I know its not perfect but it is a learning curve and I have to start somewhere.

so, in the interest of assisting you further, and since you had an interest in using an array...

I'm not sure about the colours of the LED's, so you may have to adjust that on your board.

Not perfect either, but a progression.

// set pin numbers:
const int buttonPin0 = 2;     // the number of the buttonPin1 pin
const int buttonPin1 = 3;     // the number of the buttonPin2 pin
const int buttonPin2 = 4;     // the number of the buttonPin3 pin
const int buttonPin3 = 5;     // the number of the buttonPin4 pin
const int ledPin1 =  9;       // yellow?
const int ledPin2 =  10;      // green?
const int ledPin3 =  11;      // yellow?
const int ledPin4 =  12;      // red?
//
const int ledArray [4] [3] = {
 { HIGH, HIGH,  LOW},
 {  LOW, HIGH,  LOW},
 {  LOW,  LOW, HIGH},
 {  LOW, LOW ,  LOW}
 };
//
unsigned long startTime = millis();
unsigned long blinkInterval = 250UL;// change this for slower/faster flashing of green LED
boolean blinkToggle = false;
int state = 3;
//
void setup() 
{
  // initialize the LED pin as an output:
  pinMode(ledPin1, OUTPUT); 
  pinMode(ledPin2, OUTPUT); 
  pinMode(ledPin3, OUTPUT); 
  pinMode(ledPin4, OUTPUT); 
  // initialize the buttonPin pin as an input:
  pinMode(buttonPin0, INPUT);
  pinMode(buttonPin1, INPUT);
  pinMode(buttonPin2, INPUT);
  pinMode(buttonPin3, INPUT);
}
//
void loop()
{
  if (digitalRead(buttonPin0) == HIGH) state = 0; 
  else if (digitalRead(buttonPin1) == HIGH) state = 1;
  else if (digitalRead(buttonPin2) == HIGH) state = 2;
  else if (digitalRead(buttonPin3) == HIGH) 
  {
    state = 3;
    blinkToggle = (true ? false : true);
  }
  updateLeds(state);
}
//
void updateLeds(int value)
{
  digitalWrite(ledPin1, ledArray [value] [0]);
  digitalWrite(ledPin3, ledArray [value] [1]);
  digitalWrite(ledPin4, ledArray [value] [2]);
  if (blinkToggle == true)
  {
    if (millis() - startTime >= blinkInterval)
    {
      digitalRead(ledPin2) == HIGH ? digitalWrite(ledPin2, LOW): digitalWrite(ledPin2, HIGH);
      startTime = millis();
    }
  }
  else
  {
    digitalWrite(ledPin2, LOW);
  }
}
12

Hi BullogLowell

I was just about to post this when I saw your message and would like to thank you for you help

I have been looking at arrays and this is what I have so far

int ledPins[] = {9,10,11,12 };       // an array of pin numbers to which LEDs are attached

int pinCount = 4;           // the number of pins
int buttons[] = {2,3,4,5};  // an array of pin numbers to which LEDs are attached
int buttonCount = 4;        // the number of pins


void setup() {
  // the array elements are numbered from 0 to (pinCount - 1).
  // use a for loop to initialize each pin as an output:
  for (int thisPin = 0; thisPin < pinCount; thisPin++)  {
    pinMode(ledPins[thisPin], OUTPUT); // initialize each led pin as an output:     
  }
  for (int buttonPin = 0; buttonPin < buttonCount; buttonPin++) {
    pinMode(buttons[buttonPin], INPUT); // initialize each button pin as an input:
  }
}

void loop() {
  

 if (digitalRead(buttons[0]) == HIGH)
{
  digitalWrite(ledPins[0], HIGH);
  for(int i = 0; i<4; i++)
    {
      if( i!=0)
      {
        digitalWrite(ledPins[i],LOW);
      }
    }
  
}
 
  if (digitalRead(buttons[1]) == HIGH)
{
  digitalWrite(ledPins[1], HIGH);
  for(int i = 0; i<4; i++)
    {
      if( i!=1)
      {
        digitalWrite(ledPins[i],LOW);
      }
    }
}
}

I am hoping I am on the right track, if not please let me know

13

[quote author=adrwebs link=topic=253544.msg1794944#msg1794944 date=1404680885
I am hoping I am on the right track, if not please let me know
[/quote]

yes.

One thing to keep in mind is that this structure:

if (thisButton){}
else if(thisOtherButton){}
else if(yetAnotherButton{}
etc...

lets you get away with some of the problems that may come about from having a bouncy switch as it essentially looks at each pin state and "if not" simply moves on. for the short time the switch may bounce, it will eventually see a HIGH and then change state. This is my only concern for you on the green light... you may have bounce issues there, which can be easily fixed with a little more code. likewise, if you are holding dow buttonPin0 and then press buttonPin1, it won't change state because the logic moves on once it gets a true if. You can deal with that too if you use:

if (thisButtonIsPressed && thatButtonIsPressed)

and will give you more opportunities for increased functionality.

All of this is why I put the simple state change on the 'revised' version to make sure we are driving to a state, not just an instantaneous evaluation of a pin.

I think focusing on a state and driving the program from 'changes in state' is an alternative that you will likely prefer, and find more useful in the future.

14

I would prefer to read all the buttons at the same time and save their values. Then use the saved values in the IF statements

for (byte n = 0; n < 4; n ++) {
   buttonState[n] = digitalRead(buttons[n];
}

Followed by something like this

 if (buttonState[0]) == HIGH)
{
  digitalWrite(ledPins[0], HIGH);
  for(int i = 0; i<4; i++)

etc

...R