using PIR Sensor to fading LED

OK I am wanting to turn on some LED's with some PIR sensors and I've been successful at that. I even have them turn on and fade off but I'm wanting to keep the LED's solidly lit until the PIR stops detecting motion and then fade off. I've tried a few different IF statements but no success. Below is my sketch for the basic turn on, pause, and fade off. It works well except that if the PIR continues to detect motion it just cycles through.

Any Ideas?

const int firstLED = 3; // pin for the LED
const int inputPin = 2; // input pin for the PIR sensor

void setup() {

pinMode(inputPin, INPUT); //declare PIR as input

}

void loop() {

int val = digitalRead(inputPin); // read input value
if (val == HIGH) // check if the input is HIGH
{
analogWrite(firstLED, 255); //turn LED on if motion dected
delay(5000);
for(int fadeValue = 255 ; fadeValue >= 0; fadeValue -=5) {
// sets the value (range from 0 to 255):
analogWrite(firstLED, fadeValue);
// wait for 30 milliseconds to see the dimming effect
delay(30);
}
analogWrite(firstLED, 0);
}

}

I even have them turn on and fade off but I'm wanting to keep the LED's solidly lit until the PIR stops detecting motion and then fade off.

So, why do you start the fading 5 seconds after the PIR goes HIGH. The fading should start when the PIR goes LOW.

In order to do what you want, you need to detect the transition from LOW to HIGH and from HIGH to LOW, not just whether the PIR is HIGH or LOW. To do this, you need to save the state at the end of loop(). At the beginning of loop, read the state, and compare it to the previous state. If they are the same, no transition has occurred. If they are different, a transition has occurred, and you can then determine whether it was HIGH to LOW (now LOW) or LOW to HIGH (now HIGH).

If a transition occurred, and it was HIGH to LOW, start the fading.

PS: The Tools + Autoformat function will make your code more readable.

Ok the saving makes sense but I don't know what code to use for that though

What I'm trying to do is have an automatic light in the kitchen so that's why I put in the 5 second delay so the LED's don't blink repeatedly and I have a seizure lol.
Here's the code after the auto format. yeah it does a really nice job of cleaning it up.

I'm also unclear where I would have to save it after the 5s delay?

const int firstLED = 3; // pin for the LED
const int inputPin = 2; // input pin for the PIR sensor

void setup() {

pinMode(inputPin, INPUT); //declare PIR as input

}

void loop() {

int val = digitalRead(inputPin); // read input value
if (val == HIGH) // check if the input is HIGH
{
analogWrite(firstLED, 255); //turn LED on if motion dected
delay(5000);
for(int fadeValue = 255 ; fadeValue >= 0; fadeValue -=5) {
// sets the value (range from 0 to 255):
analogWrite(firstLED, fadeValue);
// wait for 30 milliseconds to see the dimming effect
delay(30);
}
analogWrite(firstLED, 0);
}

}

(Newbie hint: Use the "#"-button when entering code, it makes it appear inside a nice white scrollable window)

void loop() {  
  static int lastval = HIGH ; // the HIGH is only used on the first pass through the loop
  int val = digitalRead(inputPin);      // read input value
  if (val != lastval)                      // check if the input has changed
  { // fadecode 
   lastval = val ;
  } 
}

This will trigger on any change. You can insert extra tests if val is HIGH or LOW, which distiguishes the two differenet changes.

Note: Your use of the delay() means you are ignoring any changes there may be on the pin while it fades. That may, or may not be our intention. See Arduino Playground - AvoidDelay

You can work on re-formulating the logic of what you really want.
The output, as a function of time since last detection, is:

if (T < 5 seconds) O = on else O = lerp(on, off, (T-5)/fadetime)

An important note is that "time of last detection" keeps updating while the detector is high.

Thus, the code might look something like:

void loop() {
    static unsigned long lastTime;
    unsigned long now = millis();
    if (inputIsHigh()) {
        lastTime = now;
    }
    int wantedState = 0;
    if (now - lastTime < 5000) {
        wantedState = 255; // assuming 255 is "fully on"
    }
    else if (now - lastTime - 5000 < FADE_TIME) {
        wantedState = (int)(255 * (float)(FADE_TIME - now - lastTime - 5000) / (float)FADE_TIME);
    }
    outputState(wantedState);
}

You'll have to massage this to fit your particular parameters, but it ought to serve as an illustration of how it can be done in a robust way that keeps minimal state (only one variable) between iterations.

ok the first bit of code by msquare works but just cycles the dim forever.

and the robust code from jwatte didn't work at all.

My origional I did that didn't work was a nested if such as:

if(val == LOW){
if(firstLED, 255){
    for(int fadeValue = 255 ; fadeValue >= 0; fadeValue -=5) { 
      // sets the value (range from 0 to 255):
      analogWrite(firstLED, fadeValue);         
      // wait for 30 milliseconds to see the dimming effect    
      delay(30);                            
    } 
  }
}

but I got the same result as msquare's code just continual loop of the fade

Is my fade code the problem?

My origional I did that didn't work was a nested if such as:

Quit with the "such as" crap. Post ALL of your code. That last stuff you posted doesn't even make sense.

if(firstLED, 255){

Computers and compilers are stump stupid. They will do exactly what you tell them to do, even when you can't tell them to do what you want, or don't understand the language well enough to tell them something that makes sense.

C has a comma operator, so, syntactically, that statement is valid, but it does not do what you might think it does. If you explain what you think it is supposed to do, we can explain why that is not the case.

sorry about that. I'm trying to become more affluent. ok here's my entire sketch

const int firstLED = 3;  // pin for the LED
const int inputPin = 2; // input pin for the PIR sensor


void setup() {

  pinMode(inputPin, INPUT);  //declare PIR as input

}

void loop() {  
  int val = digitalRead(inputPin);      // read input value
  if (val == HIGH)                      // check if the input is HIGH
  {
    analogWrite(firstLED, 255);      //turn LED on if motion dected
  }
  analogWrite(firstLED, 0);

  if(val == LOW){
    if(firstLED, 255){
      for(int fadeValue = 255 ; fadeValue >= 0; fadeValue -=5) { 
        // sets the value (range from 0 to 255):
        analogWrite(firstLED, fadeValue);         
        // wait for 30 milliseconds to see the dimming effect    
        delay(30);                            
      } 
    }
  }
}

My idea with the second if statement is to simply detect when the sensor throws a LOW and when the LED is on and then start the fade sequence. So that it doesn't just loop the fade over and over.
does this make more sense?
again sorry for being a newb

  int val = digitalRead(inputPin);      // read input value
  if (val == HIGH)                      // check if the input is HIGH
  {
    analogWrite(firstLED, 255);      //turn LED on if motion dected
  }
  analogWrite(firstLED, 0);

If the sensor is HIGH, turn the LED on. Then, regardless of whether the pin was HIGH or LOW, turn the LED off. Not exactly what I would be doing, but, OK.

  if(val == LOW){

A comment that describes whether you think this means motion or no motion would be useful. Presuming that it means that there is no motion, you do some stuff.

    if(firstLED, 255){

My idea with the second if statement is to simply detect when the sensor throws a LOW and when the LED is on and then start the fade sequence.

The idea with the second if statement may be to detect that firstLED contains 255, but that is NOT the purpose of the comma operator. That is the purpose of the == operator.

Just because the compiler didn't complain about the code does not mean that it is going to do what you think it will. It KNOWS what the comma operator does. Apparently you don't. I couldn't explain what it does, either, so don't feel too bad. I do know that it is not the correct operator to use here.

Ok here's what I have now. Now the LED turns on and off but it doesn't go into the fade

const int firstLED = 3;  // pin for the LED
const int inputPin = 2; // input pin for the PIR sensor


void setup() {

  pinMode(inputPin, INPUT);  //declare PIR as input

}

void loop() {  
  int val = digitalRead(inputPin);      // read input value
  if (val == HIGH)                      // check if the input is HIGH
  {
    analogWrite(firstLED, 255);      //turn LED on if motion dected
  }
  analogWrite(firstLED, 0);


  if(val != HIGH){                // check if the input isn't HIGH 
    analogRead(firstLED);          // check if LED is on
    if(firstLED == 255){           // If LED is on, start fade sequence
      for(int fadeValue = 255 ; fadeValue >= 0; fadeValue -=5) { 
        // sets the value (range from 0 to 255):
        analogWrite(firstLED, fadeValue);         
        // wait for 30 milliseconds to see the dimming effect    
        delay(30);   
        analogWrite(firstLED, 0);        
      } 
    }
  }
}
    analogRead(firstLED);          // check if LED is on

The analogRead() function returns data for an analog pin. Your LED is connected to a digital pin. You are not storing the value that you read (from the wrong pin). You don't need to read anything, because you assigned the value that the pin is at.

    if(firstLED == 255){           // If LED is on, start fade sequence

Look up a little ways in your code. See this line:

const int firstLED = 3;  // pin for the LED

In my universe, 3 is not equal to 255.

Add another variable, firstLEDvalue. After reading val, assign firstLEDvalue a value, of either 0 or 255. Then, use analogWrite(firstLED, firstLEDvalue) to set the pin to that value.

Then, the if test should be based on firstLEDvalue, not firstLED.

Your for loop is going to turn the LED on to some value, wait for some time, then set the PWM value to 0. Nearly immediately (a new nanoseconds later) change the PWM value. That analogWrite(firstLED, 0) in the for loop is not needed.

I'm not quite sure how to implement what you said, so here is what I got

const int firstLED = 3;  // pin for the LED
const int inputPin = 2; // input pin for the PIR sensor
const int firstLEDvalue = 3; 

void setup() {

  pinMode(inputPin, INPUT);  //declare PIR as input

}

void loop() {  
  int val = digitalRead(inputPin);           // read input value
  int firstLEDvalue = 255;                   // assign value
  if (val == HIGH)                           // check if the input is HIGH
  {
    analogWrite(firstLED, 255);              //turn LED on if motion dected
    analogWrite(firstLED, firstLEDvalue);    // write firstLEDvalue to firstLED
  }
  analogWrite(firstLED, 0);


  if(val != HIGH){                // check if the input isn't HIGH 
    analogRead(firstLEDvalue);          // check if LED is on
    if(firstLEDvalue == 255){           // If LED is on, start fade sequence
      for(int fadeValue = 255 ; fadeValue >= 0; fadeValue -=5) { 
        // sets the value (range from 0 to 255):
        analogWrite(firstLED, fadeValue);         
        // wait for 30 milliseconds to see the dimming effect    
        delay(30);   
     } 
    }
  }
}
const int firstLEDvalue = 3;
  int firstLEDvalue = 255;                   // assign value
    analogRead(firstLEDvalue);          // check if LED is on

So this should read from analog pin number 255 and discard the value? This statement is doing nothing useful and should be deleted.

  {
    analogWrite(firstLED, 255);              //turn LED on if motion dected
    analogWrite(firstLED, firstLEDvalue);    // write firstLEDvalue to firstLED
  }
  analogWrite(firstLED, 0);

Why is the first statement there? You will override it immediately.
Why is the second statement there? You will override it immediately.

Regardless of the actual sensor state, the LED is going to be turned off.

    if(firstLEDvalue == 255){           // If LED is on, start fade sequence

So, is this statement referring to the local instance that you unconditionally set to 255 or the constant global instance that you set to 3 (weird value)?

Ok I'm obviously confused on how to implement what you said

Add another variable, firstLEDvalue. After reading val, assign firstLEDvalue a value, of either 0 or 255. Then, use analogWrite(firstLED, firstLEDvalue) to set the pin to that value.

Can you show me how to properly add firstLEDvalue as variable, assign it 255, set the pin to that value, and then test it?

This is the best I can come up with

const int firstLED = 3;  // pin for the LED
const int inputPin = 2; // input pin for the PIR sensor


void setup() {

  pinMode(inputPin, INPUT);  //declare PIR as input

}

void loop() {  
  int val = digitalRead(inputPin);           // read input value
  int firstLEDvalue = 255;                   // assign value
  if (val == HIGH)                           // check if the input is HIGH
  {
    analogWrite(firstLED, firstLEDvalue);    // write firstLEDvalue to firstLED
  }

  if(val != HIGH){                // check if the input isn't HIGH 
    if(firstLEDvalue == 255){           // If LED is on, start fade sequence
      for(int fadeValue = 255 ; fadeValue >= 0; fadeValue -=5) { 
        // sets the value (range from 0 to 255):
        analogWrite(firstLED, fadeValue);         
        // wait for 30 milliseconds to see the dimming effect    
        delay(30);   
      } 
    }
  }
}

This is the best I can come up with

And? Does it work? What does it not do that you want it to do? What does it do that you don't want it to do?

The only problem I see with the code is that firstLED value gets assigned a value of 255 regardless of the state of the PIR. I would have assigned it different values if val was HIGH or LOW.

when the PIR throws HIGH the LED strobes. When the PIR throws LOW it fades while strobing repeatedly

when the PIR throws HIGH the LED strobes. When the PIR throws LOW it fades while strobing repeatedly

Not sure what you mean by "strobes". If the fading is not smooth, there is a problem with your LED or how it is wired.

I messed with my hardware a bit and It was my LED going bad... Now the LED stays solid when the PIR throws HIGH and fades (as it should) but now it cycles through the fade when the PIR throws LOW

Now the LED stays solid when the PIR throws HIGH and fades (as it should) but now it cycles through the fade when the PIR throws LOW

Have you changed the code so that firstLEDvalue has different values when the PIR throws HIGH and when it throws LOW? If not, then the behavior you are seeing is expected.

Thanks for the help so far but I still haven't found a solution to this...