Programming Help (Class Project)

Hey guys… I’m working on a school project and I’m having an issue finding out what or where i did something wrong. So i have 3 Reed switches that control 3 LED’s, each Reed Switch controls its own LED and 1 PIR Motion Sensor that also controls an LED. It will verify and upload onto my arduino, I can get the PIR sensor to work fine (no matter the situation), but the Reed switches if i take the one side off (would normally open the switch) it stays on, the only way for them to turn off, is if the motion sensor will sense motion and turn on (after this been on for the 7seconds) it will turn off as will the rest of the lights. The same thing when i re-apply the Reed Switch (would normally close the circuit making the LED’s come on) they don’t unless i activate the motion sensor again (putting motion in front of it again). Most of my sketch is also learned and taken from online sources i left most of the //comments cause it helped me make sure i was somewhat programming right. I’m not sure if i explain it well enough but i put my sketch below all who can help please do. Its due in a couple days and i want to get a decent mark and surprise my teacher :).

//be sure the project box is insulated or false motion triggers will happen
int calibrationTime = 30;
int Time = 0;
long unsigned int lowIn;
long unsigned int pause = 5000;
boolean lockinHigh = false;
int LEDsop = 8; // pin 13 includes resistor sop- signal out pin
int outpin = 12; // for any other device that doesn't require resistor
int SensorInpin = 1; // can vary, must be digital from 1-12
boolean sensorready = false;

#define LED 13     //pin for the LED
#define SWITCH 2   //input for REED SWITCH
#define LED2 12 //pin for LED2
#define SWITCH2 3 //Pin for REED SWITCH2
#define LED3 11 //pin for LED
#define SWITCH3 4//pin for REED SWITCH3

int val = 0;       //used to store input value
int val3 = 1; //^
int val4 = 1; //^

void setup (){

  pinMode(LED, OUTPUT);   //tell arduino LED is an output
  pinMode(SWITCH, INPUT);   //SWITCH is input
  pinMode(LED2, OUTPUT);   //tell arduino LED is an output
  pinMode(SWITCH2, INPUT);   //SWITCH is input
  pinMode (LED3, OUTPUT); //TELL arduino LED is an output
  pinMode (SWITCH3, INPUT); //SWITCH3 is input
  pinMode (LEDsop, OUTPUT);  

  pinMode (LEDsop, OUTPUT);
  digitalWrite (LEDsop, LOW);
  pinMode (outpin, OUTPUT);
  digitalWrite (outpin, LOW);
  pinMode (SensorInpin, INPUT);
  pinMode (0, INPUT);
  pinMode (1, INPUT);
  pinMode (3, INPUT);
  pinMode (4, INPUT);
  pinMode (5, INPUT);
  pinMode (6, INPUT);
  pinMode (7, INPUT);
  pinMode (8, INPUT);
  pinMode (9, INPUT);
  pinMode (10, INPUT);
  pinMode (11, INPUT);
  do {
    digitalWrite (LEDsop, HIGH);
    delay (500);
    digitalWrite (LEDsop, LOW);
    delay (500);
    Time = Time + 1;
  }
  while (Time < calibrationTime);
  digitalWrite (LEDsop, LOW); //if led turns off, sensor is ready
  sensorready = true;
}
void loop (){
  if (sensorready == true){
    if (digitalRead(SensorInpin))
    {
      digitalWrite (LEDsop, HIGH);
      digitalWrite (outpin, HIGH);
      delay (7000); //modify to meet your required delay time
      digitalWrite (LEDsop, LOW);
      digitalWrite (outpin, LOW);
      delay (250);
      val=digitalRead(SWITCH);   //read input value and store it
      val3=digitalRead(SWITCH2);//^SAME
      val4=digitalRead(SWITCH3);//^SAME
      //check whether input is HIGH (switch closed)
      if (val==HIGH) {
        digitalWrite(LED, HIGH);   //turn LED on
      } 
      else{
        digitalWrite(LED, LOW);
      }
      if (val3==HIGH) {
        digitalWrite(LED2, HIGH);   //turn LED on
      } 
      else{
        digitalWrite(LED2, LOW);
      }
      if (val4==HIGH) {
        digitalWrite(LED3, HIGH);   //turn LED on
      } 
      else{
        digitalWrite(LED3, LOW);

      }
    }
  }
}

Moderator edit: [code][/code] tags added. Also code reformatted. (Nick Gammon)

Please wrap you code in [ code ] tags (the "#" button in the editor). You might do yourself a favor and in the Arduino IDE, run Tools -> Auto Format.

they don't unless i activate the motion sensor again

There's a really good reason for that...

   if (digitalRead(SensorInpin))

You only update the LEDs if SensorInpin is HIGH. This becomes very obvious when your code is properly indented.

 val=digitalRead(SWITCH);
 if (val==HIGH) {
    digitalWrite(LED, HIGH);   //turn LED on
  } else{
    digitalWrite(LED, LOW);
  }

Using generic variables like val, val2, val3, etc don't really give a description of what you are trying to do. It is better to name it something useful like "reed1State".

Regardless you don't actually need to do your if-statements here. You could simplfy these blocks of code like this:

digitalWrite(LED, digitalRead(SWITCH));
boolean sensorready = false;

...

  sensorready = true;

...

  if (sensorready == true){

When does sensorready ever become false? If never, why use it?


  while (Time < calibrationTime);

When does it ever exit that loop?

Ooops - spotted it:

 do {
    digitalWrite (LEDsop, HIGH);
    delay (500);
    digitalWrite (LEDsop, LOW);
    delay (500);
    Time = Time + 1;
  }  
  while (Time < calibrationTime);

A for loop would be more readable:

for (Time = 0; Time < calibrationTime; Time++)
   {
    digitalWrite (LEDsop, HIGH);
    delay (500);
    digitalWrite (LEDsop, LOW);
    delay (500);
   }

      val=digitalRead(SWITCH);   //read input value and store it
...
      //check whether input is HIGH (switch closed)
      if (val==HIGH) {
        digitalWrite(LED, HIGH);   //turn LED on
      } 
      else{
        digitalWrite(LED, LOW);
      }

This may as well be:

        digitalWrite(LED, digitalRead(SWITCH));   //turn LED on or off depending on SWITCH

(edit) Ooops … James C4S said that too.


#define SWITCH2 3 //Pin for REED SWITCH2
...
#define SWITCH3 4//pin for REED SWITCH3

...

  pinMode(SWITCH2, INPUT);   //SWITCH is input
...
  pinMode (SWITCH3, INPUT); //SWITCH3 is input

...

  pinMode (3, INPUT);
  pinMode (4, INPUT);

Some duplication there.

Thanks for the feedback it was my first time really coding after the button + the blinking diode I tried to use Auto format on the Arduino but it didn't do anything, and I'm also not sure what you mean by proper indenting? Anyways back to the project so should i just change the

  if (digitalRead(SensorInpin))
    {
      digitalWrite (LEDsop, HIGH);
      digitalWrite (outpin, HIGH);
      delay (7000); //modify to meet your required delay time
      digitalWrite (LEDsop, LOW);
      digitalWrite (outpin, LOW);
      delay (250);
    }

??
and if what do i change it to? Like i said before first time ever really programming sorry if i sound redundant.

I've simplified your loop to what it really does:

void loop ()
  {
  if (digitalRead(SensorInpin))
    {
    digitalWrite (LEDsop, HIGH);
    digitalWrite (outpin, HIGH);
    delay (7000); //modify to meet your required delay time
    digitalWrite (LEDsop, LOW);
    digitalWrite (outpin, LOW);
    delay (250);
    digitalWrite(LED, digitalRead(SWITCH));
    digitalWrite(LED2, digitalRead(SWITCH2));
    digitalWrite(LED3, digitalRead(SWITCH3));
    }  // end if pin high
}  // end of loop

Now as I read it, if the sensorpin is HIGH you turn on two LEDs, wait 7 seconds, and then them off again. Then you set 3 other LEDs to reflect the settings of three switches.

Now what do you want it to do exactly?