Kitchen LED lighting with fade in + out on motion detected

Hello all.

My first post here. Having just made a prototype of some in cupboard led lighting for my kitchen i thought i would share the code in case anyone else finds it useful.

This will work on an arduino, but I have it running on an attiny85 for space saving.

It will drive a SINGLE colour 12v led strip using a mosfet or npn transistor and a 5v regulator between the two. Be sure to use a PWM pin for the leds.

When motion is detected it will fade in the leds, and hold them on for the time you set in the script. If motion is detected whilst they are on it will reset the timer. Once motion is gone, it will fade off.

#define LEDPIN 0
#define PIRPIN 1

int ledState = 0;
unsigned long previousMillis = 0;
unsigned long currentMillis = 0;
boolean alreadyfadedup  = false;
boolean alreadyfadeddown  = false;
const long interval = 5000;  //amount of time the leds will stay up in millis

void setup() {
  pinMode(PIRPIN, INPUT);
}


void loop() {

  currentMillis = millis();

  if (digitalRead(PIRPIN) == 1) {
    ledState = 1;
    previousMillis = currentMillis;
    if (alreadyfadedup == false) {
      for (int i = 0; i < 256; i++) {
        analogWrite(LEDPIN, i);
        delay(2);
      }
      alreadyfadedup  = true;
      alreadyfadeddown = false;
    }
  }
  if (currentMillis - previousMillis >= interval) {
    if (ledState == 1) {
      if (alreadyfadeddown == false) {
        for (int i = 255; i >= 0; i--) {
          analogWrite(LEDPIN, i);
          delay(2);
        }
        alreadyfadeddown  = true;
        alreadyfadedup = false;
      }
      ledState = 0;
    }
  }
}

I hope someone has a use for this.

Nice work. I'm surprised 5 second PIR OFF delay is enough time.

Some comments on your code...

  1. you don't need to use pinMode() for output pins that use PWM ( analogWrite() )

  2. For loops usually use int i, j, then k. So, i++ better choice than o++ in second loop. In C++, for loop int i only survives inside the loop unless you declare them 'static', so you can assign another int i in a later loop.

  3. "ledState" instead of "ledstate" provides better variable naming readability. Same for other variable names.

  4. Good to see use of #define instead of common mistake of defining a variable which unnecessarily takes up RAM.

  5. For others, more comments would be helpful about what your code intends to do, and what processor it is run on.

Thanks for your comments. I have made some edits. I will comment the code more later on.

This is all new to me so im just glad it works as it should! 8)

I should add. That if you use a board like an uno or mega. (With a barrel jack connector). You can drive the led strip off the same power supply as the arduino.

I used a 12v barrel connection, which the uno 5v regulator then protects the processor. And ran the vin from the uno to the 12+ on the strip.

Saving on wiring. 8)

borland: 4. Good to see use of #define instead of common mistake of defining a variable which unnecessarily takes up RAM.

No no no, terrible to see the use of define!! C++ has the saver alternative of a const for that. And let the compiler optimizer deal with it. And with the small test sketch below you can see the macro version takes NO less RAM or ROM then the const version. The compile is smart, don't try to outsmart it ;)

#define TestVal 6
//const byte TestVal = 6;

void setup() {
  pinMode(TestVal, OUTPUT);
  digitalWrite(TestVal, HIGH);
}

void loop() {
  // put your main code here, to run repeatedly:
}

Both compile to 710 / 9 bytes.

@philicibine

borland: 3. "ledState" instead of "ledstate" provides better variable naming readability. Same for other variable names.

I agree but it applies to all variable names. dontnamethemlikethis butNameThemLikeThis. So alreadyFadedUp and alreadyFadedDown. And even do that for macros. So NOTLIKETHIS but LIKE_THIS.

BUT, those variables are unnecessarily and even confusing. Why? Because now all your state variables can be conflicting. Because would it make sens to have?

alreadyFadedUp = tue
alreadyFadedDown = true

Or even

alreadyFadedUp = false
alreadyFadedDown = true
ledState = 1

So those states are all linked together. It would have been better to use a single state for them. So use a single ledState and let it's value determine all this. For example: 0 = led off, no action 1 = fade on 2 = led on, no action 3 = fade off

        analogWrite(LEDPIN, i);
        delay(2);

If the only thing you want to do is control the light this might be okay but remember this is blocking code so you can't do anything else while fading.

Also, you fade linear which isn't how we see light. You get a nicer fade if you use gamma correction. If you want that and non-blocking code for fading, have a look at my signature ;)

Thanks for your input. I do see your various points. It is all a learning curve for me. I will try to make some changes later.

With regards to your library. One of my earlier iterations of the sketch did use it thanks. But in the end I figured that the blocking that the for loop does was only for a second and the circuit doesn't really need to do anything else.

Maybe it would prevent a complete fade-out if it had started but motion is detected. But I also wanted to try and make it work without using any libraries.

Again. Thanks for your input.

philicibine: Thanks for your input. I do see your various points. It is all a learning curve for me. I will try to make some changes later.

That's alright :)

philicibine: With regards to your library. One of my earlier iterations of the sketch did use it thanks. But in the end I figured that the blocking that the for loop does was only for a second and the circuit doesn't really need to do anything else.

That's because you fade pretty quick. But if you are 100% sure you don't want to add the blocking part isn't a problem. But keep in mind it's for example not easy to add dimming via a button or something.

And, like I said, gamma corrected will look better.

philicibine: Maybe it would prevent a complete fade-out if it had started but motion is detected.

For example, that is already something that's more complex to add now the code is blocking.

philicibine: But I also wanted to try and make it work without using any libraries.

Why? That is just bull. And with the Arduino environment you're already on the wrong way for that. Because even though you did not add it yourself, you are using libraries. digitalWrite(), digitalRead() and analogWrite() for example all come from a library. A build in library, but still. So not wanting to use a library is a weird argument ;)

And even if you want to learn, use libraries that do what you want. Even from that you'll learn. And if you keep doing projects you will start to dive into the code of those libraries and you'll see which libraries are terrible and you will start to edit them or write your own code etc. But "I want to learn" isn't a valid argument for not using libraries in my opinion.

And if you really want to try to do it yourself as an exercise, try to make it non-blocking and time it in the loop (instead of a for-loop). That would be a proper exercise. Using delay() isn't ;) (Chapter two of every Arduino tutorial should be called "DON'T EVER USE DELAY() AGAIN!")

So, having heard your comments i tried to include the FadeLED library.

I wrote the sketch again using fewer variables aswell.

However now, when powered up, there is a delay between the pir going high and the leds coming on. It looks like the leds dont come on until the pir pin goes low.

Any advice as to whats going on would be appreciated.

#include <LEDFader.h>
#include <Curve.h>

#define PIRPIN 12
LEDFader led = LEDFader(9);

int ledState = 0;
unsigned long previousMillis = 0;
unsigned long currentMillis = 0;
const long interval = 10000;  //amount of time the leds will stay up in millis

void setup() {
  pinMode(PIRPIN, INPUT);
}

void loop() {

  currentMillis = millis();

  if (digitalRead(PIRPIN) == 1) {
    previousMillis = currentMillis;
    led.fade(255, 3000);
    ledState = 1;
  }

  if (ledState == 1) {
    if (currentMillis - previousMillis >= interval) {
      led.fade(0, 3000);
      ledState = 0;
    }
  } led.update();
}

philicibine:
i tried to include the FadeLED library.

#include <LEDFader.h>

Spot the difference? :wink: But I must say they do look a quite the same…

You can try this

#include <FadeLed.h>

const byte PirPin = 12;

FadeLed led(9);

const unsigned int LightOffDelay = 10000;

unsigned long lightOnTime;

void setup() {
  led.setTime(3000);

  pinMode(PirPin, INPUT);
}

void loop() {
  FadeLed::update();
  
  if(digitalRead(PirPin)){
    lightOnTime = millis();
    led.on();
  }

  if(led.get() && (millis() - lightOnTime >= LightOffDelay) ){
    led.off();
  }

  //and here you can do all the other stuff you like :)
}

thanks, but for some reason the leds now stay on?

Whahaha, of course they will :roll_eyes: I wrote a bug into it... Please try the code again (I edited it) and please don't try to sport the error, it's embarrassing. :slightly_frowning_face:

i see what u did. but like i said im so new to this i blindly trusted, and copied and pasted!

Thanks anyways.