Help with code, please..

Hi all

I would like to ask for assistance with this bit of code. It is meant to read an LDR and if light turn relay on, if dark turn other relay on.
The code tests and works fine but it will not switch off the relay after the delay(4000);.

#define RELAY1  2   

                     
#define RELAY2  3                        

 int LDR = 1;
 int LDRValue = 0;
 int Light_sensitivity = 1000;

void setup()
{    
  pinMode(RELAY1, OUTPUT);   
  pinMode(RELAY2, OUTPUT);
}
 void loop()
{
  LDRValue = analogRead(LDR);
 
  if (LDRValue < Light_sensitivity)
  {
   digitalWrite(RELAY1,HIGH);
   delay(4000);
   digitalWrite(RELAY1,LOW);
  
  }
  else
  { 
  digitalWrite(RELAY2,HIGH);  
  delay(4000);
  digitalWrite(RELAY2,LOW);

  } }

What I would like to achieve is: when light is detected a relay turns on for 4 secs then turns off and the same when dark is detected.
Is it because it is either light or dark and just stays in that state??

Any help, ideas would be appreciated.

Thanks

Don

The code tests and works fine but it will not switch off the relay after the delay(4000);.

How do you know? Perhaps what is happening is that it is immediately being turned back on.

Add a delay(1000) after turning it off.

Then, read, understand, and embrace the philosophy of the blink without delay example, and get rid of the delay()s altogether.

Donmerrick:
What I would like to achieve is: when light is detected a relay turns on for 4 secs then turns off and the same when dark is detected.

Do you mean when it changes from light to dark it operates one output for four seconds, and when it changes from dark to light it operates another relay for four seconds?

If so, you need to remember what the previous state is so you can tell when it has changed. The StateChangeDetection example sketch shows you how this can be achieved.

Yes PeterH thats what i am trying to achieve with this

Not clear what you are exactly trying to achieve.
You are doing exactly the same thing whether LDR value is greater or less than your sensitivity.
ie. Your turn the relay on for 4 seconds then shut it off. But loop(), as the name suggests is constantly looping. So in the next cycle you are again turning it on for 4 seconds and shutting off. Between these cycles there is no delay. So the relay is shut off for the briefest time that it takes to execute 2 lines of code. Not visible to naked eye as the previous poster says. So if you add another delay of say 1 second then you will see relay flickering as On for 4 sec, Off for 1 sec ...... forever.

Maybe you are trying to achieve "state change" of light? ie. for example, if you detect that now it is sunrise, then turn it on for 4 seconds and turn it off. Now next time you want to do it is only when the light intensity has changed its state. ie. sunset. So all throughout the daylight hours you don't want to do anything. At sunset again you want to flip the relay for 4 seconds. and so on.
If that is your purpose, then you need to declare another variable called say "boolean Is_It_Bright". You light reading constantly checks and sees if you need to flip the value of this variable. Blink the relay only when you actually flip this variable.

Thanks to all for your help, but, now I am really confused...

Does the light actually achieve 1000?

I have edited your code to add Serial.print() statements to make it easier to debug what the code is doing. As others have pointed out, it may not always be possible to see what the code is doing by watching an LED, because the LED may flash on and off faster than you can see. The example code below is a little extreme--I have put in a debug line for nearly every line of code, even some that should just be assumed to happen--but it's just intended as an example.

#define DEBUG

#define RELAY1  2                      
#define RELAY2  3                        

int LDR = 1;
int LDRValue = 0;
int Light_sensitivity = 1000;

void setup()
{    
  pinMode(RELAY1, OUTPUT);   
  pinMode(RELAY2, OUTPUT);

#ifdef DEBUG
  Serial.begin(9600);
#endif
}

void loop()
{
  LDRValue = analogRead(LDR);

#ifdef DEBUG
  Serial.print("loop(): LDRValue = ");
  Serial.println(LDRValue);
#endif

  if (LDRValue < Light_sensitivity)
  {
#ifdef DEBUG
    Serial.println("loop(): LDRValue < Light_sensitivity");
#endif

    digitalWrite(RELAY1,HIGH);

#ifdef DEBUG
    Serial.println("loop(): Set relay 1 high");
#endif

    delay(4000);  
    digitalWrite(RELAY1,LOW);

#ifdef DEBUG
    Serial.println("loop(): Set relay 1 low");
#endif
  }
  else
  { 
#ifdef DEBUG
    Serial.println("loop(): LDRValue not < Light_sensitivity");
#endif

    digitalWrite(RELAY2,HIGH);  

#ifdef DEBUG
    Serial.println("loop(): Set relay 2 high");
#endif

    delay(4000);
    digitalWrite(RELAY2,LOW);

#ifdef DEBUG
    Serial.println("loop(): Set relay 2 low");
#endif    
  } 
}

I always like to put my debug-related Serial.print() statements into a #ifdef so that I can drop them out of the final code. The strings take up a lot of memory that's not needed in the final code.

Donmerrick:
Thanks to all for your help, but, now I am really confused...

Look at the State Change Example example sketch. It demonstrates how to do what you're trying to do.

Donmerrick:
The code tests and works fine but it will not switch off the relay after the delay(4000);.

Which relay?

Actually it is both relays, in either dark or light state the relays activate but do not de-energise.

My bad!!!
In a haste, I didn't read your code properly. You have TWO different relays.

However, the issue is still nearly the same.
After you turn a given relay OFF, very quickly (in order of milliseconds) you read the light value again. If the light value hasn't changed, you are going to turn it ON again. There is not much chance for that relay to be in OFF state. Although, Nick's question about which relay, is valid. You should only see on of the relays staying ON not both together.

Just for debugging tryout, why don't you add a "delay(6000);" after each of the lines digitalWrite(RELAY1, LOW); & digitalWrite(RELAY2, LOW);
This way you will know if the relays are switching off at all. Then we can go from there to analyze the issue further.

Just for debugging tryout, why don't you add a "delay(6000);" after each of the lines digitalWrite(RELAY1, LOW); & digitalWrite(RELAY2, LOW);
This way you will know if the relays are switching off at all. Then we can go from there to analyze the issue further.

I doubt that OP will do this. It was suggested in reply #1, and ignored there, too.

Hi guys

Sorry, I am back again now.

Paul S, I did not ignore any of the suggestions, I work long hours in care and am sometimes too tired to rig everything back together in such a state that I chance blowing the duino up. I am new to code full stop, I have been trying to learn this prog language now for about 11 hours total. I do find some things very confusing but I am serious about learning this.

I have just finished for the day, so seeing as I have about 7 hours before my next shift, I will try and test some of the ideas put forward by you great folks.

I am sorry if this seems like an inconvenience to some but I do appreciate your replies and your efforts to help a newbee.

Don

Right then....

Paul S, I added delay(1000); after both (low) and one relay stays on and the other comes on for a sec and off for 4 secs.

Joshuabardwell, this is what I got, seemed like relay 2 did nothing!!!

Loop(): LDRValue  = 890
Loop(): LDRValue < Light_sensitivity
Loop(): Set relay 1 high
Loop(): Set relay 1 low
Loop(): LDRValue  = 790
Loop(): LDRValue < Light_sensitivity
Loop(): Set relay 1 high
Loop(): Set relay 1 low

Also one thing I did notice with the std code is relay 1 stays on but during the delay span the relay 2 led flickers.

Don

Donmerrick:
Joshuabardwell, this is what I got, seemed like relay 2 did nothing!!!

Loop(): LDRValue  = 890

Loop(): LDRValue < Light_sensitivity
Loop(): Set relay 1 high
Loop(): Set relay 1 low
Loop(): LDRValue  = 790
Loop(): LDRValue < Light_sensitivity
Loop(): Set relay 1 high
Loop(): Set relay 1 low

Sure! Because Light_sensitivity = 1000 and your LDRValue is always (so far) less than 1000, the "else" clause never gets invoked and relay 2 never gets activated. Is this not the behavior you wanted?

Sorry Joshuabardwell.

In the state I posted, relay1 stays on but relay2 flicks on quickly then goes off for about 4 secs, if I cover the ldr, relay2 stays on but relay1 flicks on and off.

When you say the relay flicks on and off is it an indicator led flickering? Try loading the relay output down with a suitable resistor like a light bulb.

Donmerrick:
In the state I posted, relay1 stays on but relay2 flicks on quickly then goes off for about 4 secs, if I cover the ldr, relay2 stays on but relay1 flicks on and off.

Can you post the DEBUG output of that occurrence? The exact flow of the program should be made clear from that. The debug output you posted previously never shows the program activating relay 2, so if relay 2 is flickering, it is due to something else--such as hooking the relay's control wire up to a floating pin, for example.

Hi Justinbardwell

Loop(): LDRValue  = 1023
Loop(): LDRValue not < Light_sensitivity
Loop(): Set relay 2 high
Loop(): Set relay 2 low
Loop(): LDRValue  = 941
Loop(): LDRValue < Light_sensitivity
Loop(): Set relay 1 high
Loop(): Set relay 1 low
Loop(): LDRValue  = 949
Loop(): LDRValue < Light_sensitivity
Loop(): Set relay 1 high
Loop(): Set relay 1 low

And it continues with no relay2 popping up again

while running even though the debug does not mention relay2 again, it still flickers as before.