scrolling leds

Projects Programming
1

hi,
i'm new to the arduino community, and also to the programming with arduino
so i have kind of a basic question

i want to scroll through 3 leds connected to pins 9,10 and 11
i want to do this using a potentio (10k)

the problem is he doesn't scroll, it just lights up 1 led
no matter the value of the potentio

so this is what i've got

int potpin = 0;
int potval;
int led1 = 9;
int led2 = 10;
int led3 = 11;

void setup()
{
pinMode(led1, OUTPUT);
pinMode(led2, OUTPUT);
pinMode(led3, OUTPUT);
}

void loop()
{
int potval = analogRead(potpin);

if(0 < potval < 342)
{ digitalWrite(led1, HIGH);
digitalWrite(led2,LOW);
digitalWrite(led3,LOW);
}
else if (342 < potval < 684)
{ digitalWrite(led1, LOW);
digitalWrite(led2,HIGH);
digitalWrite(led3,LOW);
}
else if (potval > 684)
{digitalWrite(led1,LOW);
digitalWrite(led2,LOW);
digitalWrite(led3,HIGH);
}
}

2 
 if((0 < potval) && (potval < 342))

Etc

3

thank you, i knew it was gonna be a stupid mistake

4

Personally I would always put the variable on the left side of a comparison as it reads more naturally to me, especially when using and/or
if((potval > 0) && (potval < 342))as it makes it easier to see the range of values being checked, at least it does to me.

Both ways work, of course, but putting the value first seems unnatural somehow.

5

UKHeliBob:
Both ways work, of course, but putting the value first seems unnatural somehow.

The advantage of putting the value first comes in when you make this mistake: if ('a' = incomingChar)

6

I see an easier and more flexible way to do this than a bunch of if statements. I'd declare the ledPins in an array and then use a for loop to display them all. A value calculated from reading the pot decides whether the led is on or off. The advantage being that it becomes trivial to add more leds to the deal.

something like:

int limitValue = analogRead(potPin) / ((1024 / ledCount) + 1); // redundant parentheses for clarity
for (byte n = 0; n < ledCount; n++)
{
    if (n <= limitValue) digitalWrite(ledPin[n], HIGH);
    else digitalWrite(ledPin[n], LOW);
}

Then I'd probably add a variable and a condition to only update the leds if the limitValue has changed.

7

Personally I'd use map() to map the pot value to a range of 0-2 and use that for an array index selector.

8

majenko:
Personally I'd use map() to map the pot value to a range of 0-2 and use that for an array index selector.

One of these days I'll actually remember that function and use it. LOL

9

[quote author=James C4S link=topic=182047.msg1349204#msg1349204 date=1376151228]

UKHeliBob:
Both ways work, of course, but putting the value first seems unnatural somehow.

The advantage of putting the value first comes in when you make this mistake: if ('a' = incomingChar)
[/quote]I know what you mean but I still prefer the "variable first" construction and anyway I would never make such a mistake, well not often anyway. :slight_smile: