Weird error with Random numbers and a do - while filter

Hello!

I have a program where I want to randomly pick a number between 0 - 11 and turn on an LED attached to that pin. Subsequently I want to pick another pin, check that isn’t already picked and turn on that LED. This is repeated until 11 of 12 pins are on. If a random selection is already chosen, it keeps picking a random number until if finds one not used.

It almost works but it has a weird problem. See results file below:

TLC5974 test

7,10,8,0,11,9,6,2,7,4,5 Note: two 7s
4,11,3,10,9,7,1,0,5,8,2
9,8,2,5,1,4,7,6,3,11,10
2,7,4,1,9,8,5,0,3,6,10
0,7,4,9,6,8,11,2,5,10,1
7,6,5,4,1,10,9,2,0,11,3
9,8,2,5,1,4,7,6,3,11,10
5,6,2,9,10,8,1,4,3,11,0
3,9,7,11,2,4,1,8,5,6,0
2,7,4,1,9,8,5,0,3,6,10
2,9,7,6,8,3,1,5,11,0,4
11,5,8,0,3,1,9,6,11,10,2 Note: two 11s
5,11,3,8,1,2,7,4,10,9,6
9,8,2,5,1,4,7,6,3,11,10
4,11,3,10,9,7,1,0,5,8,2
4,11,3,10,9,7,1,0,5,8,2
5,11,3,8,1,2,7,4,10,9,6
7,8,9,11,10,3,0,6,4,2,5
5,6,2,9,10,8,1,4,3,11,0
5,6,2,9,10,8,1,4,3,11,0
5,6,2,9,10,8,1,4,3,11,0
8,2,4,1,7,9,11,6,5,0,10
2,7,4,1,9,8,5,0,3,6,10
0,1,10,6,9,7,2,5,0,11,3 Note: two 0s
9,8,2,5,1,4,7,6,3,11,10
11,5,8,0,3,1,9,6,11,10,2 Note: two 11s
7,6,5,4,1,10,9,2,0,11,3
3,9,7,11,2,4,1,8,5,6,0

Most of the text above was generated by the program; the ‘>’ and notes are my edits to highlight the problem. Here is the code:

/***************************************************
This program attempts to solve the call a random number multiple times problem

****************************************************/

#include “Adafruit_TLC5947.h”

// How many boards do you have chained?
#define NUM_TLC5974 2

#define data 4
#define clock 5
#define latch 6
#define oe -1 // set to -1 to not use the enable pin (its optional)

Adafruit_TLC5947 tlc = Adafruit_TLC5947(NUM_TLC5974, clock, data, latch);

int i;
int j;
int k;
int l;

void setup() {

Serial.begin(9600);

Serial.println(“TLC5974 test”);
tlc.begin();
if (oe >= 0) {
pinMode(oe, OUTPUT);
digitalWrite(oe, LOW);
}
}

void loop() {

// Set all pins to off
for(uint16_t i=0; i<12; i++) {
tlc.setLED(i,0, 0, 0);
tlc.write();
}

// This code section simply turns on all RGB LEDs to ensure all are white. This is to rule out an electrical problem.
tlc.setLED(0,500,500,500);
tlc.setLED(1,500,500,500);
tlc.setLED(2,500,500,500);
tlc.setLED(3,500,500,500);
tlc.setLED(4,500,500,500);
tlc.setLED(5,500,500,500);
tlc.setLED(6,500,500,500);
tlc.setLED(7,500,500,500);
tlc.setLED(8,500,500,500);
tlc.setLED(9,500,500,500);
tlc.setLED(10,500,500,500);
tlc.setLED(11,500,500,500);
tlc.write();
delay(1000);

int sel;
int sel1;
int sel2;
int sel3;
int sel4;
int sel5;
int sel6;
int sel7;
int sel8;
int sel9;
int sel10;
int sel11;
int cond;

int tim; // delay time
tim =2000;

randomSeed(analogRead(A0));

sel = random(12);
Serial.print(sel);
Serial.print(",");
tlc.setLED(sel,0,0,1000);
tlc.write();
delay(tim);
do {
sel1 = random(12);
} while (sel1 == sel);
Serial.print(sel1);
Serial.print(",");
tlc.setLED(sel1,0,0,1000);
tlc.write();
delay(tim);
do {
sel2 = random(12);
} while ((sel2 == sel1) || (sel2 == sel));
Serial.print(sel2);
Serial.print(",");
tlc.setLED(sel2,1000,0,0);
tlc.write();
delay(tim);
do {
sel3 = random(12);
} while ((sel3 == sel2) || (sel3 == sel1) || (sel3 == sel));
Serial.print(sel3);
Serial.print(",");
tlc.setLED(sel3,1000,0,0);
tlc.write();
delay(tim);
do {
sel4 = random(12);
} while ((sel4 == sel3) || (sel4 == sel2) || (sel4 == sel1) || (sel4 == sel));
Serial.print(sel4);
Serial.print(",");
tlc.setLED(sel4,0,1000,0);
tlc.write();
delay(tim);
do {
sel5 = random(12);
} while ((sel5 == sel4) || (sel5 == sel3) || (sel5 == sel2) || (sel5 == sel1) || (sel5 == sel));
Serial.print(sel5);
Serial.print(",");
tlc.setLED(sel5,0,1000,0);
tlc.write();
delay(tim);
do {
sel6 = random(12);
} while ((sel6 == sel5) || (sel6 == sel4) || (sel6 == sel3) || (sel6 == sel2) || (sel6 == sel1) || (sel6 == sel));
Serial.print(sel6);
Serial.print(",");
tlc.setLED(sel6,1800,1000,0);
tlc.write();
delay(tim);
do {
sel7 = random(12);
} while ((sel7 == sel6) || (sel7 == sel5) || (sel7 == sel4) || (sel7 == sel3) || (sel7 == sel2) || (sel7 == sel1) || (sel7 == sel));
Serial.print(sel7);
Serial.print(",");
tlc.setLED(sel7,1800,1000,0);
tlc.write();
delay(tim);
do {
sel8 = random(12);
} while ((sel8 == sel7) || (sel8 == sel6) || (sel8 == sel5) || (sel8 == sel4) || (sel8 == sel3) || (sel8 == sel2) || (sel8 == sel1) || (sel5 == sel));
Serial.print(sel8);
Serial.print(",");
tlc.setLED(sel8,1800,1000,0);
tlc.write();
delay(tim);
do {
sel9 = random(12);
} while ((sel9 == sel8) || (sel9 == sel7) || (sel9 == sel6) || (sel9 == sel5) || (sel9 == sel4) || (sel9 == sel3) || (sel9 == sel2) || (sel9 == sel1) || (sel9 == sel));
Serial.print(sel9);
Serial.print(",");
tlc.setLED(sel9,1000,0,1000);
tlc.write();
delay(tim);
do {
sel10 = random(12);
} while ((sel10 == sel9) || (sel10 == sel8) || (sel10 == sel7) || (sel10 == sel6) || (sel10 == sel5) || (sel10 == sel4) || (sel10 == sel3) || (sel10 == sel2) || (sel10 == sel1) || (sel10 == sel));
Serial.println(sel10);
// Serial.print(",");
tlc.setLED(sel10,1000,0,1000);
tlc.write();
delay(tim);
// delay(100000000);
//do {
// sel11 = random(12);
// } while ((sel11 == sel10) || (sel11 == sel9) || (sel11 == sel8) || (sel10 == sel7) || (sel11 == sel6) || (sel11 == sel5) || (sel11 == sel4) || (sel11 == sel3) || (sel11 == sel2) || (sel11 == sel1) || (sel11 == sel));
// tlc.setLED(sel11,1000,0,1000);
// tlc.write();
// delay(tim);

// int tim; // delay time in each fade loop
// tim =2000;

// Turns pin to red; red max = 2950,0,0
// tlc.setLED(sel,0,0,1000);
// tlc.setLED(sel1,0,0,1000);
// tlc.setLED(sel2,1000,0,0);
// tlc.setLED(sel3,1000,0,0);
// tlc.setLED(sel4,0,1000,0);
// tlc.setLED(sel5,0,1000,0);
// tlc.setLED(sel6,1800,1000,0);
// tlc.setLED(sel7,1800,1000,0);
// tlc.setLED(sel8,1800,1000,0);
// tlc.setLED(sel9,1000,0,1000);
// tlc.setLED(sel10,1000,0,1000);
// tlc.setLED(sel11,1000,0,1000);
// tlc.write();
// delay(tim);

}

Does anyone have a clue what is causing this problem?

Thanks!
Peter

“Sampling without replacement.”

I don’t remember if this is the “proper” technique/algorithm, but the way I do it is to create an array containing all of the possible values.

After an array element is randomly selected, the data “above” that element is “shifted left” to over-write that element and the range of random values is reduced (treating it as a smaller array).

Please read the sticky post at the top of the forum about how to properly post your code using code tags. It helps people help you.

You should also study up on arrays and for() loops so you don’t bloat your code the nearly identical code over and over

/***************************************************
  This program attempts to solve the call a random number multiple times problem

****************************************************/

#include "Adafruit_TLC5947.h"

// How many boards do you have chained?
#define NUM_TLC5974 2

#define data   4
#define clock   5
#define latch   6
#define oe  -1  // set to -1 to not use the enable pin (its optional)

Adafruit_TLC5947 tlc = Adafruit_TLC5947(NUM_TLC5974, clock, data, latch);

const unsigned long tim = 2000; // delay time

const int ledCount = 12;

bool selection[ledCount];

void setup() {

  Serial.begin(9600);

  Serial.println("TLC5974 test");
  tlc.begin();
  if (oe >= 0) {
    pinMode(oe, OUTPUT);
    digitalWrite(oe, LOW);
  }
  randomSeed(analogRead(A0));
}

void loop() {

  int sel;
  bool okay;

  // Set all pins to off and mark selections as not on
  for (int i = 0; i < ledCount; i++) {
    tlc.setLED(i, 0, 0, 0);
    tlc.write();
    selection[i] = 0;
  }

  // This code section simply turns on all RGB LEDs to ensure all are white. This is to rule out an electrical problem.
  for (int i = 0; i < ledCount; i++) {
    tlc.setLED(i, 500, 500, 500);
  }
  tlc.write();
  delay(1000);

  for ( int i = 0; i < ledCount - 1; i++ ) {
    okay = false;
    while ( !okay ) {
      sel = random(ledCount);
      if ( selection[sel] == 0 ) {
        // this selection is not on so use it
        okay = true;
        selection[sel] = 1;
      }
    }
    Serial.print(sel);
    Serial.print(",");
    tlc.setLED(sel, 0, 0, 1000);
    tlc.write();
    delay(tim);

    //  int tim; // delay time in each fade loop
    //  tim =2000;

    // Turns pin to red; red max = 2950,0,0
    //      tlc.setLED(sel,0,0,1000);
    //      tlc.setLED(sel1,0,0,1000);
    //      tlc.setLED(sel2,1000,0,0);
    //      tlc.setLED(sel3,1000,0,0);
    //      tlc.setLED(sel4,0,1000,0);
    //      tlc.setLED(sel5,0,1000,0);
    //      tlc.setLED(sel6,1800,1000,0);
    //      tlc.setLED(sel7,1800,1000,0);
    //      tlc.setLED(sel8,1800,1000,0);
    //      tlc.setLED(sel9,1000,0,1000);
    //      tlc.setLED(sel10,1000,0,1000);
    //      tlc.setLED(sel11,1000,0,1000);
    //      tlc.write();
    //      delay(tim);

  }
}

Last condition on the test of sel8 is wrong.

To randomize an array of numbers, you use a “Fisher Yates Shuffle”.

There have been plenty of implementations is c/c++ for Arduino. Mr Google will be your friend.

I think you will be generating the random shuffle and then managing the leds from the completed array rather than lighting leds on the fly. I think you will shuffle the 12 numbers 0 - 11, and then turn on the first 11 from the random list.

Here’s one implementation of the Fisher Yates Shuffle

void setup() {
  Serial.begin(115200);
  randomSeed(analogRead(A0));//floating pin
  byte a[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11};
  byte n = 12;

  for (int i = n - 1; i > 0; --i) {
    int j = random(0, i + 1); // 0 ≤ j ≤ i
    // exchange a[j] and a[i]
    int t = a[i];
    a[i] = a[j];
    a[j] = t;
  }
  for (byte k = 0; k<n;k++)
  {
    Serial.print(a[k]);
    Serial.print(" ");   
  }
  Serial.println();
}

void loop() {
  // put your main code here, to run repeatedly:

}

To all:

Thanks for all the great suggestions! Obviously as many could perceive, I'm not a coder. My special thanks to david_2018. I suspected I had a type-o somewhere, but despite looking at it until I was nearly cross-eyed, I still couldn't find it!

Thanks Again!
Peter

If your code is highly repetitious, its a sign something is wrong - and it will have many more places
for bugs to hide in.

This problem has at least two good styles of solution.

One is to keep track of the set of chosen values in some value or structure (a bit set / bit mask would
be good here).

Another is the take a list of the values to be chosen and randomize it fairly using a fair shuffle as mentioned already.

Lists and sets are two powerful higher level datastructures often needed for coding.