Outdoor lamp controlled by 2 buttons

Hello,

So, my project is to have an outdoor lamp mounted on the garage with two functions.

One button will sit on the outside for when i get home to have lights when in plug in the car caharger and go to the house.
Button 1 rule: Activate light for 1 minute.

The other button will be sitting on the inside with the following rules on push:
Button 2 rules:

  1. If the light is on, turn it off
  2. If the light is out, turn it on for 4 hours

I have came up with the code below (i'll skip the wiring, it's quite straight forward).
Pin 13 currently fires a LED, but will be attached to a 220V AC relay for the lamp.

Since this is my first project i would really appreciate any inputs on the approach below.

int intervalShort = 600000; //1 min
int intervalLong = 14400000; //4 hours

int interval = 0;
static unsigned char ledState = LOW;
static unsigned long ledCameOn = 0;

const int shortbuttonPin = 2;
const int longbuttonPin = 3;
const int ledPin =  13;

void setup()
{
  pinMode(shortbuttonPin, INPUT);
  pinMode(longbuttonPin, INPUT);
  pinMode(ledPin, OUTPUT);
}

void loop()
{ 
  // If the light is on, check if it should be off by now
  if(ledState == HIGH)
  {
    if(millis()-ledCameOn > interval)
    {
      digitalWrite(ledPin,LOW);
      ledState = LOW;
    }
  }

  // Short interval button is pressed, set it to short timer
  if(digitalRead(shortbuttonPin) == HIGH)
  {    
    interval = intervalShort;
    digitalWrite(ledPin,HIGH);
    ledState = HIGH;
    ledCameOn = millis();
  }
  
  //Long interval button is pressed, start timer or shut off
  if(digitalRead(longbuttonPin) == HIGH)
  {
    if(ledState == LOW)
    {
      ledState = HIGH;
      interval = intervalLong;
      digitalWrite(ledPin, HIGH);
      ledCameOn = millis();
    }
    else
    {
      ledState = LOW;
      digitalWrite(ledPin, LOW);
    }
  }
}

Hi,
I’ve given you a karma for using code tags.
You didn’t say if your code works, or has a problem.

What happens if button1 is pressed while the LED is in its 4hour ON period?

Thanks… Tom… :slight_smile:

Linus1337:

int intervalShort = 600000; //1 min

int intervalLong = 14400000; //4 hours

int interval = 0;
static unsigned char ledState = LOW;
static unsigned long ledCameOn = 0;

const int shortbuttonPin = 2;
const int longbuttonPin = 3;
const int ledPin =  13;

The static keyword is not doing anything useful.
The int for intervals is the wrong type; use unsigned long ints for that (or it just doesn't fit for starters, and unsigned for properly rolling over).

ledState should be a byte or bool; unsigned char is an alias of byte but just doesn't make sense here. So you end up with these variable declarations:

const unsigned long intervalShort = 600000; //1 min
const unsigned long intervalLong = 14400000; //4 hours

unsigned long interval = 0;
byte ledState = LOW;
unsigned long ledCameOn = 0;

const byte shortbuttonPin = 2;
const byte longbuttonPin = 3;
const byte ledPin =  13;

The way you programmed it, the short interval button will override the 4 hours.

nangulbas:
I'd make four comments:

  • You're looking for the buttons to go high when pressed, so do you have pull down resistors installed to hold the pins low while the buttons are not pressed? It's easier and more usual to work the other way round; use a pullUP and look for the button to go low. The processor has pullUPs built in, and just turn them on with INPUT_PULLUP in pinMode. That saves f@rting around with loose extra components.
  • Button 2 has two purposes which is fine, ie turn off if on, or on if off. But the result of the press is surely going to depend on how long you hold it down. Let's say the led is off, and you press button 2. Led goes on. But loop() is so fast, the button will still be pressed that split second later when it checks the button again and the "if" which is checking the ledstate will have a different result. You probably ought to look at the state change detect tutorial, and only act on new presses of the button
  • "static" is to prevent a variable being re-initialised inside a function. I'm not sure of the effect (if any?) of using it like you did at the top as in static unsigned char ledState = LOW; but it's unnecessary to use "static" there.
  • When you get to the relay stage (and of course you'll choose one that suits the electrical characteristics of the lamp :wink: ) and assuming you get a relay module on a board not a loose relay, check its logic. Many require a low to go on, some require a high, some have a jumper so you can choose. Make sure to match the high or low of what is now high to "on" the led, to the relay.

Thanks for inputs!

  1. The button is wired as in the link.

  2. This is great info. I was actially thinking of using delay() for some second after the press actions for avoiding the next press iteration.

  3. I feel a bit embarresed about this one. To my defence this project took the best out of me - i posted and went to bed about 06.00 in the morning. Really fun to fiddle with electronics and programming, apparently. :slight_smile:
    Good spot.

  4. Noted. No relay yet.

  5. Oh! I was using much shorter intervals for test and changed these two values untested for posting on the forum. I guess long is the way to go. Can't fint bigint?

Linus1337:
bigint?

Not a valid type in C++.

wvmarle:
The way you programmed it, the short interval button will override the 4 hours.

Yes. That’s on purpose. 4 hours is just to have some kind of automated switch off, it will probably never fire off. The thinking is that the long time interval is for i.e. changing tyres, when ready i’ll leave the garage and press inside button just to switch off the outside light, or hit the outside button so the light will be there while i go to the house and then go off after a minute.

As you want the inside button to toggle the light, it needs debounce - i.e. switch on the moment the button goes high (state change), and then ignore any changes for 50 ms to allow the button to stop bouncing. Otherwise you'll have a 50/50 chance of actually switching on/off the light on each individual press :slight_smile:

int intervalShort = 600000; //1 min
int intervalLong = 14400000; //4 hours

int interval = 0;
static unsigned char ledState = LOW;
static unsigned long ledCameOn = 0;

all of your time variables should be the same type and that type unsigned;

8 bit byte / 16 bit word / 32 bit unsigned long / 64 bit unsigned long long

What millis() and micros() return is 32 bit unsigned long but you can time shorter intervals with 8 or 16 bits. When timing a minute or less, using 16 bit millis runs faster and uses less RAM. 8 bit millis works for debounce periods.