PIR digital read to LED digital write, problem

high guys. I’m new to programming, I have two PIR sensors, I want a green led to activate with one PIR active and a red LED to activate with both PIR’s active… if somebody could help me a little that would be sweet. THANKS

int PIRa = 3; // PIR sensor 1
int PIRb = 4; // PIR sensor 2
int LEDg = 7; // Green LED
int LEDr = 8; // Red LED

int PIRvalueA;
int PIRvalueB;

void setup() {
  // put your setup code here, to run once:

// to show LED's work
  pinMode(LEDg,OUTPUT);
  pinMode(LEDr,OUTPUT);

  digitalWrite(LEDg,HIGH);
  delay(1000);
  digitalWrite(LEDg,LOW);
  delay(500);
  digitalWrite(LEDr,HIGH);
  delay(1000);
  digitalWrite(LEDr,LOW);

}

void loop() {
  // put your main code here, to run repeatedly:
  pinMode(PIRa,INPUT);
  pinMode(PIRb,INPUT);
  pinMode(LEDg,OUTPUT);
  pinMode(LEDr,OUTPUT);


  if  (digitalRead,PIRa==HIGH || digitalRead,PIRb==HIGH) // if one or the other PIR active 
  {  
      digitalWrite(LEDg,HIGH);
      delay(1500);
  }

  if (digitalRead,PIRa==HIGH && digitalRead,PIRb==HIGH) // if both PIR's are active
  {
    digitalWrite(LEDr,HIGH);
    delay(1500);
  }

}

All you really need to do is look up the correct syntax for digitalRead() in the learning/reference page.

You are forgiven for not mentioning that it won't compile without errors - this time. :slight_smile:

If you look in the IDE under "Examples" you'll see a bunch of useful worked .... examples.

ok thanks, i'll give them a look.

it didn't give me any error codes at all.

also can any one suggest a good coarse/class for learning coding? C and C++

as a newbie to programming it would help a lot.

The examples of the IDE are a very good starting point :slight_smile:

@aarg, it does indeed compile. It's indeed weird but valid code using a function pointer in the comma operator :stuck_out_tongue:

Why are you calling pinMode in loop? Set the pin modes in setup and leave them be. It doesn't need to happen over and over. Once is enough.

OK I changed my code, now the LED's don't ever turn off. the PIR sensors give signal when something passes. why are the LED's always on

int PIRa = 3; // PIR sensor 1
int PIRb = 4; // PIR sensor 2
int LEDg = 7; // Green LED
int LEDr = 8; // Red LED

int PIRvalue = 0;

void setup() {
  // put your setup code here, to run once:

// to show LED's work
  
  pinMode(PIRa,INPUT);
  pinMode(PIRb,INPUT);
  pinMode(LEDg,OUTPUT);
  pinMode(LEDr,OUTPUT);


  digitalWrite(LEDg,HIGH);
  delay(1000);
  digitalWrite(LEDg,LOW);
  delay(500);
  digitalWrite(LEDr,HIGH);
  delay(1000);
  digitalWrite(LEDr,LOW);

}

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


  if  (PIRvalue = digitalRead(PIRa || PIRb)) // if one or the other PIR active 
  {  
      digitalWrite(LEDg,HIGH);
   
  }

  if (PIRvalue = digitalRead(PIRa && PIRb)) // if both PIR's are active
  {
    digitalWrite(LEDr,HIGH);
   
   }

}

You've got code to turn the leds on but no code to ever turn them off. So once they're on they're on for good. You need some code to write those pins LOW again under some condition.

if  (PIRvalue = digitalRead(PIRa || PIRb))

You've got two big issues here. First is that you used the assignment = instead of compare ==.

But the second is that is not how digitalRead works. You can't read two pins like that. Please don't just make up syntax. If you don't know how to write something, then go look at some of the examples.

I can't really even ferret out what you intended this line to do so I'm not real sure how one would go about fixing it. Why don't you explain what you were trying to do there.

it seems to be working a little better. still not quite what I want though...

is this better as far as the code goes?

int PIRa = 3; // PIR sensor 1
int PIRb = 4; // PIR sensor 2
int LEDg = 7; // Green LED
int LEDr = 8; // Red LED

int PIRvalue = 0;

void setup() {
  // put your setup code here, to run once:

// to show LED's work
  
  pinMode(PIRa,INPUT);
  pinMode(PIRb,INPUT);
  pinMode(LEDg,OUTPUT);
  pinMode(LEDr,OUTPUT);


  digitalWrite(LEDg,HIGH);
  delay(1000);
  digitalWrite(LEDg,LOW);
  delay(500);
  digitalWrite(LEDr,HIGH);
  delay(1000);
  digitalWrite(LEDr,LOW);

}

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


  if  (PIRvalue==digitalRead(PIRa) || PIRvalue==digitalRead(PIRb)==HIGH) // if one or the other PIR active 
  {  
      digitalWrite(LEDg,HIGH);
  }
  else
  {
      digitalWrite(LEDg,LOW);
  }

  if (PIRvalue==digitalRead(PIRa) && PIRvalue==digitalRead(PIRb)==HIGH) // if both PIR's are active
  {
    digitalWrite(LEDr,HIGH);  
   }
  else
  {
    digitalWrite(LEDr,LOW);
    
  }
}
PIRvalue==digitalRead(PIRb)==HIGH

Huh?

There are plenty of good examples for this sort of thing, why don't you look at some instead of just guessing around?

Delta_G:

if  (PIRvalue = digitalRead(PIRa || PIRb))

You’ve got two big issues here. First is that you used the assignment = instead of compare ==.

you suggested the ==

LandonW:
you suggested the ==

Yeah, you need == to compare two values. What was the line with three values in it supposed to be doing?

  if  (PIRvalue==digitalRead(PIRa) || PIRvalue==digitalRead(PIRb)==HIGH) // if one or the other PIR active 
  {  
      digitalWrite(LEDg,HIGH);
  }
  else
  {
      digitalWrite(LEDg,LOW);
  }

if either one is high??

I don't know how to write it and the examples don't really explain anything.
there is not much here for learning the mechanics of coding.

and in the examples it says to use a single "=" for val=digitalRead

PIRvalue==digitalRead(PIRb)==HIGH)

This compares PIRvalue with whatever PIRb reads. Then it compares the result of that comparison (ture or false) with HIGH.

OK, I'll give you one, but you seriously need to work through some tutorials and learn how the language works. Programming isn't something you can really learn by the seat of your pants. You're going to have to do a little study. If that's something you just don't want to do then get out now while you still can. This isn't a good hobby for people who don't like to sit and read.

If you want to see if either sensor went HIGH do something like this.

if(digitalRead(PIRa) == HIGH || digitalRead(PIRb) == HIGH)

See, that one compares both reads to HIGH and ors the two results together.

I don't know what the purpose of PIRvalue was in your code. It looks like you probably had a code at one time that read the state of the PIR into that variable and then compared the variable with HIGH. That works too, but if you have zero understanding of it and start trying to modify it then you end up with this ridiculousness. Instead, go do some learning and learn how the language works. It shouldn't take more than a day or two. Then you'll be on your way to being able to understand this stuff.

LandonW:
and in the examples it says to use a single "=" for val=digitalRead

Yes, that is if you want to set val to be equal to what you read from the pin. That's a completely different thing than trying to compare a value to something. You're talking about two completely different things.

thank you sir...

see the examples don't really tell you anything about that.
no, I don't mind study time. that's why I asked earlier in the convo for good places to learn and every body said "look at the examples"

so that's where I got the PIRvalue from.

will what you gave me work with && also? instead of ||

Google up a tutorial on C++. Learn the language and those examples will start making sense.