LED AND PIR

Using Arduino Project Guidance
1

Hello Everyone,

i'm currently working my arcade cabinet and i would like some help please, I just added my led display and I would like to turn off the leds with my pir, so when there is motion they turn off and when there isn't any motion the the demo will continue, the problem i'm having is if the leds are on just HIGH
no demo i can turn them off but when i add my demo loop i can't turn the demo off here is the short version of my sketch if possible could some one guide me in the right direction i'm a newbie.

arcade.txt (1.71 KB)

2

In the future, please post your code, enclosed in [code]code tags[/code], don't just attach it as a text document.
And before doing so, hit Ctrl-T or click ">Tools >Auto Format" in the IDE to format it properly.
Also, it's a good idea to only have one statement per line, to make your code easier to read.
(I've done this below.)

// Pir control
const int led0  = 3;
const int led1  = 5;
const int led2  = 4;
const int led3  = 9;
const int led4  = 10;
// control led with Pir when motion led
// turn off
const int pirPin = 2;
//varibles used to adjust pattern
const int  ledx = 10;
const int  delayloop = 100;
const int  masterDelay = 10;

void setup()
{
    pinMode(led0, OUTPUT);
    pinMode(led1, OUTPUT);
    pinMode(led2, OUTPUT);
    pinMode(led3, OUTPUT);
    pinMode(led4, OUTPUT);
    pinMode(pirPin, INPUT);
    delay(1000);
}
void loop()
{
    int pirState = digitalRead(pirPin); // Read Pir

    if (pirState == LOW) // motion, turns led off
    {
        digitalWrite(led0, HIGH);
        digitalWrite(led3, HIGH);
        digitalWrite(led1, HIGH);
        digitalWrite(led4, HIGH);
        digitalWrite(led2, HIGH);
    }

    else if (pirState == HIGH) //turn on led with no motion
    {
        digitalWrite(led0, LOW);
        digitalWrite(led3, LOW);
        digitalWrite(led1, LOW);
        digitalWrite(led4, LOW);
        digitalWrite(led2, LOW);
    }

    //loopforward();
}

void loopforward()
{
    for (int loopcount = 0 ; loopcount <= 8; loopcount += 1)
    {
        digitalWrite(led0, HIGH);
        delay(delayloop);
        digitalWrite(led0, LOW);      
        delay(delayloop);
        digitalWrite(led1, HIGH);     
        delay(delayloop);
        digitalWrite(led1, LOW);      
        delay(delayloop);
        digitalWrite(led2, HIGH);     
        delay(delayloop);
        digitalWrite(led2, LOW);      
        delay(delayloop);
        digitalWrite(led3, HIGH);     
        delay(delayloop);
        digitalWrite(led3, LOW);      
        delay(delayloop);
        digitalWrite(led4, HIGH);     
        delay(delayloop);
        digitalWrite(led4, LOW);      
        delay(delayloop);
    }
}
3

here is the short version of my sketc

Here is a short version of the solution

What you need to do is to add code that will ...

4

Are your LEDs connected between the output pin and +5V as the code/comment indicates? :-

if (pirState == LOW) // motion, turns led off
    {
        digitalWrite(led0, HIGH);

and is your PIR detector active-low? (Normally high, goes low when movement is detected.)

Do you have a link to the PIR detector?

but when i add my demo loop i can't turn the demo off

I assume that by "demo" you mean the 'loopforward()' function. Since it's outside the conditionals for the PIR, it will always run, once per cycle of 'loop()', (if you uncomment the call to it).

5

Are your LEDs connected between the output pin and +5V as the code/comment indicates? :-
ANSWER No they are not, does that make a difference?
and is your PIR detector active-low? (Normally high, goes low when movement is detected.)
ANSWER Yes
the led are high and i can turn them off with motion, but when i use loopforward and the lights
start blinking and fading the motion detection doesn't work

6

jxf2487:
Are your LEDs connected between the output pin and +5V as the code/comment indicates? :-
ANSWER No they are not, does that make a difference?

Yes it does. If the LEDs are connected between the output pin and ground, making them high in the following will turn the LEDs 'on', not 'off':-

if (pirState == LOW) // motion, turns led off
    {
        digitalWrite(led0, HIGH);

They would only turn 'off' when the pin goes high if they were connected between the output pin and +5V, (always with a series resistor, of course).

And this will turn the LEDs 'off':-

else if (pirState == HIGH) //turn on led with no motion
    {
        digitalWrite(led0, LOW);

So your code will make the LEDs turn 'on' with motion, 'off' when there's no motion.

.....but when i use loopforward and the lights
start blinking and fading the motion detection doesn't work

In the 'loopforward()' function, you have a total delay of 1 second, so nothing else can happen until the end of that delay. If you want movement detection during that cycle, you need to arrange your program differently and use 'millis()'-based timing for the LED blinking in 'loopforward()'.

and fading

There's no LED fading in your code, only blinking. LED fading would require 'analogWrite()', (on appropriate (PWM) pins, needless to say).

7

actually that's 1/4 of the original code, my original code is 712 lines and it has fading in it as well, i'm using a mega to run 19 arcade buttons i just want to turn them off during motion

8

i copied this code from someone else then i added and modified it he written the original with a button and he used a teensy

9

jxf2487:
actually that's 1/4 of the original code, my original code is 712 lines and it has fading in it as well, i'm using a mega to run 19 arcade buttons i just want to turn them off during motion

You can't turn buttons off because they are inputs. Do you mean stop reading them?

This is why we ask for all your code here. Which is what I was hinting at.

10

ok i'm sorry well i got an error exceeding 9000 characters so sent it as a text
i do apologize for the trouble but i'm a novice at this and i want to learn more
this is my first project

complete demo.txt (35.7 KB)

11

i want to learn more

Good because that is appalling code, let me show you how start to do it better.
You have each LED pin defined as a separate variable:-

const int led0  = 3;
const int led1  = 4;
const int led2  = 5;
const int led3  = 6;
const int led4  = 7;
const int led5  = 8;
const int led6  = 9;
const int led7  = 10;
const int led8  = 11;
const int led9  = 12;
const int led10 = 13;
const int led11 = 14;
const int led12 = 15;
const int led13 = 16;
const int led14 = 17;
const int led15 = 18;

now a minor point is that an int is two bytes and you can save a bit of memory by using a byte type variable. But the big problem with this is that you have a fixed name variable for each LED pin. If you could make that variable a variable then you can shorten the code considerably. You do this by making an array.

byte ledPin[] = { 3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18};

Now the value of ledPin[0] is 3, the value of ledPin[1] is 4,and so on. But the big thing is that the number inside the square brackets can be a variable, so if you want to do something to all the LEDs then put that in a for loop. For example:-

pinMode(led0, OUTPUT);
pinMode(led1, OUTPUT); 
pinMode(led2, OUTPUT); 
pinMode(led3, OUTPUT); 
pinMode(led4, OUTPUT); 
pinMode(led5, OUTPUT);
pinMode(led6, OUTPUT);
pinMode(led7, OUTPUT);
pinMode(led8, OUTPUT);
pinMode(led9, OUTPUT);
pinMode(led10,OUTPUT);
pinMode(led11,OUTPUT);
pinMode(led12, OUTPUT);
pinMode(led13, OUTPUT);
pinMode(led14,OUTPUT);
pinMode(led15,OUTPUT);

Becomes simply

for(int i=0; i<16;i++){
pinMode(ledPin[i],OUTPUT);
}

In the same way

void loopspeedDwnRvs() 
{ for (int loopspeed = 1 ; loopspeed <=101; loopspeed +=masterDelay) {
  digitalWrite(led15, HIGH);     delay(loopspeed);              
  digitalWrite(led15, LOW);      delay(loopspeed);  
  digitalWrite(led14, HIGH);     delay(loopspeed);              
  digitalWrite(led14, LOW);      delay(loopspeed);
  digitalWrite(led13, HIGH);     delay(loopspeed);              
  digitalWrite(led13, LOW);      delay(loopspeed);  
  digitalWrite(led12, HIGH);     delay(loopspeed);              
  digitalWrite(led12, LOW);      delay(loopspeed);  
  digitalWrite(led11, HIGH);     delay(loopspeed);              
  digitalWrite(led11, LOW);      delay(loopspeed);  
  digitalWrite(led10, HIGH);     delay(loopspeed);              
  digitalWrite(led10, LOW);      delay(loopspeed);                 
  digitalWrite(led9, HIGH);     delay(loopspeed);
  digitalWrite(led9, LOW);      delay(loopspeed);
  digitalWrite(led8, HIGH);     delay(loopspeed);
  digitalWrite(led8, LOW);      delay(loopspeed);
  digitalWrite(led7, HIGH);     delay(loopspeed);
  digitalWrite(led7, LOW);      delay(loopspeed);
  digitalWrite(led6, HIGH);     delay(loopspeed);
  digitalWrite(led6, LOW);      delay(loopspeed);
  digitalWrite(led5,HIGH);     delay(loopspeed);
  digitalWrite(led5,LOW);      delay(loopspeed);
  digitalWrite(led4,HIGH);     delay(loopspeed);
  digitalWrite(led4,LOW);      delay(loopspeed);
  digitalWrite(led3,HIGH);     delay(loopspeed);
  digitalWrite(led3,LOW);      delay(loopspeed);
  digitalWrite(led2,HIGH);     delay(loopspeed);
  digitalWrite(led2,LOW);      delay(loopspeed);
  digitalWrite(led1,HIGH);     delay(loopspeed);
  digitalWrite(led1,LOW);      delay(loopspeed);
  digitalWrite(led0,HIGH);     delay(loopspeed);
  digitalWrite(led0,LOW);      delay(loopspeed);}
}

Becomes:-

void loopspeedDwnRvs() 
{ 
for (int loopspeed = 1 ; loopspeed <=101; loopspeed +=masterDelay) {
     for(int i=0; i<16;i++){
         digitalWrite(ledPin[i], HIGH);     
         delay(loopspeed);              
         digitalWrite(ledPin[i], LOW);      
         delay(loopspeed);  
    }// end of LED itteration
}// end of loopspeed
} // end of function

Now this bit in your loop function looks wrong to me:-

else {
 digitalWrite(led0, HIGH);   
 digitalWrite(led3, HIGH);   
 digitalWrite(led1, HIGH);   
 digitalWrite(led4, HIGH);   
 digitalWrite(led2, HIGH);   
 digitalWrite(led5, HIGH);
 digitalWrite(led6, HIGH); 
 digitalWrite(led7, HIGH);}
 digitalWrite(led8, HIGH);
 digitalWrite(led9, HIGH);
 digitalWrite(led10,HIGH);
 digitalWrite(led11,HIGH);
 digitalWrite(led12,HIGH);
 digitalWrite(led13,HIGH);
 digitalWrite(led14,HIGH);
 digitalWrite(led15,HIGH);

LEDs 0 to 7 get set HIGH as a result of the test on the button state failing but LEDs 8 to 15 get set HIGH every time round the loop. Is that what you want here?

OK so go through your code and try and shorten it, remember loops can count forward as well as backward, but functions like your zigzag will need another layer of arrays so don't worry about that for now, I will show you how to do that once you have used this principle to make most of your code bearable to read.

12

i'm using a mega to run 19 arcade buttons

Note that for a Mega

PWM: 2 to 13 and 44 to 46. Provide 8-bit PWM output with the analogWrite() function.

So the fade code, as you have defined the LED pins, will not work correctly anyway. Even if you reassign them their are still not enough.

13

so i'm in Texas and its 7:20 pm, I don't know your location but everyday i have from 6 to 10 pm, to do this, so thanks for you help, I tried to compile what little you gave and i understand some of it i guess,So by using (const) I limited what my led could do ? and ( i ) represent 3-16 pins and ( for) is what i used to tell (i) what to do in the void loop? so is (for) a condition? or am i confused
by the way i can't compile the code i'm missing a step somewhere

// new arcade set up

byte ledPin[] = { 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18};

void setup () {
 
 for (int i = 0; i < 16; i++) {
    pinMode(ledPin[i], OUTPUT);
  }

  void loop() {
    void  loopspeedDwnRvs();
  }
  for (int loopspeed = 1 ; loopspeed <= 101; loopspeed += masterDelay); {
    for (int i = 0; i < 16; i++);
    digitalWrite(ledPin[i], HIGH);
    delay(loopspeed);
    digitalWrite(ledPin[i], LOW);
    delay(loopspeed);
  }
  // end of LED itteration
  // end of loopspeed
  // end of function

}
14

,So by using (const) I limited what my led could do

No you just use twice as much memory using int as you would using byte. The const bit is not so relevant it just means that if your code attempts to change it then it will throw a compile error.

Seeing as you had use the for loop in your code I assumed you knew what it was. I guess you just copied it from the origional code. It is used to loop a fixed number of time through the code in the curly braces following the statement. Look it up in the help bit of th IDE. The variable i is called the loop index every time through this is one higher. So the statement for(int i = 0; i <16; i ++) creates a variable called i, it could be any variable name but often i is used for index, it sets this equal to zero. The loop keeps going so long as the loop index is less that 16, and each time round the loop the variable i is increased by one. That is the i ++ bit.

That code will not compile because it is wrong. The loop function definition ends with the } and what follows should be a function definition of loopspeedDwnRvs() but it is not. Also you do not use the word void infront of a function unless you are defining it. You don't define functions inside functions. I think you need to do a lot more basic work on understanding code. Look at the examples in the IDE.

I am in the UK.

15

Grumpy_Mike:
No you just use twice as much memory using int as you would using byte. The const bit is not so relevant it just means that if your code attempts to change it then it will throw a compile error.

Using 'const' for a pin allocation also conserves SRAM and flash program memory.
In the following code, testing each of the two methods in turn:-
'byte' uses 184 bytes of SRAM and 2074 bytes of flash.
'const byte' uses 182 bytes of SRAM and 2068 bytes of flash.
So there are 3 good reasons for using 'const' when allocating pins:- the value can't be accidentally changed, less SRAM is used and less flash memory is used.

//byte led = 5;        // Bad :(
const byte led = 5;    // Good :)

void setup()
{
    Serial.begin(115200);
    pinMode(led, OUTPUT);
    digitalWrite(led, HIGH);
}

void loop(){}