Fridge door alarm subtle bug

Hi Community,

I have built a fridge door alarm using an ATTiny85 Digispark, it works as expected with the exception of a single call of the buzzer function piezoBuzzer(); five minutes after a door is opened and closed which is driven by the long tps_before_activation = 300000;.
I based it off some code from intructables: https://www.instructables.com/id/Fridges-door-alarm/ and modified it to suit my needs.

The code:

// Attiny85 Digispark
const int interruptPin = 0;
const int buzzerPin = 1;
const int testPin = 4;

// customizable variables
long tps_before_activation = 300000;          // time before buzzing (300000 milliseconds = 5 minutes)
int tps_between_activations = 5000;           // time between 2 buzz (5 seconds)
int tps_minimum_activation = 0;               // minimum buzzing time duration
int tps_maximum_activation = 300;             // maximum buzzing time duration (0.3 second)

// Do not touch variables
int status_fridge = 0;                        // variable for reading the magnetic switch status
int closed = 1;                               // 1 = door was closed
int open_counter = 0;                         // door's open counter
int status_testPin = 0;                       // test pin set to zero

void setup() {
  pinMode(buzzerPin, OUTPUT);
  pinMode(interruptPin, INPUT);
  pinMode(testPin, INPUT);
  digitalWrite(buzzerPin, LOW);
}

void loop() {
  status_fridge = digitalRead(interruptPin);
  status_testPin = digitalRead(testPin);

  if (status_fridge == HIGH) {
    if (closed == 1) {
      delay(tps_before_activation);
      closed = 0;
      status_testPin = 0;
    }
    digitalWrite(buzzerPin, LOW);
    delay(500);
    piezoBuzzer();                              // turn buzzer on:
    //digitalWrite(buzzerPin, HIGH);            // used if using the tone library for sound
    delay(tps_minimum_activation + open_counter);
    digitalWrite(buzzerPin, LOW);
    delay(tps_between_activations);
    if (open_counter <= tps_maximum_activation) open_counter = open_counter + 25;
  }

  else if (status_testPin == HIGH) {
    piezoBuzzer();                              // test the buzzer and also that the loop is running
    digitalWrite(testPin, LOW);
    status_testPin = 0;                         // test line of code to remove the single beep 5 minutes after door open event
    delay(5000);
  }

  else {
    digitalWrite(buzzerPin, LOW);              // turn buzzer off:
    closed = 1;
    open_counter = 0;
    status_testPin = 0;
  }
  digitalWrite (buzzerPin, LOW);
  delay(500);
}

void piezoBuzzer() {
  digitalWrite (buzzerPin, HIGH);
  delay (100);
  digitalWrite (buzzerPin, LOW);
  delay (200);
  digitalWrite (buzzerPin, HIGH);
  delay (100);
  digitalWrite (buzzerPin, LOW);

  //static void tone(uint8_t note) {
//  // Fixed octave
//  TCCR0B = (TCCR0B & ~((1 << CS02) | (1 << CS01) | (1 << CS00))) | _BV(CS01);
//
//  // Use notes from 24 to 18 (highest)
//  OCR0A = note - 1;
//}

  // Use this section if using an actual passive piezo to play a tune
  //    tone(buzzerPin, 1000, 500);
  //    delay(500);
  //    noTone(buzzerPin);
  //    delay(250);
}

The intended goal is to have it beep continuously after five minutes to alert me that a door is open with the ability to press the test button (pin 4) to test its alive.

I am pretty sure it is a logic issue that I am not seeing as to why it is waiting five minutes in the code to produce a single beep after a door has been opened then closed. I would like it to still retain the test button on pin 4 to ensure the thing is alive and running the loop.

Thanks for having a look!

Do you have pulldown resistors on the input pins?
Don’t know if I would trust an electronics teacher who can’t spell “schematic”.

your code is hard to follow. it can be simpler

i don’t understand why after the last else statement in loop() you (always) set buzzerpin LOW. shouldn’t it depend on the conditions above?

the delays prevent further processing. you want to check if 5 minutes has elapsed since the door was open, but you don’t want to wait that long without allowing any further processing (such as checking if the test button is pressed).

each iteration of loop() should check if the door is open. if it was closed, i suggest capturing a timestamp using millis() (e.g. msecOpen = millis()) and set closed to zero as you do, without delaying for 5 minutes.

subsequent processing under the condition for the door being open and ! closed (closed == 0), check if 5 minutes has elapsed since the door was opened by comparing the elapsed time (millis() - msecOpen > 30000), if it has, turn the buzzer on.

the else condition, implying that the door is closed, does what it needs to door: turn off the buzzer and set closed.

if the test pin is pressed, you can turn the buzzer on on and delay for 5 sec. there’s no need to turn it off since, since it will be turned off in the else condition during the next iterations of loop()

Everything gcjr said except he said better than I might have done.

I suggest you study: Using millis for timing Demonstration for several things at the same time Both of which give you advice on structuring code far better than you have done and meeting all the suggestions gcjr has made.

Also , in your code:

digitalWrite(testPin, LOW);

Look at the reference :

If the pin is configured as an INPUT, digitalWrite() will enable (HIGH) or disable (LOW) the internal pullup on the input pin. It is recommended to set the pinMode() to INPUT_PULLUP to enable the internal pull-up resistor.

Regards, bidouilleelec

Hi Community,

After having a good friend look over my code, he suggested most of what you have all pointed too. He is an Electrical Eng. and uses Atmel's AVR Studio for his programming, which is a whole level again for a poor Civil Eng. like me :confused:

I must admit, I used an example and put too much trust in the existing logic and thought I had enough understanding for it to work when the delay was set to 5 seconds during testing.

I agree with all the comments made, oh and yes I have pull-down resistors on the input pins. As for the millis suggestion, it was one of the first ones my friend had made and he had suggested this site: http://forward.com.au/pfod/ArduinoProgramming/TimingDelaysInArduino.html

I am pretty confident the boards wiring is correct and will try to write something from bare bones and include the millis instead of the delay.

Many thanks for your review and suggestions!

Hi Community,

I have totally rewritten the code to use the millis() function and it is now working as expected. I am posting it here for future posterity and that it may assist others in their endeavours. It’s probably not the most elegant solution but I am happy with it :slight_smile:

// Attiny85 Digispark & or Uno / Nano etc..

const int doorPin = 0;      // 0 on Digispark 5 on Uno / Nano etc..
const int buzzerPin = 1;    // 1 on Digispark 13 on Uno / Nano etc..
const int testPin = 4;      // 4 on Digispark & or Uno / Nano etc..
int status_testPin;
int status_doorOpen;
int counter = 0;

boolean SerialDebug = false; // set this to true to get serial print comments for debugging

unsigned long doorOpenDelay = 3000; // 300000 milliseconds = 5 minutes timeout before buzzer sounds
unsigned long currentTime = 0;
unsigned long previousTime = 0;
volatile int doorOpen = LOW;


// function to see if the door is open or closed
void fridgeDoorOpen() {

  doorOpen = digitalRead(doorPin);

  // if the door is closed set the previousTime variable to the current millis() on the internal timer
  if ( doorOpen == LOW ) {
    previousTime = millis();
  }

  // if the door is open set the currentTime variable to the current millis() on the internal timer
  if ( doorOpen == HIGH ) {
    currentTime = millis();

    if (SerialDebug) {
      counter = counter + 1;
      Serial.print("count ");
      Serial.println(counter);
    }

    // as the door opens it goes from low to high: compare the currentTime - previousTime and see if it is greater than doorOpenDelay time
    // if not keep checking until it is then move onto the alarm in the do while
    if ( currentTime - previousTime > doorOpenDelay ) {
      currentTime = millis();

      if (SerialDebug) {
        Serial.println("currentTime - previousTime  >= doorOpenDelay");
      }

      // while the currentTime - previousTime is greater than doorOpenDelay time and the door is still open sound the alarm
      do {
        soundAlarm();
        doorOpen = digitalRead(doorPin); // recheck the door if it is open or closed
        if (SerialDebug) {
          Serial.println("soundAlarm(); called");
        }
      } while ( doorOpen == HIGH );

      // when the door closes
      do {
        doorOpen = digitalRead(doorPin); // recheck the door if it is open or closed

        if (SerialDebug) {
          counter = 0;
          Serial.println("Exited the do loop doorPin is now LOW");
          Serial.print("doorOpen = ");
          Serial.println(doorOpen);
          Serial.println("doorOpen = digitalRead(doorPin);");
        }
        break; // totally exit the nested if statments and do while loops
      } while ( doorOpen == LOW );
    }
  }
}

// Sound the alarm function
void soundAlarm() {

  if (SerialDebug) {
    Serial.println("soundAlarm triggered");
  }

  digitalWrite (buzzerPin, HIGH);
  delay (200);
  digitalWrite (buzzerPin, LOW);
  delay (100);
  digitalWrite (buzzerPin, HIGH);
  delay (200);
  digitalWrite (buzzerPin, LOW);
  delay (2000);
  return;
}

// Test alarm function
void testBuzzer() {
  status_testPin = digitalRead(testPin);
  if (status_testPin == HIGH) {
    soundAlarm();

    if (SerialDebug) {
      Serial.println("Test triggered");
    }
  }
}

void setup() {
  pinMode(buzzerPin, OUTPUT);
  pinMode(doorPin, INPUT);
  pinMode(testPin, INPUT);
  digitalWrite(buzzerPin, LOW);
  if (SerialDebug) {
    Serial.begin(9600);
  }
}

void loop() {
  fridgeDoorOpen();
  testBuzzer();
}