0

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?

  • 4
    Remove that int count = 1; from within the while-loop. – ott-- Apr 3 '16 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 '16 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 '16 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 '16 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 '16 at 10:26
2

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
            }
        }
    }
}
0

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.

  • (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 '16 at 11:11
0

I think the main problem with the code not breaking within the while() statement is what @ott pointed out above.

To expand on the exact problem, note that you have defined two different count variables in your code. Each has a different scope.

As soon as you enter the alarm function, you define an int count variable and initialize it. Then, when you enter the while () statement, you define a second int count variable and initialize it. These are NOT the same variable. This second variable is the one being used and unfortunately, is reinitialized each time through the loop. It will always have a value of 1 so your test for it becoming greater than or equal to 5 will never test true. Thus no break.

If you comment out the second int count line, your loop should then work and break as expected. it should look like this:

  while(AlarmState==0)
  {
  //  int count = 1;
    AlarmState = digitalRead(alarmStop);                   

Others have pointed out other improvements that can be made to the code but the two variables of the same name with different scopes appear to be the specific cause of the while() not breaking.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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