Arduino Stack Exchange is a question and answer site for developers of open-source hardware and software that is compatible with Arduino. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I'm trying to build a circuit that buzzes until a button is pressed, or it buzzes 5 times:

void alarm()
{
  int AlarmState = 0; //Just to get while loop going
  int count = 1;
  while(AlarmState==0)
  {
    int count = 1;
    AlarmState = digitalRead(alarmStop);                   
    digitalWrite(buzz,HIGH);
    AlarmState = digitalRead(alarmStop);
    delay(500);
    AlarmState = digitalRead(alarmStop);
    digitalWrite(buzz,LOW);
    AlarmState = digitalRead(alarmStop);
    delay(500);
    count++;
    if(count>=5){
       break;
    }
  }

I put a bunch of digitalRead's so it has a better chance of catching the button press while the loop is going, there's a better way to do that I'm sure but it's not my main concern right now.

This is part of a larger project, but I hope I've posted only the necessary code. The function runs, but does not break after count equals 5. Why?

share|improve this question
3  
Remove that int count = 1; from within the while-loop. – ott-- Apr 3 at 2:42
1  
It would make more sense to use a for loop to count to 5, and then if (AlarmState != 0) break;. – Nick Gammon Apr 3 at 4:17
    
@NickGammon for(unsigned char i = 0;i<5 && AlarmState != 0;i++){ ? Not sure if it fits the problem, but it's a little more readable (since you can see when the loop will end, within the statement/check). – Paul Apr 4 at 11:01
    
Well it should be && AlarmState == 0 to match the original post, but yes, that is another way. Actually the posted code in the question reads alarmStop multiple times which is a bit alarming (no pun intended). I suppose it depends how quickly you want to react to a change in alarmStop. – Nick Gammon Apr 4 at 21:45
    
Please note that with AlarmState = digitalRead(alarmStop); you just read the last value. Replace all those lines with AlarmState |= digitalRead(alarmStop);; this way as soon as one reading is 1 it will exit – frarugi87 May 4 at 10:26

Try This One its Simple:

void alarm()
{
  int count = 0;
  while ( digitalRead(alarmStop) && (count++ <5))
  {
    //A
    digitalWrite(buzz, HIGH);
    delay(500);
    digitalWrite(buzz, LOW);
    delay(500);
  }
}

But, There is a logical problem with this Code. Lets say that your code is at point A (metioned in comment) and at same time button is pressed. Now This code above will, still buzz and then stop.

To avoid This try using interrupts. To use interrupts on Arduino I prefer This Library. Download Here. Extract this and inside you will find a folder Named PinChangeInt. Copy This whole folder to your Documents\Arduino\libraries folder. Now, restart Arduino IDE and copy paste this code:

#include <PinChangeInt.h>

int buzz = 3;

bool alarmStatus = false;
#define PIN3 4
void pin3func() {
  digitalWrite(buzz, LOW);
  alarmStatus = true;
}

void setup() {
  pinMode(PIN3, INPUT); digitalWrite(PIN3, HIGH);
  PCintPort::attachInterrupt(PIN3, &pin3func, CHANGE);
}

int count = 0;
void loop() {
  if (count < 5)
  {
    if (!alarmStatus)
    {
      digitalWrite(buzz, HIGH);
    }
    delay(500);
    digitalWrite(buzz, LOW);
    delay(500);
  }
}

Because, of unavailability of hardware, I haven't tested this code. But, I hope It will work.

share|improve this answer
    
(On your second piece of code) #define PIN3 4 ? You might want to use a constant rather than a define. If you write PIN31 somewhere it will be replaced with 41. It's also a little weird to define PIN3 to 4. Also, int count = 0 is placed in between functions. Use a static variable (or a global variable placed on the top of your code). Plus I think you broke some logic since the count is never increased. The alarmStatus variable should be volatile since it's used in an interrupt (not sure if that's also on Arduino)? – Paul Apr 4 at 11:11

The problem with your code is your using delay(). This is a function that should most of the time be avoided for one simple reason: when the Arduino is executing the delay() instruction, it does nothing but wait. This means, among other things, that it cannot be reactive to user input. As outlined by ARK in his answer, interrupts could be used to overcome this problem. But you actually do not need them: the standard way to avoid delays is to use instead the millis() function to control timings. This approach is well described in the Blink without delay Arduino tutorial.

Here is how you can apply this technique to your alarm() function: In a nutshell, you can think of delay() to be roughly equivalent to this:

void naiveDelay(uint32_t ms)
{
    uint32_t start = millis();
    while (millis() - start < ms) {
        // do nothing
    }
}

The problem here is the “do nothing” part. You do not want your program to do nothing, you want it to monitor the “alarm stop” button. So the simple solution to your problem is to replace delay() by the code above, and then change it so that it monitors the button instead of doing nothing. Then you get something like this:

void alarm()
{
    for (int count = 0; count < 5; count++) {

        // start the buzzer
        digitalWrite(buzz, HIGH);

        // first delay
        uint32_t start = millis();
        while (millis() - start < 500) {
            if (digitalRead(alarmStop) == HIGH) {
                digitalWrite(buzz, LOW);  // stop the buzzer
                return;
            }
        }

        // stop the buzzer
        digitalWrite(buzz, LOW);

        // second delay
        start = millis();
        while (millis() - start < 500) {
            if (digitalRead(alarmStop) == HIGH) {
                return;  // buzzer already off
            }
        }
    }
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.