Correct If statement

Hi,

I`m building a countdown timer with a LED segment display that counts down from 9 to 0. It also makes a beep on each count on a speaker...

I´m having trouble with the code, though. A switch is to trigger this event. If I press the switch now, it all starts, but I can´t stop it with unpressing the switch. What causes this ?

Heres the code:

#define input 10

void setup()
{

  
  pinMode(11,LOW);
  pinMode(8,HIGH);
  pinMode(7,HIGH);
  pinMode(6,HIGH);
  pinMode(5,HIGH);
  pinMode(4,HIGH);
  pinMode(3,HIGH);
  pinMode(2,HIGH);
  pinMode(1,HIGH);
  pinMode(0,HIGH);
  
  
}

void loop()
{
if
   (digitalRead(input))
   
               {delay(1000);
		PORTD=0b10011000; // 9
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10000000; // 8
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b11111000; // 7
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10000010; // 6
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10010010; // 5
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10011001; // 4
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10110000; // 3
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10100100; // 2
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b11111001; // 1
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b11000000; // 0
                tone(8, 980, 30000);
                delay(30300);}
else
{

}

}

The way you have written the code, once you press the switch, you enter the if statement. There is no code to check if the button is still pressed once you've entered the if.

It sounds like you want the countdown to only happen while the button is pressed. A good start would be to look at the Blink without Delay tutorial. http://arduino.cc/en/Tutorial/BlinkWithoutDelay

This will free you up to check for other actions while the countdown is happening.

Just a couple of style notes:

You should use the values INPUT and OUTPUT for the pinMode() function rather than HIGH and LOW.

If the "else" part is empty there is no need to include it.

I have changed some of the code, to see if it did anything. Is there something here that I am logically not understanding ?

const int  buttonPin = 11;

int buttonState = 0;
int lastButtonState = 0;

void setup()
{

  pinMode(buttonPin, INPUT);
  pinMode(10,OUTPUT);
  pinMode(8,OUTPUT);
  pinMode(7,OUTPUT);
  pinMode(6,OUTPUT);
  pinMode(5,OUTPUT);
  pinMode(4,OUTPUT);
  pinMode(3,OUTPUT);
  pinMode(2,OUTPUT);
  pinMode(1,OUTPUT);
  pinMode(0,OUTPUT);
  
  
}

void loop()
{

  buttonState = digitalRead(buttonPin);
  
  if (buttonState != lastButtonState) {
    
    if (buttonState == HIGH) {
   
                delay(1000);
		PORTD=0b10011000; // 9
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10000000; // 8
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b11111000; // 7
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10000010; // 6
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10010010; // 5
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10011001; // 4
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10110000; // 3
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10100100; // 2
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b11111001; // 1
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b11000000; // 0
                tone(8, 980, 30000);
                delay(30300);}}
                
lastButtonState = buttonState;
}

While the code does look a little better you are still fundamentally doing the same thing.

The way you have the if statement written, when you press the button you are kind of telling the arduino to go into another room and execute a series of commands. Your list of commands is basically...

-display number
-do absolutely nothing else for 1 second
-display another number
-do absolutely nothing else for 1 second.
...

So, the arduino dutifully goes into a dark room all alone and executes exactly what you asked it to do without ever checking back with you to confirm that you still want it to continue.

What you want the arduino to really do is check in with you frequently to make sure it is still doing what you want.

if (button is pressed)
   save current time in milliseconds
   display the first number
end

if (button is still pressed)
   check the amount of time that has passed
   if (one second has passed)
      play sound and,
      change the number
   end 
else 
   stop the countdown or don't change number or whatever it is you want it to do (or not do).
end

There's a little more code involved there with saving the time when you update the display and you probably want to debounce the switch (you can find some good posts on debouncing here) but the fundamental idea is that you want the code to "check in" with you very frequently and not just blindly execute a sequence of commands.

Please consider the use of array's and for loops, that shortens your code quite a lot.

then you can do things like: (just snippets will not compile)

...
array[10] = { 0b11000000, 0b11111001, 0b10100100, 0b10110000, 0b10011001, 0b10010010, 0b10000010, 0b11111000, 0b10000000, 0b10011000 };

void setup()
{
  for (int i=0; i<10; i++) pinMode(i, OUTPUT);
  pinMode(buttonPin, INPUT);
}

void loop()
{
...

      delay(1000);
      for (int i = 9; i >= 1; i--)
      {
        PORTD= array[i];
        tone(8, 440, 200);
        delay(1000);
      }
      PORTD=0b11000000; // 0
      tone(8, 980, 30000);
      delay(30300);

...

(all prev comments might still apply :wink:

just snippets will not compile

This certainly wont:

array[10] = { 0b11000000, 0b11111001, 0b10100100, 0b10110000, 0b10011001, 0b10010010, 0b10000010, 0b11111000, 0b10000000, 0b10011000 };

Initialization of an entire array can only be performed when the initialization and declaration are performed in one statement.

YOU're right Paul,

the datatype should be there,

uint8_t array[10] = { 0b11000000, 0b11111001, 0b10100100, 0b10110000, 0b10011001, 0b10010010, 0b10000010, 0b11111000, 0b10000000, 0b10011000 };

Thanx for pointing out,

Thanks for the help. I´m still scratching my head, as I´m new to all of this. I ´m making progress though, so I´ll post some updates when I get "there".

Now, I have one question however. Since I have to use this thing in a couple of days I don´t have time to finish it properly. Due to the afformentioned difficulties. Everything works, except it cycles through two times even though I´m holding in the button. After two cycles, it stops correctly and starts again if I push the button. What causes it to do this?

const int buttonPin = 11;

int buttonState;
int lastButtonState = LOW;

long lastDebounceTime = 0;
long debounceDelay = 50;

void setup()
{
  pinMode(buttonPin, INPUT);
  pinMode(10,OUTPUT);
  pinMode(8,OUTPUT);
  pinMode(7,OUTPUT);
  pinMode(6,OUTPUT);
  pinMode(5,OUTPUT);
  pinMode(4,OUTPUT);
  pinMode(3,OUTPUT);
  pinMode(2,OUTPUT);
  pinMode(1,OUTPUT);
  pinMode(0,OUTPUT); 
  digitalWrite(buttonPin, HIGH);
}

void loop()
{
  int reading = digitalRead(buttonPin);
                
  if (reading != lastButtonState) {
    lastDebounceTime = millis();
  }
  
  if ((millis() - lastDebounceTime) > debounceDelay) {
    buttonState = reading;
  }
   if (buttonState == HIGH) {         
                tone(8, 1800, 200);
                delay(500);
                tone(8, 1800, 200);
                delay(500);
                tone(8, 1800, 200);
                delay(500);
                digitalWrite(10, HIGH);
                delay(200);
                digitalWrite(10, HIGH);
                delay(200);
                digitalWrite(10, HIGH);
                delay(200);
                digitalWrite(10, LOW);
                delay(40);
                digitalWrite(10, HIGH);
                delay(40);
                digitalWrite(10, LOW);
                delay(40);
                digitalWrite(10, HIGH);
                delay(40);
                digitalWrite(10, LOW);
                delay(40);
                digitalWrite(10, HIGH);
                delay(40);
                digitalWrite(10, LOW);
                delay(40);
                digitalWrite(10, HIGH);
                delay(40); 
                delay(200);
		PORTD=0b10011000; // 9
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10000000; // 8
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b11111000; // 7
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10000010; // 6
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10010010; // 5
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10011001; // 4
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10110000; // 3
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10100100; // 2
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b11111001; // 1
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b11000000; // 0
                tone(8, 980, 16000);
                digitalWrite(10, LOW);
                delay(80);
                digitalWrite(10, HIGH);
                delay(80);
                digitalWrite(10, LOW);
                delay(80);
                digitalWrite(10, HIGH);
                delay(80);
                digitalWrite(10, LOW);
                delay(80);
                digitalWrite(10, HIGH);
                delay(80);
                digitalWrite(10, LOW);
                delay(80);
                digitalWrite(10, HIGH);
                delay(80);
                digitalWrite(10, LOW);
                delay(80);
                digitalWrite(10, HIGH);
                delay(80);
                digitalWrite(10, LOW);
                delay(80);
                digitalWrite(10, HIGH);
                delay(80);
                digitalWrite(10, LOW);
                delay(80);
                digitalWrite(10, HIGH);
                delay(80);
                digitalWrite(10, LOW);
                delay(80);
                digitalWrite(10, HIGH);
                delay(80);
                digitalWrite(10, LOW);
                delay(80);
                digitalWrite(10, HIGH);
                delay(15000);
                digitalWrite(10, LOW);              
   }
   

lastButtonState = reading;

}

You have reading, buttonState, and lastButtonState all in play. You only need two values. One of these is redundant, and not reset at the proper time. I'll leave it to you to figure out which one (hint, its the one that starts with a b).

There is a Button library that manages switches very well. I think, since you are having so much trouble with switches, that you'd be well advised to use a tested class for reading the switches.

Of course, all those delay()s are making it so that the switch state is rarely being read, so all the debouncing stuff you are doing is a waste of effort.

The debouncing is a work in progress, that´s why I left it in there. It is supposed to make me check the switch state inside the actions at a later time. I´ll try to figure out what piece of code is redundant :wink:

So, I have cleaned up the code a little bit now. It resets properly if I push the button during the countdown and other sequences. Thats good! And intentionally good enough for my planned use on Saturday. I have some free time however, and would really like to explore the next set of events. I NEED to check the code during execution IF the buttonState has changed to LOW. All the material I can find on the Internetz has little code in it, not the way I have written up mine. They basically have one statedetection, and change ledPin to high or low accordingly. That doesnt really explain it to me. I need more info, what to look for in order to read up. I need by teaspoons. So far, I think I have gotten it damn right! Please, nudge me in the RIGHT direction, dont just push me off a cliff....

const int buttonPin = 11;

int buttonState = 0;
int lastButtonState = 0;

long lastDebounceTime = 0;
long debounceDelay = 50;

void setup()
{
  pinMode(buttonPin, INPUT);
  pinMode(10,OUTPUT);
  pinMode(8,OUTPUT);
  pinMode(7,OUTPUT);
  pinMode(6,OUTPUT);
  pinMode(5,OUTPUT);
  pinMode(4,OUTPUT);
  pinMode(3,OUTPUT);
  pinMode(2,OUTPUT);
  pinMode(1,OUTPUT);
  pinMode(0,OUTPUT); 
}

void loop()
{
  buttonState = digitalRead(buttonPin);
  
  if (buttonState != lastButtonState)
     if (buttonState == HIGH) {         
                tone(8, 1800, 200);
                delay(500);
                tone(8, 1800, 200);
                delay(500);
                tone(8, 1800, 200);
                delay(500);
                digitalWrite(10, HIGH);
                delay(200);
                digitalWrite(10, HIGH);
                delay(200);
                digitalWrite(10, HIGH);
                delay(200);
                digitalWrite(10, LOW);
                delay(40);
                digitalWrite(10, HIGH);
                delay(40);
                digitalWrite(10, LOW);
                delay(40);
                digitalWrite(10, HIGH);
                delay(40);
                digitalWrite(10, LOW);
                delay(40);
                digitalWrite(10, HIGH);
                delay(40);
                digitalWrite(10, LOW);
                delay(40);
                digitalWrite(10, HIGH);
                delay(40); 
                delay(200);
		PORTD=0b10011000; // 9
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10000000; // 8
                tone(8, 440, 200);
                delay(1000);
                PORTD=0b11111000; // 7
                tone(8, 440, 200);
                delay(1000);
                PORTD=0b10000010; // 6
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10010010; // 5
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10011001; // 4
                tone(8, 440, 200);
                delay(1000);
		PORTD=0b10110000; // 3
                tone(8, 740, 800);
                delay(1000);
		PORTD=0b10100100; // 2
                tone(8, 740, 800);
                delay(1000);
		PORTD=0b11111001; // 1
                tone(8, 740, 800);
                delay(1000);
		PORTD=0b11000000; // 0
                tone(8, 2000, 16000);
                digitalWrite(10, LOW);
                delay(80);
                digitalWrite(10, HIGH);
                delay(80);
                digitalWrite(10, LOW);
                delay(80);
                digitalWrite(10, HIGH);
                delay(80);
                digitalWrite(10, LOW);
                delay(80);
                digitalWrite(10, HIGH);
                delay(80);
                digitalWrite(10, LOW);
                delay(80);
                digitalWrite(10, HIGH);
                delay(80);
                digitalWrite(10, LOW);
                delay(80);
                digitalWrite(10, HIGH);
                delay(80);
                digitalWrite(10, LOW);
                delay(80);
                digitalWrite(10, HIGH);
                delay(80);
                digitalWrite(10, LOW);
                delay(80);
                digitalWrite(10, HIGH);
                delay(80);
                digitalWrite(10, LOW);
                delay(80);
                digitalWrite(10, HIGH);
                delay(80);
                digitalWrite(10, LOW);
                delay(80);
                digitalWrite(10, HIGH);
                delay(15000);
                digitalWrite(10, LOW);              
   }
lastButtonState = buttonState;
}

It resets properly if I push the button during the countdown and other sequences.

It can't possibly.

                delay(500);
                delay(500);
                delay(500);
                delay(200);
                delay(200);
                delay(200);
                delay(40);
                delay(40);
                delay(40);
                delay(40);
                delay(40);
                delay(40);
                delay(40);
                delay(40); 
                delay(200);
                delay(1000);
                delay(1000);
                delay(1000);
                delay(1000);
                delay(1000);
                delay(1000);
                delay(1000);
                delay(1000);
                delay(1000);
                delay(80);
                delay(80);
                delay(80);
                delay(80);
                delay(80);
                delay(80);
                delay(80);
                delay(80);
                delay(80);
                delay(80);
                delay(80);
                delay(80);
                delay(80);
                delay(80);
                delay(80);
                delay(80);
                delay(80);
                delay(15000);

By my count, that's 27 seconds during which you do nothing. Nada, zip. zilch. Completely ignore the switch.

You will eventually need to implement a state machine. During each pass through loop you will collect sensor input to see if any input requires a change in state.

Then, you'll check the time to see if that necessitate a change in state.

Then you'll execute whatever code is required as you transition from one state to another, including keep track of what the new state is, and when you transitioned to that state. The blink without delay example implements two states - led on and led off, but it illustrates the concept.

PaulS:

It resets properly if I push the button during the countdown and other sequences.

It can't possibly.

Perhaps by "the button" he means the Reset button on the Arduino and not the button he uses to initiate the sequence. :\

To clear things off, this is a release switch. When I open the lid on my briefcase it release the switch, meaning is closes the switch. Now, if I close the lid during countdown (to open the switch) it finishes of the sequences and resets. Then it waits for the switch to close again. If I open the lid again, it all starts counting down again. I want to be able to open the lid, panic, and then close it, which would make it stop the countdown, and reset and wait for high button state again.

I want to be able to open the lid, panic, and then close it, which would make it stop the countdown, and reset and wait for high button state again.

Then the delay()s have got to go. You need a state machine.

My only real code experience is from Applescript, there too delays are bad for behaviour. millis() would be better for delays ...?

State Machine allows you to break segment of code into "shortcuts" to get called upon later, right..?

millis() would be better for delays

millis() would be better for timing-related activities, eliminating the need to standAroundDoingNothing() (otherwise known as delay()).

State Machine allows you to break segment of code into "shortcuts" to get called upon later, right..?

Something like that. You might have a state "led on" and one "led off". The transition from the led off to the led on state might involve calling a function lenOn(), while the transition from led on to led off might involve calling ledOff(). Or, they might both involve calling toggleLed().