random note player

hello I've been trying to write a random note player which will play random notes at random intervals , however whenever I upload my code the notes always play at the same very quick rate?

const byte notesToPlay[] = {
  1, 2, 3, 4,5,6,7,8,9,10,}; //My notes

const unsigned long durations[] = {
  2000,2000,2000,2000,2000,2000,2000,2000,2000,2000};// my durations
these are all at 2000 so I can hear if there is a significant delay in the speed of the notes being played
unsigned long timeStartedPlaying; // a varible 

byte noteIndex; the randomised variable


boolean playing = false;// the bole an that kicks it off

long randNumber; // my random number








void setup() 
{
  Serial.begin(115200);



  randomSeed(analogRead(0));


}

void loop()
{

  Keyboard.press(KEY_CAPS_LOCK);// Im using the logic pro caps lock keyboard

  randNumber = random(1, 11); 

  noteIndex = randNumber; 

  Serial.print (noteIndex);











  while (!playing) // so I thought If i had all my note acutations inside this while loop with the condition to break the loop each time a note was played that…..(scroll down)
  {

    if (notesToPlay[noteIndex]==1){
      Keyboard.write ('a');
      playing=true;
    }

    if (notesToPlay[noteIndex]==2){
      Keyboard.write ('s');
      playing=true;
    }

    if (notesToPlay[noteIndex]==3){
      Keyboard.write ('d');
      playing=true;
    }

    if (notesToPlay[noteIndex]==4){
      Keyboard.write ('f');
      playing=true;
    }
    if (notesToPlay[noteIndex]==5){
      Keyboard.write ('g');
      playing=true;
    }

    if (notesToPlay[noteIndex]==1){
      Keyboard.write ('h');
      playing=true;
    }

    if (notesToPlay[noteIndex]==1){
      Keyboard.write ('j');
      playing=true;
    }
    if (notesToPlay[noteIndex]==1){
      Keyboard.write ('k');
      playing=true;
    }
    if (notesToPlay[noteIndex]==1){
      Keyboard.write ('l');
      playing=true;
    }
    if (notesToPlay[noteIndex]==1){
      Keyboard.write (';');
      playing=true;
    }
  }

  if (playing) {
    timeStartedPlaying = millis();  
  }

  if (playing && millis() - timeStartedPlaying >= durations[noteIndex])// this section would become true and the sketch would wait for required amount of time before setting playing to false again and playing another note from the loop?
  {
    playing = false; 






  }
}

thanks very much, any help would be much appreciated.

any help would be much appreciated.

There

doesn’t
seem

to

be

a

purpose

to
all
those

blank
lines.

Get rid of most of them

Using an analog pin value as a seed for the random number generator is not a good idea.

The only use of randNumber is to hold the value returned by random(), before it is assigned to noteIndex. Therefore, it is useless. Get rid of it.

There is no reason for noteIndex to be global.

Consistent placement of { makes for readable code. Yours isn’t.

okay I’ve tried to make all the amendments you suggested and re-write the code as a much shorter prototype to make it easier to read. this code doesn’t seem to print though.

basically what I’m trying to get the arduino to do is do something based off a random number and then make a non delay type pause of a duration which is also randomly taken from an array.

boolean change =false;
const byte prints[] = {
  1, 2, 3,}; 
const unsigned long times[] = {
  5,2000,200};
unsigned long timestartedplaying;



void setup(){
  Serial.begin(9600);
}



void loop() {

  byte common = random(1, 3);

  while (!change) {

    if (prints[common]==1){
      Serial.print (1);
      change=true;

    }
    else if  (prints[common]==2) {
      Serial.print (2);
      change=true;
    }
    else if (prints[common]==3) {
      Serial.print (2);
      change=true;
    }
  }


  while (change){
    timestartedplaying=millis();
    if (millis() -timestartedplaying >= times[common]){
      change=false;
    }
  }
}

prints[] has 3 elements.

The indices range from 0 to 2.

Why are you generating random numbers from 1 to 2 to address all the elements of the array?

Do you not recognize a blank line when you see one?

Don't you suppose that actually doing something with the timestartedplaying value would be useful? Like, when now minus then is less than interval, do something.

okay , is this any better

boolean change =false;
const byte prints[] = {
  1, 2, 3,}; //My notes
const unsigned long times[] = {
  5,2000,200};
unsigned long timestartedplaying;

void setup(){
  Serial.begin(9600);
}
void loop() {
byte common = random(1, 4);
while (!change) {
if (prints[common]==1){
      Serial.print (1);
      change=true;
}
    else if  (prints[common]==2) {
      Serial.print (2);
      change=true;
}
    else if (prints[common]==3) {
      Serial.print (2);
      change=true;
    }
  }
 while (change){
    timestartedplaying=millis();
    if (millis() -timestartedplaying >= times[common]){
      change=false;
    }
  }
}

when the interval is execeded the boolean change is set to false, which I thought would cause the program to enter into the !change while loop again ?

is this any better

Somewhat. A reasonable use of blank lines IS appropriate.

void setup()
{
  Serial.begin(9600);
}

void loop()
 {

Now, you are generating random numbers from 1 to 3, where the correct range is still 0 to 2.

when the interval is execeded the boolean change is set to false, which I thought would cause the program to enter into the !change while loop again ?

It will, where it may print another "1", "2", or "3" to the serial port. If that's all you want it to do, you're almost done. It does seem, though, that actually playing a note might be important.

ah yes I see my mistake. however when I upload

boolean change =false;
const byte prints[] = {
  1, 2, 3,}; 
const unsigned long times[] = {
  5,2000,200};
unsigned long timestartedplaying;

void setup(){
  Serial.begin(9600);
}
void loop() {
byte common = random(0, 2);
while (!change) {
if (prints[common]==1){
      Serial.print (1);
      change=true;
}
    else if  (prints[common]==2) {
      Serial.print (2);
      change=true;
}
    else if (prints[common]==3) {
      Serial.print (2);
      change=true;
    }
  }
 while (change){
    timestartedplaying=millis();
    if (millis() -timestartedplaying >= times[common]){
      change=false;
    }
  }
}

Nothing appears in the serial monitor at all ? I waited a good deal of time. when I have this concept working I will substitute serial.print for keyboard.write commands which will play notes in logic pro via the caps lock keyboard.

cheers sam

byte common = random(0, 2);

Pay attention this time. This sets the range of random numbers to 0 to 1. Is that what you want?

      Serial.print (1);

1 is not a printable character. “1” is. ‘1’ is.

    else if (prints[common]==3) {
      Serial.print (2);

It’s not copy and paste. It’s copy, paste, AND edit.

thank you.sorry.

I just tried

boolean change =false;
const byte prints[] = {
  1, 2, 3,}; 
const unsigned long times[] = {
  5,2000,200};
unsigned long timestartedplaying;

void setup(){
  Serial.begin(9600);
}
void loop() {
byte common = random(0, 3);
while (!change) {
if (prints[common]==1){
      Serial.print ("1");
      change=true;
}
    else if  (prints[common]==2) {
      Serial.print ("2");
      change=true;
}
    else if (prints[common]==3) {
      Serial.print ("3");
      change=true;
    }
  }
 while (change){
    timestartedplaying=millis();
    if (millis() -timestartedplaying >= times[common]){
      change=false;
    }
  }
}

still nothing appears in the serial monitor. I also tried swapping "" for '' with the same results?

thanks sam

still nothing appears in the serial monitor

I really don't see what's wrong. But, you [u]could[/u] try printing common, to see that it at least does something...

well I thought it would print 1, 2 and 3 at varying intervals? I tried printing common and still nothing appeared. I then deleted both while loops and common printed fine. I can't see why those loops are causing a problem though?

cheers

sam

I can't see why those loops are causing a problem though?

It's really hard to see any order to your code when it jumps all over the place, like yours does.

Put every { on a line by itself. Use Tools + Auto Format to properly indent your code. I think then that you'll see the problem. If not, at least it will be easier for me to see it.

ok I've put every curly brace on it's own line , and auto-fromatted my code , but I still can't see what the problem is. To my mind the boolean change starts out as false which should cause it to enter the first while loop and print one of the three numbers and then set change to true, which should cause it to enter the second while loop and wait for one of the durations before setting the change to false and entering the first while loop again.

boolean change =false;
const byte prints[] = 
{
  1, 2, 3,
}; 
const unsigned long times[] = 
{
  5,2000,200
};
unsigned long timestartedplaying;

void setup()
{
  Serial.begin(9600);
}
void loop() 
{
  byte common = random(0, 3);
  while (!change) 
  {
    if (prints[common]==1)
    {
      Serial.print ("1");
      change=true;
    }
    else if  (prints[common]==2) 
    {
      Serial.print ("2");
      change=true;
    }
    else if (prints[common]==3) 
    {
      Serial.print ("3");
      change=true;
    }
  }
  while (change)
  {
    timestartedplaying=millis();
    if (millis() -timestartedplaying >= times[common])
    {
      change=false;
    }
  }
}

thanks

sam

  while (change)
  {
    timestartedplaying=millis();
    if (millis() -timestartedplaying >= times[common])
    {
      change=false;
    }
  }

Write down the values of timestartedplaying (lousy name; timeStartedPlaying is far better), millis(), times[common], and change, for every pass through this while loop.

I'm sure that you will soon notice that timeStartedPlaying should be set when something happens (nothing does, now), not in the while loop.

[code]  while (change)//change is true
  {
    timestartedplaying=millis();= timestartedplaying is eqaul to the time since the program began so lets 7000 milliseconds
    
if (millis() -timestartedplaying >= times[common]) millis is 7000 , timestartedplaying is 7000 so 0,so if 0 is greater than common which will be either 200 , 5 , or , 200 (milliseconds)which it will be after duration equal to any of those times
    { then
      change=false; set change to false , 
    }
  }

is the problem that the loop runs much faster than the duration times so when timestartedplaying is set to millis it never exceeds any of the times ?

I changed it to

 while (change){
   
    if (millis() -timestartedplaying >= times[common]){
      change=false;
      timestartedplaying=millis();
    }
  }
}

and it now works , but I'm not sure I understand why? I suppose because change is set to false before timestartedplaying is set to millis it can now exit the while loop. I think I find it difficult to understand because it seems like on the first loop through the program wouldn't know what value time started playing is set to because it hasn't been set yet?

in any case thanks for helping me

}[/code]

You don't need change at all. In fact, you should get rid of it altogether.

You want something to happen in loop(), if it is time for something to happen.

You determine if it is time for something to happen by comparing now with the last time something happened. If the difference is greater than the time to the next event, it is time to do something. That can, and usually does involve setting the time to the next event. It always includes setting the time of the last event.

void loop()
{
   if(millis() - lastNotePlayed >= noteDuration)
   {
       playNextNote();
   }
}

Now, playNextNote() would randomly choose an note to play, which defines the time to play that note, so you'd set noteDuration in the playNextNote() function, as well as setting the lastNotePlayed time.

Can you see how to write the playNextNote() function? Does this make sense?

hmmm im not quite sure I just had a little go at writing something like that but when I looked at it again I realised it wouldn't have worked. I thought Boolean and while loops would be useful because I'm going to have 9 of these random note players in the same sketch that can be triggered or stopped by other inputs.

booleans are fine. while statements are not. The loop() function is called in an infinite loop. There is no reason to introduce another loop inside that. An if statement, to determine if it is time to do something is all that is needed.

I'm going to have 9 of these random note players

Get one working, first. Then, make that a function that you can call with different parameters (using arrays, probably). It may be necessary to call the function on every pass through loop.

ok , i got it working fine with while loops but then realised , as you say , that if I want these multiple random generators to be able to independently and simaltaenously I cannot use whiles. I then tried substituting the whiles for ifs but I got the same problem as I got at the beginning of the thread in that the timing section is completely ignored?

boolean change =false;
const byte prints[] = 
{
  1, 2, 3,}; 
const unsigned long times[] = 
{
  5,2000,200};
unsigned long timestartedplaying;

void setup()
{
  Serial.begin(9600);
}
void loop() 
{
  byte common = random(0, 3);
  if (!change) 
  {
    if (prints[common]==1)
    {
      Serial.print ("1");
      change=true;
    }
    else if  (prints[common]==2)
    {
      Serial.print ("2");
      change=true;
    }
    else if (prints[common]==3) 
    {
      Serial.print ("3");
      change=true;
    }
  }
  if (change)
  {
    if (millis() -timestartedplaying >= times[common])
    {
      change=false;
      timestartedplaying=millis();
    }
  }
}

I also tried

 if (change && millis() -timestartedplaying >= times[common])
    {
      change=false;
      timestartedplaying=millis();
    }
  }

but I think that just makes it slightly neater. I suppose the problem is with the latter timing section. as the numbers print without delay I can only assume that either change remains false , which seemed impossible as every time a number prints it sets change to true or the latter section executes instantly.

 if (change)//so change has been set to true by the printing of one of the numbers
  {
    if (millis() -timestartedplaying >= times[common]) // on the first go loop I can't imagine what this does as change has no \\value , but on the second go round it seems this should pause before...
    {
      change=false;//...change is set to false again...
      timestartedplaying=millis();…because timestartedplaying has been set to millis

if the above loop never completed due to the valueless nature of timestartedplaying on the first loop the numbers would not print?

I then tried setting timestartedplaying to millis at various additional points in the code but either there was no change or the numbers stopped printing entirely?

You don't need a change variable. Get rid of it.

The only thing that change is doing is telling you to choose another random number. That should happen automatically when the time to play the note has elapsed.