Random

Hi,

I try to turn on max 5 leds by pressing 2 buttons.
For this I used this script below.
But, after reset, the most time the same led is beaming.

Is there any improvement on it?

Gr.
Johan

int led[] = {3,4,5,6,7,8};
int numberLeds = 6;
int button1 = 9;
int button2 = 10;
 
void setup() {                
  for (i=0;i<5;i++){
    pinMode(led[i],OUTPUT);
    digitalWrite(led[i],LOW);
  }
  pinMode(button1,INPUT);
  pinMode(button2,INPUT);
}
 
void loop() {
  randomSeed(analogRead(0));
  if (digitalRead(button1)&&digitalRead(button2)){
    if(random(100)>50){
      digitalWrite(led[random(numberLeds)],HIGH);
    }
    if(random(100)>50){
      digitalWrite(led[random(numberLeds)],HIGH);
    }
    if(random(100)>50){
      digitalWrite(led[random(numberLeds)],HIGH);
    }
    if(random(100)>50){
      digitalWrite(led[random(numberLeds)],HIGH);
    }
    if(random(100)>50){
      digitalWrite(led[random(numberLeds)],HIGH);
    }
    while(true){
      delay(1000);
    }
  }
}

You random seed should be in "setup"

Why analogRead is a bad choice...

The reliable but not very sexy way to seed random...

And, of course, heed AWOL's advice.

As there's manual input ( button push ), you could use millis() of the first button press as a random seed, too.
Or simply call random(); in the beginning of loop() where now is your randomSeed(), this will be repeated an undefinite number of times, until both buttons are pressed.

BTW: Your code does not generate a sequence at all, as it never ends the loop().
Strange design... you need to press 3 buttons: Reset, then the 2 others together.

Edit: sent too early

Thanks all.
It's working for the purpose we want.
And yes, it's a strange design, but we have no time to make it better.
(pupils always start to late with their projects...)

Gr.
Johan

    while(true){
      delay(1000);
    }

That looks plain stupid. If you want the Arduino to stop doing anything useful,

    while(true)
    {
    }

It makes no difference whether it executes the while loop several million times per second, or wastes time in the delay() function.

Personally I use

while(1);

It is short, simple and easy to type. The danger is it could it lost in the code and cause it to "Hang".

exit( 1 );

...is a good choice. Stops execution. Easy to spot. No ambiguity regarding its purpose.

http://www.nongnu.org/avr-libc/user-manual/group__avr__stdlib.html#ga137096a48cc0c731052cadfb69c39b34

Would this still allow you to continue to service interrupts?
I assume the Watch Dog would still restart, even after an exit.

RandallR:
Would this still allow you to continue to service interrupts?

No. exit disables interrupts.

I assume the Watch Dog would still restart, even after an exit.

If the watchdog is configured to generate an interrupt the interrupt is ignored. If the watchdog is configured to reset the processor then, when it expires, the processor is reset.

Yes, that's a trap. Compare:

Serial.println ("Goodbye!");
while (true) ;

to:

Serial.println ("Goodbye!");
exit (1);

One prints "Goodbye!", one doesn't.