one after another dot loop wont work

/*This is a Led show prototype, it has many modes and they can
be changed with a simple buttonpress */

int ledPins[] = {2,3,4,5,6,7,8,9,10,11,12,};
int ButtonPin = 12; //all the different pins/variables
int Buttonread1 = LOW;
int Buttonread2 = LOW;
int ButtonState = 0;

void setup()
{
for (int i = 0; i < 10; i++){
pinMode(ledPins*, OUTPUT); //LEDs all Output*

  • {*

  • pinMode(ButtonPin, INPUT); //Pot and button are input*

  • {*
    _ Serial.begin(9600); //serial for possible debugging_

  • }*

  • }*
    }
    }
    void loop()
    {

  • Buttonread1 = digitalRead(ButtonPin); //new data and delaytime*

  • delay(50);*

  • Buttonread2 = digitalRead(ButtonPin);*

  • while(Buttonread1 == HIGH && Buttonread1 == Buttonread2) {*

  • ButtonState = ButtonState + 1;*

  • delay(10);*

  • if(ButtonState == 5){*

  • ButtonState = 0;*

  • }*

  • Buttonread1 = LOW;*

  • Buttonread2 = LOW;*
    _ Serial.println(ButtonState);_

  • }*

  • {*

  • switch (ButtonState)*

  • {*

  • case 0: {*

  • nothing();*

  • break;*

  • }*

  • case 1: {*

  • lineforth();*

  • lineback();*

  • break;*

  • }*

  • case 2: {*

  • oneafteranotherfor();*

  • break;*

  • }*

  • case 3: {*

  • strobe();*

  • break;*

  • case 4: {*

  • inandout();*

  • }*

  • }*

  • }*

  • }*
    }

void nothing()
{
}
void lineforth()
{

  • for (int i = 0; i <= 9; i++){*
    _ digitalWrite(ledPins*, HIGH);_
    _
    delay(60);_
    _
    }_
    _
    }_
    void lineback()
    _
    {_
    _
    for (int i = 9; i >= 0; i--){_
    _ digitalWrite(ledPins, LOW);
    delay(60);
    }
    }*_

void oneafteranotherfor()
{
* int b;*
* int c;*
* for (int i = 0; i <= 9; i++)*
* b = i;*
_ digitalWrite(ledPins**, HIGH);
b = c;
if(b == 0)
{
c = 10;
}
digitalWrite(ledPins[c - 1], LOW);
delay(50);
}
void strobe()
{
}
void inandout()
{
}
[/quote]**
I got this but the second mode will not work. One led lights up and thats it. It switches to the right mode but it wont work.
Sorry for all these problemed posts but i am still learning alot.
it will not let manipulate i after the for loop more than once. I'd like help minimising my code and for it to be effective. Once i get the hang of things ill worry more about what to do than how to do it. ::slight_smile:_

You need to learn about where curly braces ({ and }) are required, and where they are not.

    [glow]{[/glow]
      switch (ButtonState)
      {
        case 0: [glow]{[/glow]
        nothing();
        break;
        [glow]}[/glow]
.
.
.
        }
        [glow]}[/glow]

The highlighted braces are not needed.

for (int i = 0; i <= 9; i++)
  b = i;

The "block" of code following the for statement is executed 10 times, in this case. The variable b is set to 10 different values.

Then, this code is executed:

  digitalWrite(ledPins[b], HIGH);
  b = c;
  if(b == 0)
  {
    c = 10;
  }
  digitalWrite(ledPins[c - 1], LOW);
  delay(50);

The led attached to ledPins[9] is turned on, variable b is now set to the value in c (0, since no explicit initialization was done), compared to 0 (true), and then c is set to 10, and the led connected to ledPins[9] is turned off. Total light on time is nearly 0.

After 50 milliseconds, the function ends.

Please explain what you meant to have this function do.

Yeah, i guess i need to fix some stuff. I want the mode to light 1 then 2, off 1, on 3, off 2, on 4, and so on. Thanks for the help.

im very confused. I need some serious help... :o :smiley: :cry:

Do you want two lights at a time on? After turning 1 on, should 2 be turn off immediately, or after some delay? How long until the next light is turned on? Once the last light is turned off, should the function end?

it should go one dot at a time through all the leds. It should repeat if no button is pressed. It somewhat works.

It somewhat works.

Show us what you have now. We'll help you make it work.

Hey there.

The problem is what the code is doing.

I still don't like the way you are getting the button being pushed:

Buttonread1 = digitalRead(ButtonPin); //new data and delaytime
delay(50);
Buttonread2 = digitalRead(ButtonPin);

while(Buttonread1 == HIGH && Buttonread1 == Buttonread2) {
ButtonState = ButtonState + 1;
delay(10);

if(ButtonState == 5){
ButtonState = 0;
}
Buttonread1 = LOW;
Buttonread2 = LOW;
Serial.println(ButtonState);
}

should be more like

//See if pushed and held
if(digitalRead(ButtonPin)) {
  delay(40);
  while(digitalRead(ButtonPin)) {

    //button is on...
    //set button flag to on
    button_flag = 1;

 }
}

And in the code for Nothing(), you should have:

if(button_flag) {
ButtonState = 1;
button_flag = 0;
}

...And at the end of the code for lineback()

if(button_flag) {
ButtonState = 2;
button_flag = 0;
}

I think that a large part of your problems in your code are stemmed from the fact that you don't change states in your functions, you are trying to do that when the button is held down.

void setup()
{
for (int i = 0; i < 10; i++){
pinMode(ledPins*, OUTPUT); //LEDs all Output*

  • {*
    [/quote]
    should be
    * *void setup() {  for (int i = 0; i < 10; i++){  pinMode(ledPins[i], OUTPUT);     //LEDs all Output  [glow]}[/glow]* *
    You also need to count the amount of {}'s used manually to see where you have too many, not just chuck a few in the end until it compiles.
    ie.
    { 1st

  • { 2nd*

  • { 3rd*

  • } 2nd*

  • { 3rd*

  • } 2nd*

  • } 1st*
    } finished

:slight_smile:

Ok, ill try to change my button reading method to the one you stated. I found that one of the problems in the second mode was that i wrote my for loop wrong. ::slight_smile: I think i'll try to get the modes written without help.
Thanks for all the support. This forum is really cool.

void linebackandforth()
{
for (int i = 0; i <= 9; i++){
digitalWrite(ledPins*, HIGH);*

  • delay(60);*
  • }*
  • for (int i = 9; i >= 0; i--){*
    _ digitalWrite(ledPins*, LOW);_
    _
    delay(60);_
    _
    }_
    _
    }_
    _
    [/quote]*_
    The new method is working. :wink: But sadly this code wont. It goes forward, back, the ALL instant forward, than normal back, then ALL instant forward, and back and so on. I dont see whats wrong.

You need one loop, not two. In that loop, set pin i HIGH and pin i-1, when i-1 is greater than 0, LOW. When i-1 is negative, set the highest pin LOW.

Wait, i think your thinking of my other effect. This should light all up in a row and then unlight them in a row, wouldent it be a go forward for loop and a back loop?

You need to clarify what linebackandforth is supposed to do, then.

Man, that function name is hard to type, and read. LineBackAndForth is much easier to understand where the words begin. Or, lineBackAndForth if you prefer lower case letters for the first letter of a function name.

"Lines" composed of discrete LEDs can hardly move back and forth.

Hello again,

I think that there is nothing wrong with that code: It seems to be well writen :slight_smile:

You can check this by using the following code:

void linebackandforth() {
for(;:wink: { // this is the prefered way to loop forever

for (int i = 0; i <= 9; i++){
digitalWrite(ledPins*, HIGH);*

  • delay(60);*
  • }*
  • for (int i = 9; i >= 0; i--){*
    _ digitalWrite(ledPins*, LOW);_
    _
    delay(60);_
    _
    }_
    _
    }_
    _
    }_
    _
    [/quote]_
    I would expect this to run fine.
    I'm suspecting that the problem is that it is jumping to another state after running through the code. To check this, add in "Serial.println("State = Nothing...");" at the top of nothing state, "Serial.println("State = Back and Forth...");" at the top of the second state, and so on...
    Try and see where it is going, and why it is going there.
    _
    :)*_

Well, the problem is it runs through onece correct, then it does that glitch thing. It worked fin when i had 2 separate functions, should i do that mabye?

As mentioned in your other post, its due to this

for (int i = 9; i >= 0; i--)

As its going from 9 to 0, and decrementing them by 1, ie it will get down to -1.

digitalWrite(ledPins*, LOW);*
When i = -1 you are tying to write to that pin, and there is no such pin as ledPins[-1]
If you get me.

@WanaGo
The i>=0 in the middle of the for loop says to execute the body of the loop only as long as i is greater than or equal to 0. i never gets to be -1 during execution of the body of the loop. i does get to be -1, but the conditional statement then causes the body of the loop to not be executed. Control transfers to the first statement after the end of the body of the loop.

Agreed however the condition is true with i = 0, therefore it will do a decrement making i = -1, and then executing the pinwrite.

void linebackandforth()  {
for(;;)  { // this is the prefered way to loop forever

   for (int i = 0; i <= 9; i++){
     digitalWrite(ledPins[i], HIGH);
     delay(60);
   }

   for (int i = 9; [glow]i >= 0[/glow]; i--){
     digitalWrite(ledPins[i], LOW);
     delay(60);
   }
 }
}

so if i=0, it will then do i--, so it will do a digitalwrite on ledpin[-1]

...

Yes? Or am I being silly

The initialization part of the for statement is executed (i = 0).

Then, the conditional part is checked (i<=9). If it true the body of the loop is executed, and then the final part of the for statement (i++) is executed, and the process begins again, except that the initialization step is not performed again.

The for statement:

for(int i=0; i<=9; i++)
{
   // Do something
}

is equivalent to

int i=0;
while(i<=9)
{
 // Do something
 i++;
}

The final part of the for statement (i++) is executed AFTER the loop body is eecuted, not BEFORE.

Yep your right, sorry bout that.