Help with code

Hi Looking for some help, I am trying to read a transmitter read the 3 values (2000, 1500, 1000) approx and use them to operate Leds. I am using a uno and my code works nearly. I have 3 temporary Leds on a bread board they switch on correct but stay on when value changes, digitalWrite LOW does not seem to work right.
int Pin_1 = 2;
int Led_1 = 3;
int Led_2 = 5;
int Led_3 = 6;

unsigned long duration;
void setup() {

Serial.begin (9600);
pinMode(Pin_1, INPUT);
pinMode(Led_1, OUTPUT);
pinMode(Led_2, OUTPUT);
pinMode(Led_3, OUTPUT);
}

void loop() {
int duration = (0);
duration = pulseIn(Pin_1, HIGH);
Serial.println(duration);
delay(1);

if (duration > 1800) { // 1987
digitalWrite (Led_1, HIGH);
}
else {
digitalWrite (Led_1, LOW);

if ((duration > 1100) && (duration < 1550)) { // 1493
digitalWrite (Led_2, HIGH);
}
else {
digitalWrite (Led_2, LOW);

if ((duration > 800) && (duration < 1000)) { // 919
digitalWrite (Led_3, HIGH);
}
else {
digitalWrite (Led_3, LOW);

}
}
}
}

Another_Transmitter_1.ino (802 Bytes)

Do you seriously imagine that digitalWrite (x, LOW) doesn't work?

Could your problem maybe, just maybe, be your code?

Uncompiled, untested

const byte Pin_1 = 2;
const byte Led_1 = 3;
const byte Led_2 = 5;
const byte Led_3 = 6;

void setup() 
{
  Serial.begin (9600);
  pinMode(Pin_1, INPUT);
  pinMode(Led_1, OUTPUT);
  pinMode(Led_2, OUTPUT);
  pinMode(Led_3, OUTPUT);
}

void loop() 
{
  unsigned long duration = pulseIn(Pin_1, HIGH);
  Serial.println(duration);
  delay(1);
  
  digitalWrite (Led_1, duration > 1800 ? HIGH : LOW);

  digitalWrite (Led_2, (duration > 1100) && (duration < 1550) ? HIGH : LOW);

  digitalWrite (Led_3, (duration > 800) && (duration < 1000) ? HIGH : LOW);
}

Wouldn't it be a lot more sensible to have

const byte Pin_2 = 2;

or, what about,

const byte pulseInputPin = 2;

...R

And what is this nonsense?

int duration = (0);
  duration = pulseIn(Pin_1, HIGH);

The parentheses around the 0 are not needed. Assigning an initial value to duration using one line of code and then, on the very next line, assigning it a different value is pointless.

Make it look (more) like you know what you are doing.

int duration = pulseIn(Pin_1, HIGH);

Better still, use the right datatype.

Well I did get it working just after post with this code:
int Pin_1 = 2;
int Led_1 = 3;
int Led_2 = 5;
int Led_3 = 6;

unsigned long duration;
void setup() {

Serial.begin (9600);
pinMode(Pin_1, INPUT);
pinMode(Led_1, OUTPUT);
pinMode(Led_2, OUTPUT);
pinMode(Led_3, OUTPUT);
}

void loop() {

duration = pulseIn(Pin_1, HIGH);
Serial.println(duration);
delay(1);

if (duration > 1800) { // 1987
digitalWrite (Led_1, HIGH);
digitalWrite (Led_2,LOW);
digitalWrite (Led_3,LOW);
}
else {
digitalWrite (Led_1, LOW);

if ((duration > 1100) && (duration < 1550)) { // 1493
digitalWrite (Led_2, HIGH);
digitalWrite (Led_1,LOW);
digitalWrite (Led_3,LOW);
}
else {
digitalWrite (Led_2, LOW);

if ((duration > 800) && (duration < 1000)) { // 919
digitalWrite (Led_3, HIGH);
digitalWrite (Led_1,LOW);
digitalWrite (Led_2,LOW);
}
else {
digitalWrite (Led_3, LOW);

}
}
}
}

I guessed I was going the wrong way, but thanks for your interest and will try some of your other ways
PS: int duration (0); wasn't supposed to be in it, that was something else I tried.

Thanks AWOL, that code worked, now I can move on, my aim is to put an arduino on a hex copter and use Ws2812 Leds that I can change via transmitter, and have the APM talk to the arduino to give warnings of battery state etc.
Thanks again for your help. Tel

Hi,
Welcome to the forum.

Please read the first post in any forum entitled how to use this forum.
http://forum.arduino.cc/index.php/topic,148850.0.html then look down to item #7 about how to post your code.
It will be formatted in a scrolling window that makes it easier to read.

Thanks.. just makes it easier to read.. Tom... :slight_smile:
PS, Then can you go back and edit the thread subject and add [SOLVED]