Code problem with using 2 relays *Update

Hello,
***See updated code at bottom of post.

Put together some code that activates relays when a motion is detected using a PIR sensor.
First tried the code with one relay (to turn ON/Off my computer) and works well.

// Turning on electrical systems on my robot via motion PIR sensor and Seeed relay module.
// Give a greeting, hello, etc.. (speech), which I have setup already on my mini ITX PC M/B.
// Then, if there is no motion detected after a certain time (15 minutes),
// shut down PC M/B followed by the robots electrical system.
// The arduino in this setup will be powered by a 9V battery.

// Some of the code used (PIR), I believe was from Kristian Gohlke
// and the Seeed relay demo code.


const int relayComPin = 6;    //Digital pin to control the relay for the computer
//the time we give the sensor to calibrate (10-60 secs according to the datasheet)
int calibrationTime = 30;        

//the time when the sensor outputs a low impulse
long unsigned int lowIn;         

//the amount of milliseconds the sensor has to be low 
//before we assume all motion has stopped
long unsigned int pause = 10000;  // Duration of no motion time to shut off system.  15min. pause or wait  (900000mS)

boolean lockLow = true;
boolean takeLowTime;  

const int pirPin = 9;    //the digital pin connected to the PIR sensor's output


void setup(){
  Serial.begin(9600);
  pinMode(pirPin, INPUT);
  pinMode(relayComPin, OUTPUT);
  digitalWrite(pirPin, LOW);

}

void loop(){


  if(digitalRead(pirPin) == HIGH){
    if(lockLow){  
      //makes sure we wait for a transition to LOW before any further output is made:
      lockLow = false;            
      Serial.println("---");
      Serial.print("motion detected at ");
      Serial.print(millis()/1000);
      Serial.println(" sec"); 
      delay(50);

    }      
    if(lockLow){  
      //makes sure we wait for a transition to LOW before any further output is made:
      lockLow = false;            

    }         
    takeLowTime = true;
    digitalWrite(relayComPin,HIGH);  //Turns on computer

  }



  if(digitalRead(pirPin) == LOW){       

    if(takeLowTime){
      lowIn = millis();          //save the time of the transition from high to LOW
      takeLowTime = false;       //make sure this is only done at the start of a LOW phase
      digitalWrite(relayComPin,HIGH);  //Turns off computer
    }
    //if the sensor is low for more than the given pause, 
    //we assume that no more motion is going to happen
    if(!lockLow && millis() - lowIn > pause){  
      //makes sure this block of code is only executed again after 
      //a new motion sequence has been detected

      lockLow = true;                        

      Serial.print("motion ended at ");      //output
      Serial.print((millis() - pause)/1000);
      Serial.println(" sec");
      delay(50);
      digitalWrite(relayComPin,LOW);  //opens relay for another cycle
    }
  }
}

Here is where I added the second relay to control the electrical system on my robot. Compiles fine but when I run it, I get a quick double click from the relay(s) and they remain ON instead of turning back OFF or NO(normally open).

// Turning on electrical systems on my robot via motion PIR sensor and Seeed relay module.
// Give a greeting, hello, etc.. (speech), which I have setup already on my mini ITX PC M/B.
// Then, if there is no motion detected after a certain time (15 minutes),
// shut down PC M/B followed by the robots electrical system.
// The arduino in this setup will be powered by a 9V battery.

// Some of the code used (PIR), I believe was from Kristian Gohlke
// and the Seeed relay demo code.

const int relayElePin = 7;  //Digital pin to control the relay for the electrical system
const int relayComPin = 6;  //Digital pin to control the relay for the computer
//the time we give the sensor to calibrate (10-60 secs according to the datasheet)
int calibrationTime = 30;        

//the time when the sensor outputs a low impulse
long unsigned int lowIn;         

//the amount of milliseconds the sensor has to be low 
//before we assume all motion has stopped
long unsigned int pause = 10000;  // Duration of no motion time to shut off system.  15min. pause or wait  (900000mS)

boolean lockLow = true;
boolean takeLowTime;  

const int pirPin = 9;    //the digital pin connected to the PIR sensor's output


void setup(){
  Serial.begin(9600);
  pinMode(pirPin, INPUT);
  pinMode(relayComPin, OUTPUT);
  pinMode(relayElePin, OUTPUT);
  digitalWrite(pirPin, LOW);

}

void loop(){


  if(digitalRead(pirPin) == HIGH){
    if(lockLow){  
      //makes sure we wait for a transition to LOW before any further output is made:
      lockLow = false;            
      Serial.println("---");
      Serial.print("motion detected at ");
      Serial.print(millis()/1000);
      Serial.println(" sec"); 
      delay(50);

    }      
    if(lockLow){  
      //makes sure we wait for a transition to LOW before any further output is made:
      lockLow = false;            

    }         
    takeLowTime = true;
    digitalWrite(relayComPin,HIGH);  //Turns on electrical system
    delay(5000);  //Adds a bit of time...
    digitalWrite(relayComPin,HIGH);  //Turns on computer

  }



  if(digitalRead(pirPin) == LOW){       

    if(takeLowTime){
      lowIn = millis();          //save the time of the transition from high to LOW
      takeLowTime = false;       //make sure this is only done at the start of a LOW phase
      digitalWrite(relayComPin,HIGH);  //Turns off computer
      delay(10000);  //Adds a bit of time to complete shutdown of computer
      digitalWrite(relayElePin,HIGH);  //Turns off system

    }
    //if the sensor is low for more than the given pause, 
    //we assume that no more motion is going to happen
    if(!lockLow && millis() - lowIn > pause){  
      //makes sure this block of code is only executed again after 
      //a new motion sequence has been detected

      lockLow = true;                        

      Serial.print("motion ended at ");      //output
      Serial.print((millis() - pause)/1000);
      Serial.println(" sec");
      delay(50);
      digitalWrite(relayElePin,LOW);  //opens relay #1 for another cycle
      digitalWrite(relayComPin,LOW);  //opens relay #2 for another cycle
    }
  }
}

Any help, suggestions appreciated.

t

 digitalWrite(relayComPin,HIGH);  //Turns on electrical system
    delay(5000);  //Adds a bit of time...
    digitalWrite(relayComPin,HIGH);

Once a pin is set high, it normally stays high.

Argh...lol
Dumb mistake hehe
Thanks AWOL :slight_smile:

t

Ok, I have the code working pretty good.
Have one problem:
I want to have a delay of 10 seconds of sensed motion before activating the relays.
Atm, any motion trigger will set off the relays (1 sec., etc... will trigger the relays).
Here's the area of the code I believe needs some adjustment:

if(digitalRead(pirPin) == HIGH){
    if(lockLow){  

      lockLow = false;            
      //Wait for PIR sensor to be ON for a certain time before activating both relays (10sec).
      delay(10000);  
      //First turn on electrical system (Will remain on until the end or no more movement).
      digitalWrite(relayPin1,HIGH);
      //Wait a few seconds before turning on computer.
      delay(5000);
      //Turn on relay for computer (will be a momentary contact).
      digitalWrite(relayPin2,HIGH);  //Turns on computer.
      delay(200);
      //Turn off relay for computer.
      digitalWrite(relayPin2,LOW);

      Serial.println("---");
      Serial.print("motion detected at ");
      Serial.print(millis()/1000);
      Serial.println(" sec"); 
      delay(50);
    }

    if(lockLow){  
      //makes sure we wait for a transition to LOW before any further output is made.
      lockLow = false;            
    }         
    takeLowTime = true;
  }

Here's the entire code:

// Turning on electrical systems on my robot via motion PIR sensor and Seeed relay module.
// Give a greeting, hello, etc.. (speech), which I have setup already on my mini ITX PC M/B.
// Then, if there is no motion detected after a certain time (15 minutes),
// shut down PC M/B followed by the robots electrical system.
// The arduino in this setup will be powered by a 9V battery.
//18Nov12

const int relayPin1 = 7;    //Digital pin to control the relay for the electrical system 'relay #1'.
const int relayPin2 = 6;    //Digital pin to control the relay for the computer 'relay #2'.
//the time we give the sensor to calibrate (10-60 secs according to the datasheet).
int calibrationTime = 30;        

//the time when the sensor outputs a low impulse.
long unsigned int lowIn;         

//The amount of milliseconds the PIR sensor has to be low before we assume all motion has stopped  
//to Shutdown (computer (relay #2)) AND (electrical system (relay #1)).  
long unsigned int pause = 10000;  //Probably will end up using 15min. (900,000mS).  

boolean lockLow = true;
boolean takeLowTime;  

const int pirPin = 9;    //the digital pin connected to the PIR sensor's output.


void setup(){
  Serial.begin(9600);
  pinMode(pirPin, INPUT);
  pinMode(relayPin1, OUTPUT);
  pinMode(relayPin2, OUTPUT);
  digitalWrite(pirPin, LOW);

}

void loop(){

  if(digitalRead(pirPin) == HIGH){
    if(lockLow){  

      lockLow = false;            
      //Wait for PIR sensor to be ON for a certain time before activating both relays (10sec).
      delay(10000);  
      //First turn on electrical system (Will remain on until the end or no more movement).
      digitalWrite(relayPin1,HIGH);
      //Wait a few seconds before turning on computer.
      delay(5000);
      //Turn on relay for computer (will be a momentary contact).
      digitalWrite(relayPin2,HIGH);  //Turns on computer.
      delay(200);
      //Turn off relay for computer.
      digitalWrite(relayPin2,LOW);

      Serial.println("---");
      Serial.print("motion detected at ");
      Serial.print(millis()/1000);
      Serial.println(" sec"); 
      delay(50);
    }

    if(lockLow){  
      //makes sure we wait for a transition to LOW before any further output is made.
      lockLow = false;            
    }         
    takeLowTime = true;
  }



  if(digitalRead(pirPin) == LOW){       

    if(takeLowTime){
      lowIn = millis();          //save the time of the transition from high to LOW.
      takeLowTime = false;       //make sure this is only done at the start of a LOW phase.
    }
    //If the sensor is low for more than the given pause, 
    //we assume that no more motion is going to happen.
    if(!lockLow && millis() - lowIn > pause){  
      //Makes sure this block of code is only executed again after 
      //a new motion sequence has been detected.

      lockLow = true;                        

      //Turns off computer, momentary on and off on computer relay.
      digitalWrite(relayPin2,HIGH);  
      delay(200);
      digitalWrite(relayPin2,LOW);
      //Wait a few seconds      
      delay(8000);
      //Then turn off electrical system.      
      digitalWrite(relayPin1,LOW);
      delay(500);



      Serial.print("motion ended at ");  
      Serial.print((millis() - pause)/1000);
      Serial.println(" sec");
      delay(500);
    }
  }
}

t

      //Wait for PIR sensor to be ON for a certain time before activating both relays (10sec).
      delay(10000);

That code does NOT do what the comment says. If the PIR sensor is HIGH, that code will wait 10 seconds, and then do whatever it was going to, regardless of whether the sensor is still HIGH, or not.

If you want 10 seconds of continuous HIGH to trigger the action, you need to detect transitions. Record the time when the sensor goes HIGH. Record when the sensor goes LOW. Periodically, see if the value has been HIGH long enough.

Get rid of all delay() calls from your code, now, before you hurt yourself. Look (again) at the blink without delay example. You can not accomplish what you want with any calls to delay (or any that are more than a few milliseconds in length).

PaulS:
If you want 10 seconds of continuous HIGH to trigger the action, you need to detect transitions. Record the time when the sensor goes HIGH. Record when the sensor goes LOW. Periodically, see if the value has been HIGH long enough.

Get rid of all delay() calls from your code, now, before you hurt yourself. Look (again) at the blink without delay example. You can not accomplish what you want with any calls to delay (or any that are more than a few milliseconds in length).

Thanks for the reply Paul.
Earlier this week I bought a couple of books which I've started reading up on: Arduino Cookbook & Sams Teach Yourself C++.

Also, went back over the blink w/o delay sketch line by line.
Before I ask about this, I had a few questions on your recent reply.

When you say:
"If you want 10 seconds of continuous HIGH to trigger the action, you need to detect transitions. Record the time when the sensor goes HIGH. Record when the sensor goes LOW. Periodically, see if the value has been HIGH long enough."
Not sure if I understand this. The PIR sensor sends 0 and 1's (or OFF and ON's) to the Arduino depending on motion.
Let's say you set the intitial state of the pir sensor to LOW (0). Why can't you start recording when the pir sensor goes HIGH (1)? And then after 10 seconds, trigger the relay(s). Once the relay(s) are triggered, start recording the pir sensor LOW values until after 15 minutes (no movement detected for 15 min.), trigger relays again to turn hardware off.

Ok, looking again at the blink w/o delay sketch (Using LED on D13 and PIR on D9 without relays).

// Blink without Delay


// constants won't change. Used here to 
// set pin numbers:
const int ledPin = 13;      //digital pin for LED
const int pirPin = 9;        //digital pin for PIR sensor
// Variables will change:
int ledState = LOW;             //ledState used to set the LED
int pirState = LOW;             //pirState used to set PIR sensor
long previousMillis = 0;        // will store last time LED was updated

// the follow variables is a long because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.

//long interval = 1000;           //(won't use interval) to blink LED (milliseconds)
long duration = 10000;           //duration after PIR goes HIGH to trigger (relays)


void setup() {
  // set the digital pin as output:
  pinMode(ledPin, OUTPUT);      
  pinMode(pirPin, INPUT);


}

void loop()
{
  // here is where you'd put code that needs to be running all the time.

  //  
  //  
  //  
  // Could the following be used for the PIR sensor?
  unsigned long currentMillis = millis();

  if(currentMillis - previousMillis > duration) {
    // save the last time you blinked the LED 
    previousMillis = currentMillis;   

    // if the LED is off turn it on and vice-versa:
    if (ledState == LOW)
      ledState = HIGH;
    else
      ledState = LOW;

    // set the LED with the ledState of the variable:
    digitalWrite(ledPin, ledState);
  }
}

t

Let's say you set the intitial state of the pir sensor to LOW (0).

Let's say you don't. You can't set the state of an input device. It will be what it is.

Why can't you start recording when the pir sensor goes HIGH (1)?

You can. What are you recording, though?

And then after 10 seconds, trigger the relay(s).

You can. But, is that is happen only if there have been no transitions from the PIR sensor pin in that time? If you, how will you know that there have been no transitions for 10 seconds, unless you know when the last transition was?

Once the relay(s) are triggered, start recording the pir sensor LOW values until after 15 minutes (no movement detected for 15 min.), trigger relays again to turn hardware off.

Why would you want to "record" over and over the state of a pin that is not changing?

All that is required is to know when the transition occurred. If the transition is to "someone/something is here", and no "nobody's here" transition occurs until after 10 seconds have gone by, activate the relays and record the time.

When do you want to shut off the relays? As soon as the "nobody's here" transition occurs? After 15 minutes, regardless of whether the "nobody's here" transition has occurred?

PaulS:

Let's say you set the intitial state of the pir sensor to LOW (0).

Let's say you don't. You can't set the state of an input device. It will be what it is.

Ok, I understand this.

Why can't you start recording when the pir sensor goes HIGH (1)?

You can. What are you recording, though?

You start recording the time it goes HIGH or the moment the PIR sensor outputs 1's.

And then after 10 seconds, trigger the relay(s).

You can. But, is that is happen only if there have been no transitions from the PIR sensor pin in that time? If you, how will you know that there have been no transitions for 10 seconds, unless you know when the last transition was?

Yes, Pir sensor has to be continuously HIGH (sending 1's) for 10 seconds.
When the PIR sensor is HIGH it's sending out 1's, if it goes LOW it will send out a 0.

Once the relay(s) are triggered, start recording the pir sensor LOW values until after 15 minutes (no movement detected for 15 min.), trigger relays again to turn hardware off.

Why would you want to "record" over and over the state of a pin that is not changing?

But the state might be. There maybe motion or no motion during the relay stage. Maybe a continous loop testing how long the PIR sensor is LOW. If the duration of this LOW reaches 15min. it again triggers the relays(in reverse order).

All that is required is to know when the transition occurred. If the transition is to "someone/something is here", and no "nobody's here" transition occurs until after 10 seconds have gone by, activate the relays and record the time.

When do you want to shut off the relays? As soon as the "nobody's here" transition occurs? After 15 minutes, regardless of whether the "nobody's here" transition has occurred?

Triggering the relays again(in reverse order). To shutdown the computer, then the electrical system. I would like it to do this when there is a 15 min. continuous LOW (no movement) from the PIR sensor.

Also, what you were saying before about delay:
Using delay() pauses the execution of the sketch, using millis() is better as it allows you to perform other tasks within the delay period or the sketch?

t

You start recording the time it goes HIGH or the moment the PIR sensor outputs 1's.

I guess I think of "start recording" to mean doing something over and over recording new data. It doesn't appear to mean the same to you. You appear to mean doing something once. So, the "start recording" phrase should be "record", which is what I said you needed to do way back at the beginning.

Yes, Pir sensor has to be continuously HIGH (sending 1's) for 10 seconds.
When the PIR sensor is HIGH it's sending out 1's, if it goes LOW it will send out a 0.

So, you need to detect when the transition from 0 to 1 occurs, and record that time. You need to detect when the transition from 1 to 0 occurs, and record that time.

Separately, you need to determine that the 0 to 1 transition has occurred and determine how long ago that happened. If it was long enough ago (the sensor has been HIGH for 10 seconds), do something.

But the state might be. There maybe motion or no motion during the relay stage. Maybe a continous loop testing how long the PIR sensor is LOW. If the duration of this LOW reaches 15min. it again triggers the relays(in reverse order).

I sense that you are trying to write all the code inside the if-low-to-high-transition block or the if-high-to-low transition block. That is not the way to do it.

You have a bit of code to read the current sensor state, and compare that to the previous state. If a transition has occurred, record the time as a start time or a stop time, depending on which transition occurred. That is all that that bit of code does.

Then, you have some more code to determine the current time, and to determine if now minus then (if there is a then) exceeds some threshold. If it does,do something.

This could be to determine if now minus then (when the sensor went HIGH) has exceeded 10 seconds. If so, turn on a relay.

It could be to determine if now minus then (when the sensor went LOW) has exceeded 15 minutes. If so, then off a relay.

It shouldn't take more than about 20 lines of code to do all this. 30 with comments.

PaulS:

You start recording the time it goes HIGH or the moment the PIR sensor outputs 1's.

I guess I think of "start recording" to mean doing something over and over recording new data. It doesn't appear to mean the same to you. You appear to mean doing something once. So, the "start recording" phrase should be "record", which is what I said you needed to do way back at the beginning.

I'm probably wrong hehe :slight_smile:
If it's sending 1's continuously, it's still 'new data?

Yes, Pir sensor has to be continuously HIGH (sending 1's) for 10 seconds.
When the PIR sensor is HIGH it's sending out 1's, if it goes LOW it will send out a 0.

So, you need to detect when the transition from 0 to 1 occurs, and record that time. You need to detect when the transition from 1 to 0 occurs, and record that time.

Separately, you need to determine that the 0 to 1 transition has occurred and determine how long ago that happened. If it was long enough ago (the sensor has been HIGH for 10 seconds), do something.

So maybe something like recording:

if(millis() - previousMillis() > duration)

where I could assign duration = 10000?

But the state might be. There maybe motion or no motion during the relay stage. Maybe a continous loop testing how long the PIR sensor is LOW. If the duration of this LOW reaches 15min. it again triggers the relays(in reverse order).

I sense that you are trying to write all the code inside the if-low-to-high-transition block or the if-high-to-low transition block. That is not the way to do it.

You have a bit of code to read the current sensor state, and compare that to the previous state. If a transition has occurred, record the time as a start time or a stop time, depending on which transition occurred. That is all that that bit of code does.

Then, you have some more code to determine the current time, and to determine if now minus then (if there is a then) exceeds some threshold. If it does,do something.

This could be to determine if now minus then (when the sensor went HIGH) has exceeded 10 seconds. If so, turn on a relay.

It could be to determine if now minus then (when the sensor went LOW) has exceeded 15 minutes. If so, then off a relay.

It shouldn't take more than about 20 lines of code to do all this. 30 with comments.

For me, it shouldn't take more than about 20 hours of coding lol :smiley:

I just put together this sketch using just the PIR and LED(LED as a test for a relay activating, etc..)

//millisDuration


const int pirPin = 9;
const int ledPin = 13;


long startTime;
long unsigned int duration;


void setup(){

  pinMode(pirPin, INPUT);
  pinMode(ledPin, OUTPUT);

  Serial.begin(9600);

}


void loop(){

  if(digitalRead(pirPin) ==HIGH)
  {
    startTime = millis();
    while(digitalRead(pirPin) == HIGH)
      ;
    long duration = millis() - startTime;

    if(duration >= 10000)
    {
      digitalWrite(ledPin, HIGH);
      delay(100);
      digitalWrite(ledPin, LOW);
    }
    Serial.println(duration);

  }
}

It works and after 10 seconds the LED lights up. The problem I'm having is for the LED to light up (after 10sec.) it has to wait until the PIR sensor goes LOW (a few extra seconds). If the PIR stays high or there is continuous motion, even if it reached 10secs. the LED will remain off.

t

If it's sending 1's continuously, it's still 'new data?

No, because a transition has not occurred.

So maybe something like recording:

That's not recording. That's testing.

where I could assign duration = 10000?

A global variable.

For me, it shouldn't take more than about 20 hours of coding lol

I've spent an hour getting one line right. OK, I'll admit that yesterday, I spent more like 4 hours getting one line right. That line defined what widget was connected to what other widget in a dialog so that the whole dialog resized correctly. What a nightmare. Generally, I'm slightly faste than that. 8)