LDR not Working in Automatic Lighting System with PIR (Arduino Uno)

Hello, everyone! I'm new to the forum and Arduino itself, and currently I'm having difficulty realizing the system that I want.

I've made this system without the LDR before. The codes and circuit worked both in Proteus simulation and the real system. The lamp always turns on every time either or both PIRs detect movement, and only turns off when there is no movement. But when I add the LDR and edit the codes for LDR to fit, the codes don't work. The lamp doesn't turn on, the relay also doesn't get active. I want to use LDR for more efficient lighting.

The circuit sketch on Proteus is attached.

Here's the code for the system without LDR:

int PIR1 = 2; //PIR sensor 1
int PIR2 = 3; //PIR sensor 2
int Relay = 8; //Relay
//int data1 = 0;
//int data2 = 1;
//int statusPIR1 = 0;
//int statusPIR2 = 1;
unsigned long t = 0;

void setup() {
  // put your setup code here, to run once:
//Set pin sensor PIR dan pin relay sebagai input
pinMode(PIR1, INPUT);
pinMode(PIR2, INPUT);
Serial.begin(9600);
pinMode(Relay, OUTPUT);
digitalWrite(Relay, LOW);
}

void loop() {
  // put your main code here, to run repeatedly:
digitalWrite(Relay, LOW);

if(digitalRead(PIR1) == HIGH || digitalRead(PIR2) == HIGH)
{
  t = millis();
  Serial.println("Gerakan terdeteksi! Lampu menyala!");
 
  while(millis() < (t + 5000))
  {
    digitalWrite(Relay, HIGH);
    if((millis() > (t + 2300)) && (digitalRead(PIR1) == HIGH) || (digitalRead(PIR2) == HIGH))
    {
      t = millis();

     /* if(Relay == HIGH);
      {
        Serial.println("Gerakan tidak terdeteksi. Lampu mati.");
      }
      */
    }
  }
}
}

And here's the one with LDR, I only changed it a little bit but it doesn't seem to be working, even the lamp doesn't turn on at all. I set the lamp to turn on when luminous intensity is less than 300.

int PIR1 = 2; //PIR sensor 1
int PIR2 = 3; //PIR sensor 2
int Relay = 8; //Relay
int LDR = A4; //LDR
//int data1 = 0;
//int data2 = 1;
//int statusPIR1 = 0;
//int statusPIR2 = 1;
unsigned long t = 0;

void setup() {
  // put your setup code here, to run once:
//Set pin sensor PIR dan pin relay sebagai input
pinMode(PIR1, INPUT);
pinMode(PIR2, INPUT);
pinMode(LDR, INPUT);
Serial.begin(9600);
Serial.println(analogRead(A4));
pinMode(Relay, OUTPUT);
digitalWrite(Relay, LOW);
}

void loop() {
  // put your main code here, to run repeatedly:
digitalWrite(Relay, LOW);

if((digitalRead(PIR1) == HIGH || digitalRead(PIR2) == HIGH) && analogRead(A4) < 300)
{
  t = millis();
  Serial.println("Gerakan terdeteksi! Lampu menyala!");
 
  while(millis() < (t + 5000))
  {
    digitalWrite(Relay, HIGH);
    if((millis() > (t + 2300)) && ((digitalRead(PIR1) == HIGH || digitalRead(PIR2) == HIGH) && analogRead(A4) < 300))
    {
      t = millis();

     /* if(Relay == HIGH);
      {
        Serial.println("Gerakan tidak terdeteksi. Lampu mati.");
      }
      */
    }
  }
}
}

Any help will be appreciated. Sorry for my bad English. Thanks!

int PIR1 = 2; //PIR sensor 1
int PIR2 = 3; //PIR sensor 2
int Relay = 8; //Relay
int LDR = A4; //LDR

Are these variables supposed to hold pin numbers? Why doesn't the name make that clear?

pinMode(LDR, INPUT);

This is diddling with the digital nature of the pin you are using as an analog pin. Do NOT do that.

if((digitalRead(PIR1) == HIGH || digitalRead(PIR2) == HIGH) && analogRead(A4) < 300)

Why do you use 2 names and a "number" in the calls? You should be using 3 names.

Nested ifs are far easier to debug than compound ifs.

    if((millis() > (t + 2300)) && ((digitalRead(PIR1) == HIGH || digitalRead(PIR2) == HIGH) && analogRead(A4) < 300))

Adding time values is not a good idea. Subtraction is guaranteed to work. Addition is NOT.

     /* if(Relay == HIGH);

Properly written if statements rarely end with a semicolon.

but it doesn't seem to be working

I can guarantee that the code IS working. That it doesn't do what you expect simply means that your expectations are wrong.

Dump all that code. Start over. Write a sketch that does nothing more than read the LDR pin and print the value. Perhaps you have something wired wrong, or the value that you are reading is not what you think it is.

Only when you KNOW that you are getting good data from the LDR should you use the data in the current sketch.

A schematic would be very helpful, too.

Thank you for the reply.

Are these variables supposed to hold pin numbers? Why doesn't the name make that clear?

Ah, yes, the variables are supposed to do that, sorry for the unclear names.

This is diddling with the digital nature of the pin you are using as an analog pin. Do NOT do that.

I don't understand why? I use the analog pin A4?

Why do you use 2 names and a "number" in the calls? You should be using 3 names.

Nested ifs are far easier to debug than compound ifs.

I just realized that I wrote that part with two names and a "number". My bad. :confused:

I'm not really familiar with the term nested ifs, is it when you type in

if ...
then if...
then...
else...?

Adding time values is not a good idea. Subtraction is guaranteed to work. Addition is NOT.

I see, I will fix that part.

A schematic would be very helpful, too.

I attached the schematic on the attachment, the ground is cut though. :confused:

Though I'll do your advice, I'll make a sketch only for detecting the luminous intensity and displaying the value on the monitor. (I need to buy more breadboards first, though. :frowning: )

I see two unhelpful things in this code

if((digitalRead(PIR1) == HIGH || digitalRead(PIR2) == HIGH) && analogRead(A4) < 300)

First, because you have not saved the values from the digitalRead() and the analogRead() you have no means to print them out to see if they are what you expect.

Second, by having 3 tests in a single statements you have no means to know which tests fail.

I suggest you do it like this so you can insert print statements for debugging

PIR1val = digitalRead(PIR1pin);
PIR2val = digiitalRead(PIR2pin);
LDRval = analogRead(LDRpin);

if ( PIR1val == HIGH) {
   if (PIR2val == HIGH) {
      if (LDRval < 300) {

...R

I don't understand why?

Is that a statement? Or a question? If it is a statement, the punctuation is wrong. If it is a question, the arrangement of the words before the question mark do not constitute a question.

You can use a pin as a digital pin, in which case you should call pinMode. Or, you can use the pin as an analog pin, in which case you should not call pinMode(), because pinMode() ONLY applies to the digital nature of the pin.

I need to buy more breadboards first, though.

No, you don't. You don't need to change ANY wiring. Just dump a lot of code that is not related to reading and printing the value of the pin that the LDR is attached to.

First, because you have not saved the values from the digitalRead() and the analogRead() you have no means to print them out to see if they are what you expect.

Second, by having 3 tests in a single statements you have no means to know which tests fail.

Hi, Robin. You're right, I didn't think of that before. Anyway, I've followed your advice, but wasn't sure whether I did it true. Should I replace the entire

if((digitalRead(PIR1) == HIGH || digitalRead(PIR2) == HIGH) && analogRead(A4) < 300)

with the one you've showed? Since the code above is typed at least two times in my code. Here's what I wrote. I haven't tested it in real life, though.

void loop() {
  // put your main code here, to run repeatedly:
digitalWrite(Relaypin, LOW);

int PIR1val = digitalRead(PIR1pin);
int PIR2val = digitalRead(PIR2pin);
int LDRval = analogRead(LDRpin);
if (PIR1val == HIGH) {
  Serial.println("PIR 1: ");
  Serial.println(PIR1val);
  if (PIR2val == HIGH) {
    Serial.println("PIR 2: ");
    Serial.println(PIR2val);
    if (LDRval < 300) {
     
      t = millis();
       Serial.println(LDRval);
 
       while(millis() < (t + 5000))
       
         digitalWrite(Relaypin, HIGH);
         if(millis() > (t - 2300)) {
          if (PIR1val == HIGH) {
            if (PIR2val == HIGH) {
              if (LDRval < 300) {
                
                t = millis();
                                }
                                 }

Just dump a lot of code that is not related to reading and printing the value of the pin that the LDR is attached to.

Paul, I've followed what you said and tested the LDR and it works properly, the monitor is showing the luminous intensity value.

vyannanda:
I haven't tested it in real life, though.

Testing is an essential part of programming, especially when interfacing with hardware. Test early and often.

If you find the need for the same lines of code in two places you should put the code into a function and call the function. Have a look at how the code is organised in Planning and Implementing a Program

Having duplicate code in a program creates scope for typos and for errors when one version is updated but the changes to other one are overlooked.

...R