Encountering Issues with If Else Statement & PIR Sensor

So I am modifying an Instructable I found to work as a door/motion detector alarm for my apartment.

I want to get the code working properly just printing to the normal serial port before i go out and spend money on the GPRS shield and a SIM Card.

The issue is that the Arduino keeps flip flopping between the else if and else statements even when the motionPin reads low.

It keeps printing

DOOR IS CLOSED
MESSAGE SENT
DOOR IS CLOSED
MESSAGE SENT

it should only print

DOOR IS CLOSED

then do nothing until the motion sensor is tripped, then print

DOOR IS OPEN
MESSAGE SENT

then

DOOR IS CLOSED again.

I am using a PIR sensor from Radioshack so i guess i may need to include the LockLow function into this code, but I am not sure. Any help would be awesome.

#include <SoftwareSerial.h>

SoftwareSerial SIM900(7, 8);

SoftwareSerial mySerial(10, 11); //rx, tx

boolean doorOpen = false;

boolean messageSent = false;

int motionPin = 3; //pin detecting motion

int ledPin = 13; //red LED on pin 13

int calibrationTime = 5;

void setup() {

pinMode(motionPin, INPUT);

SIM900.begin(19200);

Serial.begin(19200);

Serial.println("BEGINNING SERIAL COMMUNICATION");

Serial.print("calibrating sensor ");
for(int i = 0; i < calibrationTime; i++){
Serial.print(".");
delay(1000);
}
Serial.println(" done");
Serial.println("SENSOR ACTIVE");

SIM900power();

//delay(20000); // give time to log on to network.

digitalWrite(motionPin, LOW);

messageSent = false;

}

void SIM900power() { // software equivalent of pressing the GSM shield "power" button

digitalWrite(9, HIGH);

delay(1000);

digitalWrite(9, LOW);

delay(5000);

}

void sendSMS() {

SIM900.print("AT+CMGF=1\r"); // AT command to send SMS message

delay(100);

SIM900.println("AT + CMGS = "+16039983462""); // recipient's mobile number, in international format

delay(100);

SIM900.println("APARTMENT SECURITY ALERT -- THE FRONT DOOR HAS BEEN OPENED"); // message to send

delay(100);

SIM900.println((char)26); // End AT command with a ^Z, ASCII code 26

delay(100);

SIM900.println();

delay(5000); // give module time to send SMS

}

void loop() {

if (digitalRead(motionPin) == HIGH) { //if motionPin is HIGH the door has moved, and is therefore opened

Serial.println("DOOR IS OPENED");

doorOpen = true;

delay(1000);

digitalWrite(ledPin, HIGH);

delay(1000);

digitalWrite(ledPin, LOW);

}

else if (digitalRead(motionPin) == LOW) { // door is closed so do reset messageSent and doorOpen states

doorOpen = false; //door is not open

messageSent = false; //reset the message status

Serial.println("DOOR IS CLOSED");

}

else (digitalRead(motionPin == HIGH) && (messageSent == false)); { //if the door is open, and the messageSent state is false, send a message and set the state to true, so we dont send one again.

//delay(1000); this will give you a one second delay to turn the unit off -- for me and my gf when entering the house when the alarm is active. not needed in the testing phase.

sendSMS();

Serial.println("MESSAGE SENT");

//messageSent = true;

messageSent = true;

}

}

  else (digitalRead(motionPin == HIGH) && (messageSent == false)); { //if the door is open, and the messageSent state

That line is nonsense. Perhaps you meant:

  else if (digitalRead(motionPin) == HIGH && (messageSent == false)) { //if the door is open, and the messageSent state

But that line won't do anything because you already covered both the "digitalRead(motionPin) == HIGH" and "digitalRead(motionPin) == LOW" cases so this 'else' line will never be executed. If you want to check for both
"digitalRead(motionPin) == HIGH" and "messageSent == false" you should put:
if (messageSent == false) INSIDE the "digitalRead(motionPin) == HIGH" block.

so like this, inside the first digitalRead block?

#include <SoftwareSerial.h>

SoftwareSerial SIM900(7, 8);

SoftwareSerial mySerial(10, 11); //rx, tx

boolean doorOpen = false;

boolean messageSent = false;

int motionPin = 3; //pin detecting motion

int ledPin = 13; //red LED on pin 13

int calibrationTime = 5;

void setup() {

pinMode(motionPin, INPUT);

SIM900.begin(19200);

Serial.begin(19200);

Serial.println("BEGINNING SERIAL COMMUNICATION");

Serial.print("calibrating sensor ");
for(int i = 0; i < calibrationTime; i++){
Serial.print(".");
delay(1000);
}
Serial.println(" done");
Serial.println("SENSOR ACTIVE");

SIM900power();

//delay(20000); // give time to log on to network.

digitalWrite(motionPin, LOW);

messageSent = false;
}

void SIM900power() { // software equivalent of pressing the GSM shield "power" button

digitalWrite(9, HIGH);

delay(1000);

digitalWrite(9, LOW);

delay(5000);

}

void sendSMS() {

SIM900.print("AT+CMGF=1\r"); // AT command to send SMS message

delay(100);

SIM900.println("AT + CMGS = "+16039983462""); // recipient's mobile number, in international format

delay(100);

SIM900.println("APARTMENT SECURITY ALERT -- THE FRONT DOOR HAS BEEN OPENED"); // message to send

delay(100);

SIM900.println((char)26); // End AT command with a ^Z, ASCII code 26

delay(100);

SIM900.println();

delay(5000); // give module time to send SMS

}

void loop() {

if (digitalRead(motionPin) == HIGH) { //if motionPin is HIGH the door has moved, and is therefore opened

Serial.println("DOOR IS OPENED");

doorOpen = true;

if (messageSent == false) {

//delay(1000); this will give you a one second delay to turn the unit off -- for me and my gf when entering the house when the alarm is active. not needed in the testing phase.

sendSMS();

Serial.println("MESSAGE SENT");

messageSent = true;

}

digitalWrite(ledPin, HIGH);

delay(1000);

digitalWrite(ledPin, LOW);

}

else (digitalRead(motionPin) == LOW); { // door is closed so do reset messageSent and doorOpen states

doorOpen = false; //door is not open

messageSent = false; //reset the message status

Serial.println("DOOR IS CLOSED");

}

}

I am surprised you can even compile that.

why wouldnt i be able to?

[quote author=Ryan J Blajda link=topic=256109.msg1811688#msg1811688 date=1405889870]
why wouldnt i be able to?
[/quote]All the emoticons.

-___- sorry i forgot to put it in code mode.

You have a semicolon after the boolean test expression in your else clause. But even if you remove it, the program will never get there.

Two things about PIRs:

  1. They respond to heat sources not motion. So an opening door won't necessarily tigger it, but if a person walks through the door, then, yes.

  2. They don't always stay high as long as a heat source is present. Some go high for a sec or two, then low, some continuously cycle high-low while a heat source is present, some stay high continuously. Sometimes there is jumper to control the behavior.

HTH,
w

Two things about PIRs:

  1. They respond to heat sources not motion. So an opening door won't necessarily tigger it, but if a person walks through the door, then, yes.

  2. They don't always stay high as long as a heat source is present. Some go high for a sec or two, then low, some continuously cycle high-low while a heat source is present, some stay high continuously. Sometimes there is jumper to control the behavior.

HTH,
w

i know how a PIR sensor works, i just didnt explain it properly.

You have a semicolon after the boolean test expression in your else clause. But even if you remove it, the program will never get there.

not sure i understand why the program wont "get there"

Getting closer but this part doesn't make sense:

  else (digitalRead(motionPin) == LOW); { // door is closed so do reset messageSent and doorOpen states
    doorOpen = false; //door is not open
    messageSent = false; //reset the message status
    Serial.println("DOOR IS CLOSED");
  }

The 'else' clause should not be followed by a boolean expression because the 'else' is always 'anything that does not match the 'if'. Since the 'else' can only be followed only by a statement or block {} the block it's taking that boolean expression to be an "expression statement" which is the sole contents of the 'else'. That is why you had to put a semicolon after it, to terminate the statement.

What you wrote is equivalent to writing:

  else {
      digitalRead(motionPin) == LOW;  // This does nothing.  It reads the pin, compares to LOW and throws away the answer
   }
   // Do this every time through loop():
   { // door is closed so do reset messageSent and doorOpen states
    doorOpen = false; //door is not open
    messageSent = false; //reset the message status
    Serial.println("DOOR IS CLOSED");
  }

well thats the only other state the sensor would be in, so i want to reset the states of the door being opened, since if the pin is reading low the door is now closed, and i would want to send another message if the door opens again, so the messageSent state needs to be reset as well.

Should i add a variable to stop it from repeatedly printing DOOR IS CLOSED so it only prints it once?

I am a novice regarding coding, but i understand the basics for the most part. i do mostly html and PHP coding, so i get some of the similarities.

Yes. Sort of like your "message_sent" flag. If the door is closed and was open last time you looked, print the 'closed' message:

void loop() {
      static boolean door_previous_state = LOW;
      boolean door_state =digitalRead(motionPin);

     // Only do things when the state changes
     if (door_state != door_previous_state) {
        if (door_state == HIGH) {  // Door just opened
            Serial.println("DOOR IS OPENED");
            sendSMS();
            Serial.println("MESSAGE SENT");
        } else { // Door just closed
            Serial.println("DOOR IS CLOSED");
        }
    }
    door_previous_state = door_state;
}

ah i see. thats what i was thinking i should do that way it doesnt print door is closed a million times. thanks a bunch!