1
\$\begingroup\$

I'm setting up a PIR activated relay (connected to some lights) using an Arduino Nano.

I have the PIR code working (obtained from a 3rd party site) but I'd like the option to override the PIR input based on an input from a push button switch. To add even more complexity, I'd like to have an LED lit up to let me know when the PIR is being manually overridden (relay always on).

The button is a simple push button.

By default, I want the unit to be in PIR mode. The relay is activated if motion is detected, and then after X seconds it switches off. If I press the button once, it's manually overridden and the relay is switched on (constantly) and the blue LED is activated to inform me that the unit is on constantly. If the button is pressed again, the unit switches back to PIR mode.

Here's the code I've got so far - but the LED and relay seem to be constantly active. The PIR is still reading (according to the serial monitor).

/*
 * PIR sensor tester
 */ 


int PIR_override = 3;         // the number of the switch pin
int overridestatus = 12;       // the number of the led pin

int relaypin = 13;                // choose the pin for the LED
int inputPin = 2;               // choose the input pin (for PIR sensor)
int pirState = LOW;             // we start, assuming no motion detected
int val = 0;                    // variable for reading the pin status
int DELVAR = 3000;             // Delay in milliseconds
int override_state = LOW;      // the current state of the output pin

// set states for PIR override switch
int state = LOW;      // the current state of the output pin
int reading;           // the current reading from the input pin
int previous = HIGH;    // the previous reading from the input pin

///////////////////////////////////////////////////////
void setup() {
  pinMode(relaypin, OUTPUT);      // declare LED as output
  pinMode(inputPin, INPUT);     // declare sensor as input

  Serial.begin(9600);
}

void loop()
{
    digitalRead(PIR_override);  // read input value of PIR override switch
    if (val == LOW)
    {            // check if the input is LOW
        val = digitalRead(inputPin);  // read input value
        if (val == HIGH)
        {            // check if the input is HIGH
            digitalWrite(relaypin, HIGH);  // turn LED ON
            if (pirState == LOW)
            {
                // we have just turned on
                Serial.println("Motion detected!");
                // We only want to print on the output change, not state
                pirState = HIGH;
                delay(DELVAR);    // maximum delay is 32776 millisecons,
                //  delay(DELVAR);    // so add multiple delays together to get
                // delay(DELVAR);    // so add multiple delays together to get
            }
        }
        else
        {
            digitalWrite(relaypin, LOW); // turn LED OFF
            if (pirState == HIGH)
            {
                // we have just turned of
                Serial.println("Motion ended!");
                // We only want to print on the output change, not state
                pirState = LOW;
            }
            else
            {
                digitalWrite(overridestatus, HIGH);  // turn override status LED ON
                digitalWrite(relaypin, HIGH);  // turn relay ON
            }
        }
    }
}

Here's the code I've got so far - but the LED and relay seem to be constantly active. The PIR is still reading (according to the serial monitor).

There's clearly something wrong with my if-else loop but I'm not entirely sure where I need to start looking.

I'm also including a basic breadboard image of the circuit (it's my first Fritz so be gentle!) Also, please ignore the pin in on the relay board. I am using an SRD-05VDC-SL-C but it's on a slightly different board with only 3 pins (IN, VCC, GND).

enter image description here

\$\endgroup\$
1
\$\begingroup\$

I think your main problem is that you aren't assigning a variable or using an if statement with your digitalRead(PIR_override);, but even with that you may miss the button press in the loop. If it's a momentary switch then it goes high/low for only the time that it's pressed, so the processor would have to see this at the right time in the while loop. You could just hold the button down for about a second or you could connect the button to pin 2 or 3 and use an interrupt, which it looks like your button is connected to pin 3 according to your code. I think this is a better solution. In the embedded world interrupts are your friend.

So, according to the documentation (https://www.arduino.cc/en/Reference/AttachInterrupt), you could do something like this:

const byte PIR_override = 3;        // the number of the switch pin
volatile boolean override = false;          // Use this to know if your currently  in override mode or not when looking at the button press

Inside Setup() add:

pinMode(PIR_override, INPUT);
attachInterrupt(digitalPinToInterrupt(PIR_override), motionOverride, LOW); // the sense may need to be changed - from your image it looks like the button goes low when pressed

So now motionOverride will be called when the button is pressed. Just put what you want to happen inside this function. Something like this:

void motionOverride()
{
    if(override) // overriding motion and the button has been pressed - got to PIR mode
    {
        digitalWrite(overridestatus, LOW);  // turn override status LED OFF
        digitalWrite(relaypin, LOW);  // turn relay OFF - you may not want to do this... depends on what you want
        override = false;               // set the override flag
    }
    else // not overriding motion and the button has been pressed - go to override mode
    {
        digitalWrite(overridestatus, HIGH);  // turn override status LED ON
        digitalWrite(relaypin, HIGH);  // turn relay ON
        override = true;                // set the override flag
    }

   // I guess Arduino takes care of clearing the interrupt for you
}

Then inside your main loop() just look at the override flag (Note: I did not check any of your PIR motion detection logic, and hopefully I didn't mess up your brackets there):

void loop()
{
     if(!override)
     {
         val = digitalRead(inputPin);  // read PIR input value
         if (val == HIGH)  // check if the input is HIGH
         {           
              digitalWrite(relaypin, HIGH);  // turn LED ON
              if (pirState == LOW) 
              {
                   // we have just turned on
                   Serial.println("Motion detected!");
                   // We only want to print on the output change, not state
                   pirState = HIGH;
                   delay(DELVAR);    // maximum delay is 32776 millisecons,
                   //  delay(DELVAR);    // so add multiple delays together to get
                   // delay(DELVAR);    // so add multiple delays together to get
             }
        } 
        else
        {
            digitalWrite(relaypin, LOW); // turn LED OFF
            if (pirState == HIGH)
            {
                 // we have just turned of
                 Serial.println("Motion ended!");
                 // We only want to print on the output change, not state
                 pirState = LOW;
            } 
        }
     }
}

Try that out if you want. If there's a mistake I'll try and help with it.

\$\endgroup\$
  • \$\begingroup\$ Thanks DigitalNinja. The code you've suggested seems to "almost" do what I'm after. However, the state of the switch seems to be a little flakey. I've read into this a bit and it looks like I might need to use some "debouncing". Where would you suggest I put the debounce code - in the function itself or in the main loop? \$\endgroup\$ – Huskie69 Sep 8 '16 at 19:31
  • \$\begingroup\$ Probably definitely debouncing. A hardware solution would be better, for that you'd need to change the default state of the switch so that it is normally pulled low and then when pressed goes high. If you did that all you would need is a small capacitor on the input to the processor and to ground. For software, for what you're doing I'd just put a small delay at the end of the ISR. This will prevent the interrupt from being triggered again immediately after the first time. \$\endgroup\$ – DigitalNinja Sep 8 '16 at 19:46
  • \$\begingroup\$ Depending on how Arduino is clearing the interrupt, a delay might not work because if it enters the ISR and clears the flag after the delay it may just re-enter the ISR on the pending interrupt. Also, you have to use delayMicroseconds() because it doesn't use interrupts itself. If you can't do the hardware solution and the delay at the end of the ISR doesn't work I think I have a work around that will work. Let me know how it goes. \$\endgroup\$ – DigitalNinja Sep 8 '16 at 20:03
  • \$\begingroup\$ Probably a stupid question but when you say add a capacitor on the input processor, do you mean connecting it to the switch pin (PIR_override = 3)? \$\endgroup\$ – Huskie69 Sep 8 '16 at 21:09
  • \$\begingroup\$ Yes, put the capacitor in between the switch output and the processor input. But remember you must change the default state of the switch to be low (pulled to ground via your resistor) and when pressed goes high (looks like 5VDC in your image). When it goes high the capacitor charges up and filters out the bouncing. Also, if you do this remember to change the interrupt sense to be HIGH instead of LOW in your code. \$\endgroup\$ – DigitalNinja Sep 8 '16 at 21:16
1
\$\begingroup\$

I've suggested an edit to your code; indents were not present, so I added them to make nested if-else statements more clear.

I'd like to address some issues I have; Please change your pin definitions:

int PIR_override = 3;         // the number of the switch pin
int overridestatus = 12;       // the number of the led pin
int relaypin = 13;                // choose the pin for the LED
int inputPin = 2;               // choose the input pin (for PIR sensor)

to something like:

#define PIR_override 3         // the number of the switch pin
#define overridestatus 12       // the number of the led pin

#define relaypin 13                // choose the pin for the LED
#define inputPin 2               // choose the input pin (for PIR sensor)

With your current use of integer variables, it is possible for their value to change, which you most certainly do not want to do. Using integers for pin definitions also consumes room on your stack. This may not ever be a real problem, but it is something to consider.

More in-depth information about pre-processor directives can be found elsewhere.

When you read your PIR_override pin:

digitalRead(PIR_override);  // read input value of PIR override switch

The value is not being used. You must assign this to a variable.

I also see a few problems with your if-else logic. Take this code for example:

if (val == LOW)
{            // check if the input is LOW
    val = digitalRead(inputPin);  // read input value

Initially, val is set to be low. That's fine. The if statement will be true, and val will be reassigned. But what if val is reassigned to be HIGH? Once the end of the loop is reached and re-started, there is no else statement to handle val == HIGH!

It appears to me that what you are trying to do with your nested if-else logic is close to a state machine. If you haven't looked in to it already, I would suggest looking up 'if-else state machines' or 'switch-case state machines'.

Otherwise, if you would like to continue with the code that you have now, I would consider writing out the starting variable values and manually 'stepping through' your code.

\$\endgroup\$

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.