Seven LED die problems

What I am doing is using seven LEDs to show a random number b/t 1 and 6 like the dots on a die.

Using arrays, I came up with successful code using for statements, but I wanted to declare the for statement to display it as a function. After compiling, it just showed all seven LEDs:

int pinArray[] = {6, 7, 8, 9, 10, 11, 12};
int timer = 100;
int randomNumber;
int i;
int roll[7];
int x;
int y = 0;
int roll1[] = {1, 0, 0, 0, 0, 0, 0};
int roll2[] = {1, 0, 0, 0, 0, 0, 1};
int roll3[] = {1, 0, 0, 1, 0, 0, 1};
int roll4[] = {1, 0, 1, 0, 1, 0, 1};
int roll5[] = {1, 0, 1, 1, 1, 0, 1};
int roll6[] = {1, 1, 1, 0, 1, 1, 1};

void setup() {
for(i=0; i<8; i++){
Serial.begin(9600);
pinMode(pinArray*, OUTPUT);*
}
}

void loop(){

• randomNumber = random(1, 7);*
• if(randomNumber == 1){*
• display(roll1);*
• }else{*
• if(randomNumber == 2){*
• display(roll2);*
• }else{*
• if(randomNumber == 3){*
• display(roll3);*
• }else{*
• if(randomNumber == 4){*
• display(roll4);*
• }else{*
• if(randomNumber == 5){*
• display(roll5);*
• }else{*
• if(randomNumber == 6){*
• display(roll6);*
• }*
• }*
• }*
• }*
• }*
• }*
• Serial.println(randomNumber);*
• delay(1000);*
  }
  int display(int roll[]){
• for(i=0; i<9; i++){*
  _ x = roll*;_
  _ if(x = 1){_
  _ digitalWrite(pinArray, HIGH);
  }else{
  if (x = 0){
  digitalWrite(pinArray, LOW);
  }
  }
  }
  }[/quote]
  (Someone please tell me how to do a scroll box) Why won't it work?*_

In your Display function. The if statments have "=" instead of "==". The single = is an assignment, you want the == which is a comparison. Since the assignment suceeds, it returns true, so the x = 1 is always true regardless of the value of x, and your else clause is never reached. This will turn on every LED as you say is happening.

to do a scroll box, use

 tags, not [quote]

When you're programming, use space at the start of lines to block off your control structures, it makes it much easier to read.

Also, you have declared array values for 7 spots but your loop tries to read 9 values.

int display(int roll[]){
for(i=0; i<9; i++) // this should be; i < 7
x = roll*;*
Likewise, change setup:
for(i=0; i<8; i++){ // should be i < 7
In C if there are seven elements in an array, the first array element is at index 0, the last is element is at index 6 so your loop should only run while the index i is less then 7

I would also add that you shouldn't nest your if statments like that. It's very sloppy. It makes the code a nightmare to read and dubug and in a non-trivial program makes it a lot more likely you'll have a nasty bug you won't be able to find.

You should be using a chain of "else if" statments or even better still, a "switch" statement.

Okay, Oracle, I changed it to a switch statement (which I would have used originally if i had known about it). After fixing my for statements, it still doesn't work.(fixed)

int pinArray[] = {6, 7, 8, 9, 10, 11, 12}; //Top left, top middle, top right, middle, bottom left, bottom middle, bottom right
int timer = 100;
int randomNumber;
int i;
int x;
int roll1[] = {0, 0, 0, 1, 0, 0, 0};
int roll2[] = {1, 0, 0, 0, 0, 0, 1};
int roll3[] = {1, 0, 0, 1, 0, 0, 1};
int roll4[] = {1, 0, 1, 0, 1, 0, 1};
int roll5[] = {1, 0, 1, 1, 1, 0, 1};
int roll6[] = {1, 1, 1, 0, 1, 1, 1};

void setup() { 
Serial.begin(9600);
for(i=0; i<7; i++){
 pinMode(pinArray[i], OUTPUT);
}
}


    
void loop(){
  randomNumber = random(1, 7);
switch (randomNumber) {
  case 1: randomNumber == 1;
   display(roll1);
   break;
  case 2: randomNumber == 2;
   display(roll2);
   break;
  case 3: randomNumber == 3;
   display(roll3);
   break;
  case 4: randomNumber == 4;
   display(roll4);
   break;
  case 5: randomNumber == 5;
   display(roll5);
   break;
  case 6: randomNumber == 6;
   display(roll6);
   break;
}
  Serial.println(randomNumber);
  delay(1000);
}

int display(int roll[]){
  for(i=0; i<7; i++){
    x = roll[i];
    if(x == 1){
      digitalWrite(pinArray[i], HIGH);
    }else{
      if (x == 0){
        digitalWrite(pinArray[i], LOW);
      }
    }
  }
}

It also doesn't print the random number to the serial monitor in the software, but this is only for testing purposes anyway.(fixed)

Okay, Oracle, I changed it to a switch statement (which I would have used originally if i had known about it). After fixing my for statements, it still doesn't work.

Please re-read the first line of my first reply

Greetings,

In your switch statement, the "randomNumber == 1;" stuff is unnecessary at best.

In your display function, the "if (x = 1) {" is an example of the #1 most common C mistake in the world. It should be "if (x == 1) {".

Regards,
David

I hate dumb mistakes like that. Well, its fixed. I changed it in my second post, although it's trivial anyway.

David's comment was that you can eliminate all the randomNumber == statements in the switch expression

So:

switch (randomNumber) {
case 1: randomNumber == 1;
display(roll1);
break;

can be :

switch (randomNumber) {
case 1: display(roll1);
break;

And don't let yourself get upset by the learning curve. If my experience is anything to go by, no matter how much you learn, little mistakes like these will keep cropping up. Enjoy them while they are still relatively easy to find

David's comment was that you can eliminate all the randomNumber == statements in the switch expression

So:

switch (randomNumber) {
case 1: randomNumber == 1;
display(roll1);
break;

can be :

switch (randomNumber) {
case 1: display(roll1);
break;

And don't let yourself get upset by the learning curve. If my experience is anything to go by, no matter how much you learn, little mistakes like these will keep cropping up. Enjoy them while they are still relatively easy to find

How does your replacement test to see if randomNumber is 1?

How does your replacement test to see if randomNumber is 1?

When you say say "switch (randomNumber)" it means you will be comparing everything to randomNumber.

Than when when you say "case 1:" it means the case when randomNumber is 1.
"case 254:" would mean the case when randomNumber is 254.

The switch statement compares the expression in parentheses (randomNumber) to all the cases. If a case matches the expression then execution starts from that case.

edit: Oracle beat me to it

Greetings,

In your switch statement, the "randomNumber == 1;" stuff is unnecessary at best.

In your display function, the "if (x = 1) {" is an example of the #1 most common C mistake in the world. It should be "if (x == 1) {".

I wonder why he understood this but not what I posted.

My #1 most common C mistake is leaving off semicolons

My #1 most common C mistake is leaving off semicolons

My #1 most common C mistake goes something like:

“hmm, I think that should be simple to do, I can knock it up in a few minutes??.

My #1 most common C mistake is leaving off semicolons

My #1 most common C mistake goes something like:

“hmm, I think that should be simple to do, I can knock it up in a few minutes??.

That was my #1 most common mistake in PIC assembly.

Oh, okay. Thanks, guys. Now I have to think of another program to write.