Minimising repetition in a set of random numbers

In this thread
https://forum.arduino.cc/t/avoid-repeating-a-random-number-within-a-set-time/1033379/
@noiasca kindly shared his code for delivering a set of pseudo random numbers with limited repetition. It worked fine here, both in his Wokwi simulation and when I ran it on a UNO with a low-going button. I've shown his code below, unaltered except that I changed

constexpr uint8_t btn {2};

which I didn't fully understand to

int const btn = 2;

ORIGINAL CODE:

/*
  Avoid repeating a random number
  https://forum.arduino.cc/t/avoid-repeating-a-random-number-within-a-set-time/1033379/

  items needs to be a lot larger than the size of the used buffer!
*/
const uint8_t items = 42;  // items on list
const uint8_t size = 10;   // size of ring buffer for already used items
uint8_t used[size];        // stores already used colors
byte index;                // ring buffer index

//constexpr uint8_t btn {2};
int const btn = 2;

void initUsed()
{
  for (byte i = 0; i < size; i++)
  {
    used[i] = random(0, items);    // scramble the content of the array (to allow 0 in the first n trys also)
  }
}

bool isUsed(byte actual)
{
  bool result = false;
  for (byte i = 0; i < size; i++)
  {
    if (actual == used[i]) return true;
  }
  return false;
}

int getRandomNumber()
{
  bool found = false;
  int rnd;
  while (found == false)
  {
    rnd = random(0, items);  
    if (!isUsed(rnd))
    {
      used[index] = rnd;
      index++;
      if (index >= size) index = 0;
      found = true;
    }
  }
  return rnd;
}

void action()
{
  byte actual = getRandomNumber();
    Serial.print("index: "); Serial.print(index);
    Serial.print("\tactual: "); Serial.print(actual);
    Serial.println();
    delay(100);
}


void setup() {
  Serial.begin(115200);
  Serial.println(F("Random with some uniqueness for the last minutes"));
//  randomSeed(analogRead(A4));
  randomSeed(analogRead(A0));
  pinMode(btn, INPUT_PULLUP);
  initUsed(); 
}

void loop() {
  if (digitalRead(btn) == LOW) action();

}

However I've been trying for a few hours to adapt it. But I'm unable to see why the code below fails. It does not deliver any printing when the button goes low.

MY CODE

const int btn = 2; // To get the results printed, take button low
int randNumberF = 1; // Temporary for testing

const int bufferSize = 10;  // Temp: size of ring buffer for some used tracks
int used[bufferSize]; // Array stores already used tracks
byte index; // Ring buffer index

const int folderSizes[26] {96, 106, 229, 159, 188, 142, 114, 173, 37, 26, 78, 252, 71, 69, 185, 24, 81, 162, 185, 77, 91, 33, 37, 43, 73, 83 };
const int tracks = folderSizes[randNumberF - 1]; // Tracks in the target folder
// Folders are numbered 1 to 26. In this example tracks is therefore 96

// FUNCTIONS ///////////////////////////////////////////
void initUsed()
{
  for (byte i = 0; i < folderSizes[randNumberF - 1]; i++)
  {
    // Shuffle the content of the array; allow 0.
    used[i] = random(0, tracks);
  }
}

bool isUsed(byte actual)
{
  bool result = false;
  for (byte i = 0; i < bufferSize; i++)
  {
    if (actual == used[i]) return true;
  }
  return false;
}

int getRandomNumber()
{
  bool found = false;
  int rnd;
  while (found == false)
  {
    rnd = random(0, tracks);
    if (!isUsed(rnd))
    {
      used[index] = rnd;
      index++;
      if (index >= bufferSize) index = 0;
      found = true;
    }
  }
  return rnd;
}

void action()
{
  byte actual = getRandomNumber();
  Serial.print("index: "); Serial.print(index);
  Serial.print("\tactual: "); Serial.print(actual);
  Serial.println();

  Serial.print("tracks = "); Serial.print(tracks);
  Serial.println();


  delay(100);
}

////////////////////////////////////////////////////////
void setup()
{
  Serial.begin(115200);
  delay(200);
  Serial.println(F("unique_random-e1"));
  randomSeed(analogRead(A0));
  pinMode(btn, INPUT_PULLUP);
  initUsed();
  Serial.print("tracks = "); Serial.print(tracks);
  Serial.println();

}

void loop()
{
  if (digitalRead(btn) == LOW) action();

}
void action()
{
  digitalWrite(
  byte actual = getRandomNumber();

This does not compile

How did you wire the button?

In the familiar way:
..on a UNO with a low-going button.

Think that must be some mistake I made when pasting the code into my post, because I can’t find that in my actual sketch. Using iPad right now but will check again tomorrow.

This is worrying when I compile your sketch for a Leonardo (I removed the failing line that was mentioned earlier).

C:\Users\sterretje\AppData\Local\Arduino15\packages\arduino\hardware\avr\1.8.5\cores\arduino\main.cpp: In function 'main':
C:\Users\sterretje\AppData\Local\Temp\.arduinoIDE-unsaved2022915-4116-1pjzrk6.dvfkg\sketch_oct15a\sketch_oct15a.ino:18:13: warning: iteration 10 invokes undefined behavior [-Waggressive-loop-optimizations]
     used[i] = random(0, tracks);
             ^

The warning relates to the function initUsed().

➜ the used array has 10 entries.
➜ in the loop your I index goes all the way to something in the folderSizes array which are all larger than 10
➜ so used[i] goes over the bound of the used array, and you are writing somewhere inappropriate in memory... bad things will happen..

1 Like

Excellent, thanks!

I made two mistakes in the function initUsed() and after correction the program now runs correctly.

const int btn = 2; // To get the results printed, take button low
int randNumberF = 1; // Temporary for testing

const int bufferSize = 10;  // Temp: size of ring buffer for some used tracks
int used[bufferSize]; // Array stores already used tracks
byte index; // Ring buffer index

const int folderSizes[26] {96, 106, 229, 159, 188, 142, 114, 173, 37, 26, 78, 252, 71, 69, 185, 24, 81, 162, 185, 77, 91, 33, 37, 43, 73, 83 };
const int tracks = folderSizes[randNumberF - 1]; // Tracks in the target folder
// Folders are numbered 1 to 26. In this example tracks is therefore 96

// FUNCTIONS ///////////////////////////////////////////
void initUsed()
{
  for (byte i = 0; i < bufferSize; i++)
  {
    // Shuffle the content of the array; allow 0.
    used[i] = random(0, bufferSize);
  }
}

bool isUsed(byte actual)
{
  bool result = false;
  for (byte i = 0; i < bufferSize; i++)
  {
    if (actual == used[i]) return true;
  }
  return false;
}

int getRandomNumber()
{
  bool found = false;
  int rnd;
  while (found == false)
  {
    rnd = random(0, tracks);
    if (!isUsed(rnd))
    {
      used[index] = rnd;
      index++;
      if (index >= bufferSize) index = 0;
      found = true;
    }
  }
  return rnd;
}

void action()
{
  byte actual = getRandomNumber();
  Serial.print("index: "); Serial.print(index);
  Serial.print("\tactual: "); Serial.print(actual);
  Serial.println();

  Serial.print("tracks = "); Serial.print(tracks);
  Serial.println();
  delay(100);
}

////////////////////////////////////////////////////////
void setup()
{
  Serial.begin(115200);
  delay(200);
  Serial.println(F("unique_random-e1"));
  randomSeed(analogRead(A0));
  pinMode(btn, INPUT_PULLUP);
  initUsed();
  Serial.print("tracks = "); Serial.print(tracks);
  Serial.println();

}

void loop()
{
  if (digitalRead(btn) == LOW) action();

}

Presumably your results are due to the mistakes I made that @J-M-L happily discovered. But it's odd that my program verified (on my UNO) without an error report.

Just to confirm what I thought. That fragment was not in my original (which naturally I had compiled before posting). Must have been finger trouble during the paste.

File ➜ preferences, have you set warning level to ALL?

Yes.

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.