Problem with a REALLY simple code. Can't get an LED to turn on/off!

Hi all!

I'm trying to test something that seems very simple, but doesn't seem to be working... Here is the code:


//----- TVR Switchgear
int Input_Sidelights = 29;

int Output_Lights1 = 14;

int val = 0;

void setup() {
pinMode(Input_Sidelights, INPUT_PULLUP);
pinMode(Output_Lights1, OUTPUT);

void loop() {

val = digitalRead(Input_Sidelights);
if (val = 1) {
} else {digitalWrite(Output_Lights1,LOW);}


I'm trying to get an LED to light when I turn a switch on, and turn off when I turn the switch off. Simple.

I have tested val on the serial monitor, and it switches from 1 to 0 no trouble at all. It reads 0 when "on" and 1 when "off".

I've then tested digitalWrite(Output_Lights1,LOW) and it earths the pin and turns the LED on (the circuit I'm plugged into has a positive 5v feed to the LED so to turn it on, it needs to be grounded. I've also tested it the other way around and it goes out (not 100% though actually, but almost completely.

When I run the above code though, the LED stays in an off state and does not react to the switch. What am I doing wrong here??


if (val = 1) {

Please study the difference between the assignment operator (=) and the comparison operator (==):

Doh! I knew it was something obvious. Sorry and thank you!

You're welcome. I'm glad if I was able to be of assistance.

Don't be sorry. I'm certain every programmer has done this more than once. I know I have. We just have to be grateful when our bugs have an easy fix.


Thanks again! It's finally working... Last question, can this be shortened at all or is that as neat as it gets? I have a bunch of LEDs to control so would like to keep each operation to a minimum

val = digitalRead(Input_Sidelights);
if (val == 1) {
} else {digitalWrite(Output_Lights1,LOW);}

If you have no further use for the value of the reading, there is no need to save it in a variable:

if (digitalRead(Input_Sidelights) == HIGH) {
} else {

Note that I used HIGH instead of 1 in the comparison. Either will work, but HIGH makes the code easier to understand. Also, even though it's unlikely to ever be anything else, we have no guarantee that the value of HIGH will always be 1, but we do have a guarantee that HIGH will always correspond to a high state on the pin.

You could also do this one liner:

digitalWrite(Output_Lights1, digitalRead(Input_Sidelights));
 digitalWrite(Output_Lights1, digitalRead(Input_Sidelights));

Nice - Like that a lot thanks chaps! 8)