Is there someone that can help me test this Code?

This is my first code with ardiuno.
I currently don’t own an arduino, but I plan to do in a near future.
So, test the capabilities of me programming the arduino, I tried doing this code that took me 2 hours of work.

And I would like someone to test it out and record that testing for me to watch.
I will be really glad If this code works at all.

If your interested you can PM me.

const int ledpins = {
2,3,4,5,6,7};
const int pinCount = 6;
const int btnpin = 1;

int onled = 2;
int counter;

void setup() {
pinMode(btnpin, INPUT);
int thisPin;
for (int thisPin = 0; thisPin < pinCount; thisPin++) {
pinMode(ledpins[thisPin], OUTPUT);
}
}

void loop() {
int sequence = 0;
int btnmode = digitalRead(btnpin);
if(btnmode == HIGH) {
int sequence = 1;
if(onled == 7) {
int onled = 2;
}
else {
int onled = onled + 1;
}
}
if(sequence = 1){
for(int counter = 2; counter == onled; counter++){
digitalWrite(counter, HIGH);
delay(50);
}
}
}

What are you trying to accomplish? Kind of hard to test if we don't know the desired output ;).

const [glow]int[/glow] ledpins[] = {
  2,3,4,5,6,7}; [glow] // use byte instead of int  (you don't have more than 256 pins, do you?)[/glow]
[glow]const int pinCount = 6;  // not needed,  use sizeof(ledpins) instead[/glow]
const int btnpin = 1;

int onled = 2;
int counter;

void setup() {
  pinMode(btnpin, INPUT);
[glow]  int thisPin;  // skip this since you declare a new variable in the forloop[/glow]
  for (int thisPin = 0; thisPin < pinCount; thisPin++) {
    pinMode(ledpins[thisPin], OUTPUT);  
  }
}

void loop() {
  int sequence = 0;
  int btnmode = digitalRead(btnpin);
  if(btnmode == HIGH) {
    int sequence = 1;
    if(onled == 7) {
      [glow]int[/glow] onled = 2; [glow] // every time you prefix with int you create a new unrelated variable![/glow]
    }
    else {
      int onled = onled + 1;
    }
  }
  if(sequence = 1){
    for(int counter = 2; counter == onled; counter++){
      digitalWrite(counter, HIGH);
      delay(50);
    }
  }
}

Great comments drhex, but the part I am most curious of is the for loop. I am trying to understand what Enddy is trying to accomplish.

for(int counter = 2; counter == onled; counter++){

Does the same as (I think);

if(onled == 2) {

Also, for sequence it's probably better to use type "boolean", which can be either false or true.

Tip for Enddy; if you want people to help you with code please use comments describing what should happen below the comment. A comment is prefixed with //.

For example:

// If button is down (sequence is 1 if button is down)
if(sequence = 1){
    // ? -- not sure what you want to do, but a comment could be:
    // Switch all LED's with number equal to or lower then onled on
    for(int counter = 2; counter == onled; counter++){
      digitalWrite(counter, HIGH);
      delay(50);
    }
  }

First thanks for the advise. I took some of the Ideas that you have posted.
Here is the Fixed Code:

//Set the pins.
const int ledpins = {
2,3,4,5,6,7};
const int btnpin = 1;

//Start the variables.
int onled = 2;
int counter;

//Setup the pins.
void setup() {
pinMode(btnpin, INPUT);
for (int thisPin = 0; thisPin < sizeof(ledpins); thisPin++) {
pinMode(ledpins[thisPin], OUTPUT);
}
}

//Start the loop to activate a sequence.
void loop() {
int sequence = 0;//Turn off the sequence.
delay(500);//So the button press counts only once.
int btnmode = digitalRead(btnpin);//Read the inpput
if(btnmode == HIGH) {//Analyse the input.
int sequence = 1;//Sequence is on.
if(onled == 7) {//Restart the pin selection.
int onled = 2;
}
else {//Move the pin selection.
int onled = onled + 1;
}
}
if(sequence = 1){//Turn on the sequence.
for(int counter = 2; counter == onled; counter++){//Blink every LED before the selected(Selected pin included).
digitalWrite(counter, HIGH);
delay(50);
digitalWrite(counter, LOW);
}
}
else {
digitalWrite(onled, HIGH);
}
}

I don’t use Boolean because 0’s and 1’s are cheap and small.

This is what is supposed to do:

Legend:
LED’s = - off + on

Button = ^ off v on

When turned on.

^ ±----

Sequence:

v ±----
^ ------
^ -±—
^ ------
^ -±— and stay on until next press.

I’m going to skip to the sixth press.

v ±----
^ ------
^ -±—
^ ------
^ -±—
^ ------
^ --±–
^ ------
^ —±-
^ ------
^ ----±
^ ------
^ -----+
^ ------
^ -----+ and stay on until next press.

v ±----
^ ------
^ ±---- and stay on until next press.

And Jump to the first sequence.

//Start the variables.
int onled = 2;

This is the one and only time you should have “int” (or even better “byte”) before onled. Remember:

if (onled == 7) {
    int onled = 2;
}

The “onled” you set to 2 here will disappear at the “}”, the assignment thus has no effect.

for (int thisPin = 0; thisPin < sizeof(ledpins); thisPin++) {

Sorry, my bad here. sizeof does not return the number of elements, but the number of bytes used by the array.
The proper expression for the number of elements is

sizeof(ledpins)/sizeof(ledpins[0])

If you had changed ledpins to a byte array, then ledpins[0] would have been 1 and so could be omitted.

for(int counter = 2; counter == onled; counter++){

Meinaart’s comment about the line above still applies. The expression between the semicolons determine if the loop body is to be run. So it could only run when counter equals onled. Since you initialize counter to 2, it only runs if onled is 2. With the current code, onled will always be 2 since the only code that tries to assign another value:

else {//Move the pin selection.
      int onled = onled + 1;
    }

declares a new temporary variable that will disappear at the “}”.

Code:

else {//Move the pin selection. int onled = onled + 1; }

declares a new temporary variable that will disappear at the "}".

How can I fix this? Can I just do this?:

else {//Move the pin selection. const int onled = onled + 1; }

You have a global variable ..

int onled = 2;

So later, when you set the value, you had left in the declaration (int) .. which defines a new variable instead of setting the global variable.

so int onled = onled + 1; defines a new variable .. also called onled .. which has nothing to do with the global one .. they are in different memory spaces.

this should be onled = onled + 1;

or better ... onled++;

Have you programmed in basic? if so, the Dim statement is much like the int declaration.

So the statement int onled = x would be the same as Dim onled as Integer ... which declares a new variable in that function. And when any function ends .. all variables defined in that function die a horrible death.

Hope that helps.

Thanks marklar for explaining how the variables work.

This all I got by now:

//Set the pins.
const int ledpins = {
2,3,4,5,6,7};
const int btnpin = 1;

//Start the variables.
int onled = 2;

//Setup the pins.
void setup() {
pinMode(btnpin, INPUT);
for (int thisPin = 0; thisPin < sizeof(ledpins[0]); thisPin++) {
pinMode(ledpins[thisPin], OUTPUT);
}
}

//Start the loop to activate a sequence.
void loop() {
delay(500);//So the button press counts only once.
int sequence = 0;//Turn off the sequence.
int btnmode = digitalRead(btnpin);//Read the input.
if(btnmode == HIGH) {//Analyse the input.
sequence = 1;//Sequence is on.
if(onled == 7) {//Restart the pin selection.
onled = 2;
}
else {//Move the pin selection.
onled++;
}
}
if(sequence = 1){//Turn on the sequence.
for(int counter = 2; counter <= onled; counter++){//Blink every LED before the selected(Selected pin included).
digitalWrite(counter, HIGH);
delay(50);
digitalWrite(counter, LOW);
}
}
else {
digitalWrite(onled, HIGH);
}
}

Please if you can explain how does sizeof() work I’ll be glad because drhex said something about it but I’m not sure what he meant.

sizeof returns the number of bytes something uses.
An int needs 2 bytes, and there are 6 of them in ledpins, so
sizeof(ledpins) == 12
siezof(ledpins[0]) = 2 // size of only the first int

Thus, the loop in setup will read outside the array. Tis can be fixed in two ways:

  1. const byte ledpins = { 2,3,4,5,6,7 };
    so that sizeof(ledpins) == 1*6

(with the for-loop condition thisPin < sizeof(ledpins) )

and/or

  1. for (int thisPin = 0; thisPin < sizeof(ledpins)/sizeof(ledpins[0]); thisPin++) {

You probably need to rethink the ‘sequence’ variable.
Do you intend it to become 1 when the button is pressed and then stay 1 for the rest of the program?
If so, it should be prefixed with “static” or have its declaration moved outside of loop. (now, every time loop starts over sequence is set to 0)

If you actually want sequence to be 1 only when the button is pressed, the variable is not needed. You could simply put the “blink” code inside the “if (btnmode == HIGH)” block.

Oh, and if you have a nice array of pins, why not use it also in the blink code instead of assuming the pin numbers are contiguous?

Another nice improvement might be to use the index of the array instead of the pinnumbers directly. That way it’s easier to change the pin numbers (and they don’t have to be ascending).

For example:

//Set the pins.
const int ledpins[] = {
  2,3,4,5,6,7};
const int btnpin = 1;
const int countLedPins; // holds number of elements in ledPins array

//Start the variables.
int currentIndex = 0;

//Setup the pins.
void setup() {
  pinMode(btnpin, INPUT);
  countLedPins = sizeof(ledpins) / sizeof(ledpins[0]);
  for (int thisPin = 0; thisPin < countLedPins; thisPin++) {
    pinMode(ledpins[thisPin], OUTPUT);  
  }
}

//Start the loop to activate a sequence.
void loop() {
  delay(500);//So the button press counts only once.
  boolean sequence = false;//Turn off the sequence.
  int btnmode = digitalRead(btnpin);//Read the input.

  if(btnmode == HIGH) {//Analyse the input.
    sequence = true; //Sequence is on.

    // the - 1 is because of indexes being zero based
    if(currentIndex == (countLedPins - 1)) {//Restart the pin selection.
      currentIndex = 0;
    }
    else {//Move the pin selection.
      currentIndex++;
    }

    // Line below is a shorter (but more unreadable) version
    // of the if and else statement above
    // See this url for the % (modulo) operator:
    // http://arduino.cc/en/Reference/Modulo

    // currentIndex = (currentIndex + 1) % countLedPins;
  }

  if(sequence){//Turn on the sequence.
    for(int index = 0; index <= currentIndex; index++){//Blink every LED before the selected(Selected pin included).
      digitalWrite(ledpins[index], HIGH);
      delay(50);
      digitalWrite(ledpins[index], LOW);
    }
  }
  else {
    // If button is not down just light the last LED
    digitalWrite(ledpins[currentIndex], HIGH);
  }
}

Btw; just buy a board and start testing. They are not expensive ;). A lot easier then debugging without a board.

I see drhex suggested the same thing as I did while I was writing the post ;). GMTA :P

This is the last one I got but when I verify it it says:
error: uninitialized const ‘countledpins’ In function ‘void setup()’:

It also highlights line 5.

//Set the pins.
const byte ledpins = {
2,3,4,5,6,7};
const byte btnpin = 1;
const byte countledpins; // holds number of elements in ledPins array

//Start the variables.
byte currentIndex = 0;

//Setup the pins.
void setup() {
pinMode(btnpin, INPUT);
countledpins = sizeof(ledpins) / sizeof(ledpins[0]);
for (byte thisPin = 0; thisPin < countledpins; thisPin++) {
pinMode(ledpins[thisPin], OUTPUT);
}
}

//Start the loop to activate a sequence.
void loop() {
delay(500);//So the button press counts only once.
boolean sequence = false;//Turn off the sequence.
byte btnmode = digitalRead(btnpin);//Read the input.

if(btnmode == HIGH) {//Analyse the input.
sequence = true; //Sequence is on.

// the - 1 is because of indexes being zero based
if(currentIndex == (countledpins - 1)) {//Restart the pin selection.
currentIndex = 0;
}
else {//Move the pin selection.
currentIndex++;
}
}
if(sequence){//Turn on the sequence.
for(byte index = 0; index <= currentIndex; index++){//Blink every LED before the selected(Selected pin included).
digitalWrite(ledpins[index], HIGH);
delay(50);
digitalWrite(ledpins[index], LOW);
}
}
else {
// If button is not down just light the last LED
digitalWrite(ledpins[currentIndex], HIGH);
}
}

const byte countledpins; // holds number of elements in ledPins array

It's a constant. So what's its value?

My fault, it should not be a const because value is set in the setup function. Remove the const part and it should work.

const byte ledpins[] = { 2,3,4,5,6,7};
const byte btnpin = 1;
const byte countledpins = sizeof(ledpins) / sizeof(ledpins[0]); // holds number of elements in ledPins array

You were totally correct mainaart it's not giving me that error anymore. Thanks to all of you for helping me.

#define COUNT(arr)  sizeof(arr) / sizeof(arr[0])

so we can use the even more obvious

COUNT(ledpins)

I just got some spare money and I will be buying it in the next few days. Does anyone know who is the cheapest seller for USA?

Because I'm planing to buy a few shields and I don't want it to be above $60.

I would try eBay. This might be an interesting thread for you: http://www.arduino.cc/cgi-bin/yabb2/YaBB.pl?num=1278528683