Christmas Sweater - trouble with values not updating

Hi, I'm trying to make a christmas sweater that "sings" and has flashing LEDs. So far I've got it working with the test tone of cdef, but when I try to add multiple songs to the mix, my values of "notes[]" and "beats[]" don't update.

The code is supposed to run through once with the song "cdef" and then pick another song at random and play it instead. However, it just plays "cdef" over and over again. I checked, and serial monitor shows that "songsRand" and "notes[]" change at the right times, but after it loops, it just plays "cdef" again.

Hopefully I'm just missing something simple. I had to cut down my code to be within the 900 character limit, but I'm hoping to include 11 songs total that all work if I put them in one at a time as the first spot.

Thanks for looking!

int speakerPin = 13;
int led10 = 12;

int tempo = 150; //originally 300
int songs = 2; //number of songs available
int songsRand = 0;  //random songs, starting with test #0
int length = 5; //variable for length of song
char notes[] = "cdef "; //variable for number of notes in a song
int beats[] = { 1,1,1,1,4 }; //variable for beats per note in a song

void setup() {
  Serial.begin(9600);
  pinMode(speakerPin, OUTPUT);
  pinMode(led10, OUTPUT);
}

void loop() {
  
  //****** 000 - Test Song ******
  if(songsRand == 0){
  int length = 5; // the number of notes
  char notes[] = "cdef "; // a space represents a rest
  int beats[] = { 2,2,2,4,4 };
  Serial.println("Starting with:  Test Song");
  Serial.print("song notes contain:  ");
  for (int i = 0; i < length; i++){ Serial.print(notes[i]); } Serial.println("."); Serial.println();
  }
  //***********************************************
  
  
  
  Serial.print("start of void loop, notes to play: ");for (int i = 0; i < length; i++){ Serial.print(notes[i]); } Serial.println("."); Serial.println();
  for (int i = 0; i < length; i++) {
    if (notes[i] == ' ') {
      delay(beats[i] * tempo); // rest
    } else {
      playNote(notes[i], beats[i] * tempo);
    }

    // pause between notes
    delay(tempo / 2); 
  }
  Serial.println("end of void loop, time to pick a new song!");
  int songsRand = random(1,songs);
  Serial.print("Random song chosen is:  ");Serial.println(songsRand);


  //****** 001 - Twinkle Twinkle Little Star ******
  if(songsRand == 1){
  int length = 15; // the number of notes
  char notes[] = "ccggaagffeeddc "; // a space represents a rest
  int beats[] = { 1,1,1,1,1,1,2,1,1,1,1,1,1,2,4 };
  Serial.println("Song Chosen is:  (1) Twinkle Twinkle Little Star");
  Serial.print("song notes contain:  ");
  for (int i = 0; i < length; i++){ Serial.print(notes[i]); } Serial.println("."); Serial.println();
  }
  //***********************************************
  
  
  //****** 002 - God Rest Ye Merry Gentlemen ******
  if(songsRand == 2){
  int length = 69;
  char notes[] = "ddaagfedcdefga ddaagfedcdefga avgavCDagfdefgfgavaagfedfedgfgavCDagfed";
  int beats[] = { 2,2,2,2,2,2,2,2,2,2,2,2,2,4,2,2,2,2,2,2,2,2,2,2,2,2,2,2,4,2,2,2,2,2,2,2,2,2,2,2,2,2,2,4,2,2,4,2,2,2,2,2,2,4,1,1,2,4,2,2,2,2,2,2,2,2,2,2,8 };
  Serial.println("Song Chosen is:  (2) God Rest Ye Merry Gentlemen");
  Serial.print("song notes contain:  ");
  for (int i = 0; i < length; i++){ Serial.print(notes[i]); } Serial.println("."); Serial.println();
  }
  //***********************************************



} // end of void loop

//***********************************************
//*************** Functions *********************
//***********************************************

void playTone(int tone, int duration) {
  for (long i = 0; i < duration * 1000L; i += tone * 2) {
    digitalWrite(speakerPin, HIGH);
    delayMicroseconds(tone);
    digitalWrite(speakerPin, LOW);
    delayMicroseconds(tone);
  }
}

void playNote(char note, int duration) {
char names[] = { 'c', 'd', 'e', 'f', 's', 'g', 'a', 'v', 'b', 'C', 'D', 'E' };  //note "s" is a f#, v is a b-flat.
int tones[] = { 1915, 1700, 1519, 1432, 1352, 1275, 1136, 1073, 1014, 956, 852, 758 };

  // play the tone corresponding to the note name
  for (int i = 0; i < 8; i++) {
    if (names[i] == note) {
      digitalWrite(led10, HIGH);
      playTone(tones[i], duration);
      digitalWrite(led10, LOW);      
    }
  }
}

You have way too many arrays of the same name. You should have ONE array called notes and ONE array called beats that you change the values in.

Sorry to be dense, but I only really want one array. I want that array to change its value based on picking a random song.

Shouldn't "notes[]" update if the songsRand equals 2 so that the value of notes[] isn't "cdef" but instead update with "ddaagfedcdefga ddaagfedcdefga avgavCDagfdefgfgavaagfedfedgfgavCDagfed" for the 2nd song?

That's why I put the debugging serial.print in there to ensure the value of notes[] changes. However, when it goes back into the loop to play the song in notes[], it's somehow reverted back to "cdef" even though the serial.print line shows it has a new value in it when it reaches that part of the code.

Is there a trick to having arrays update other than what I'm doing by saying
if(songsRand == 2){
int length = 69;
char notes[] = "ddaagfedcdefga ddaagfedcdefga avgavCDagfdefgfgavaagfedfedgfgavCDagfed";
}

Thanks,

Perhaps:

const char *notes[] = {
  "ddaagfedcdefga ddaagfedcdefga avgavCDagfdefgfgavaagfedfedgfgavCDagfed",
  "ccggaagffeeddc",
  "Notes for your third song",
  "Notes for your fourth song" 
};

#define NUMBER_OF_SONGS sizeof(notes)/sizeof(char *)

Then use

notes[songsRand]

as a pointer to the string containing the song to play.

Neat idea! I'll give it a shot and let you know the results. I'll have to look up a few of the things you referenced to really "get it". Thanks :slight_smile:

  if(songsRand == 1){
  int length = 15; // the number of notes
  char notes[] = "ccggaagffeeddc "; // a space represents a rest
  int beats[] = { 1,1,1,1,1,1,2,1,1,1,1,1,1,2,4 };
  Serial.println("Song Chosen is:  (1) Twinkle Twinkle Little Star");
  Serial.print("song notes contain:  ");
  for (int i = 0; i < length; i++){ Serial.print(notes[i]); } Serial.println("."); Serial.println();
  }

In this block of code, "notes" and "beats" are local variables completely different from the global variables of the same name. You can initialize them and print them out but as soon as the block ends (at the '}') those variables stop existing. Your music playing code just plays the global variables over and over.

The idea to use a list of pointers is a good one. I think this will do it:

int speakerPin = 13;
int led10 = 12;

int tempo = 150; //originally 300
int songs = 2; //number of songs available
int songsRand = 0;  //random songs, starting with test #0

const char *song0 = "cdef ";
const byte beats0[] = { 1, 1, 1, 1, 4 };

const char *song1 = "ccggaagffeeddc ";
const byte beats1[] =  { 1, 1, 1, 1, 1, 1, 2, 1, 1, 1, 1, 1, 1, 2, 4 };

const char *song2 =  "ddaagfedcdefga ddaagfedcdefga avgavCDagfdefgfgavaagfedfedgfgavCDagfed";
const byte beats2[] = { 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 4, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 4, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 4, 2, 2, 4, 2, 2, 2, 2, 2, 2, 4, 1, 1, 2, 4, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 8 };

const char * Songs[] = {song0, song1, song2};

const byte * Beats[] = {beats0, beats1, beats2};

void setup() {
  Serial.begin(9600);
  pinMode(speakerPin, OUTPUT);
  pinMode(led10, OUTPUT);
}

void loop() {
  const char * song = Songs[songsRand];
  const byte * beats = Beats[songsRand];
  int length = strlen(song);
  
  Serial.print("start of void loop, notes to play: ");
  for (int i = 0; i < length; i++) {
    Serial.print(song[i]);
  } 
  Serial.println("."); 
  Serial.println();
  for (int i = 0; i < length; i++) {
    if (song[i] == ' ') {
      delay(beats[i] * tempo); // rest
    } else {
      playNote(song[i], beats[i] * tempo);
    }
    // pause between notes
    delay(tempo / 2);
  }
  
  Serial.println("Time to pick a new song!");
  int songsRand = random(1, songs);
  Serial.print("Random song chosen is:  ");
  Serial.println(songsRand);
} // end of void loop

//***********************************************
//*************** Functions *********************
//***********************************************

void playTone(int tone, int duration) {
  for (long i = 0; i < duration * 1000L; i += tone * 2) {
    digitalWrite(speakerPin, HIGH);
    delayMicroseconds(tone);
    digitalWrite(speakerPin, LOW);
    delayMicroseconds(tone);
  }
}

void playNote(char note, int duration) {
  char names[] = { 'c', 'd', 'e', 'f', 's', 'g', 'a', 'v', 'b', 'C', 'D', 'E' };  //note "s" is a f#, v is a b-flat.
  int tones[] = { 1915, 1700, 1519, 1432, 1352, 1275, 1136, 1073, 1014, 956, 852, 758 };

  // play the tone corresponding to the note name
  for (int i = 0; i < 8; i++) {
    if (names[i] == note) {
      digitalWrite(led10, HIGH);
      playTone(tones[i], duration);
      digitalWrite(led10, LOW);
    }
  }
}

Since the notes and beats are inexorably linked, might want to tie them together in a structure called ‘song’. And then ‘songList’ can be an array of songs:

typedef struct {
  char *notes;
  byte *beats;
} song;

const char notes0 = "cdef ";
const byte beats0[] = { 1, 1, 1, 1, 4 };
const char notes1 = "ccggaagffeeddc ";
const byte beats1[] =  { 1, 1, 1, 1, 1, 1, 2, 1, 1, 1, 1, 1, 1, 2, 4 };
const char notes2 =  "ddaagfedcdefga ddaagfedcdefga avgavCDagfdefgfgavaagfedfedgfgavCDagfed";
const byte beats2[] = { 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 4, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 4, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 4, 2, 2, 4, 2, 2, 2, 2, 2, 2, 4, 1, 1, 2, 4, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 8 };

const song songList[] = {
  {notes0, beats0},
  {notes1, beats1},
  {notes2, beats2}
};

#define NUMBER_OF_SONGS sizeof(songList) / sizeof(song)

Change the ‘for loop’ in ‘playNote()’ function to a ‘switch’ statement and #define the tone for each note.