Memcopy_P / retrieving array with different size

Hi. I'm programming an Arduino UNO to control a series of 6 trainhorns. The goal is to play different tunes on the trainhorns, by controlling a relay-board that connects to the airvalves of the horns. The different tunes can be selected with a 4x4 matrix keyboard, and some feedback is presented on an LCD.

The songs are stored in arrays, and are multi-tone - each note is played by a up to 6 horns at the same time. A second array stores the length that each note should be played. New as I am to Arduino (and very novice to programming) I soon hit the memory barrier. Then I learned about PROGMEM.

What I'm walking into: I've got a buffer-array SongN[6][40] (and SongL[40] for the corresponding note-lengths). I've got songs in arrays of different sizes but never more than [6][40] (6 rows since I only have 6 horns and thus will never play more horns simultaneously - "6 trainhorns should be enough for anybody" ).

Now when i use memcpy_P(SongN, Song1N, sizeof(Song1N)) I get an incorrect copy of the array, although parts of it are correct (the first byte of each row).

Brief version of code, stripped for the purpose of my question:

#include <avr/pgmspace.h>
int SongN[6][40] = {};                        // Array to store the current playing song - the number of rows must equal rowCount
int SongL[40] = {};                           // Array to store the current playing song's timing (the amount of time that each note should play)

//----------------------------------------------
//SONGS TO PLAY
//----------------------------------------------
//Song 1 = Test song multi-toned
int const Song1C = 13; // Length of the song will never change
int const Song1N[rowCount][Song1C] PROGMEM = { // Write the song to flash-memory instead of SRAM
  {6, 0, 5, 0, 4, 0, 3, 0, 2, 0, 1, 0, 1},
  {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2},
  {4, 4, 4, 4, 4, 4, 4, 0, 0, 0, 0, 0, 3},
  {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4},
  {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5},
  {1, 0, 2, 0, 3, 0, 4, 0, 5, 0, 6, 0, 6}
};
int const Song1L[] PROGMEM = {250, 250, 250, 250, 250, 250, 250, 250, 750, 250, 750, 250, 1000}; // Write the timing to flash-memory

void setup() {

  Serial.begin(9600); // For debugging purposes

}

void loop() {

  if (key == '1') {                                     // copy song 1 to the play-arrays
    noteCount = Song1C;
    memcpy_P(SongN, Song1N, sizeof(Song1N)); // Copy the array to play from flash to SRAM array
    memcpy_P(SongL, Song1L, sizeof(Song1L)); // Do the same for the corresponding note-length-array

Your array in PROGMEM has a song length of 13 whereas your array in RAM has a song length of 40. You are doing a memory copy and since the dimensions don't match it doesn't line up in RAM.

You can accomplish what you want by performing the copy this way:

  for (int i=0; i<6; i++)
  {
	memcpy_P(SongN[i], Song1N[i], sizeof(Song1N[0]));
  }

I think you should store the length of the note in the same variable as where you store the 'note' . If you have only 6 note types that easily fits into have a byte, and 16 different note lengths is more than enough for nearly every song. Have a look at ATtiny 13 melody Player Yours being multi-timbral you would need to have it millis() based (which you probably already have) and the easiest way to get the timing correct would to be to 'add' a 'timing-note' that sets the time until another note starts, using the same format as for a normal note.

Yours being multi-timbral you would need to have it millis() based (which you probably already have)

Actually.. No, there is not even a need for it since you don't have to generate the sound waves.

Certainly using int (16 bit) for values which are only going to be in range 0 through 6 seems very wasteful.
Try uint8_t and halve your memory footprint right off the bat.

Also I’d question if it is necessary to copy the whole array into RAM at all. If you are playing the notes individually, just read them one at a time direct from PROGMEM as required.

Certainly using int (16 bit) for values which are only going to be in range 0 through 6 seems very wasteful.
Try uint8_t and halve your memory footprint right off the bat.

Even that seems excessive, i think note length can be stored in that same byte, or you could consider doing a 'note-on' 'note-off' approach, (as midi does) and and just keep track of intervals between events.

If you want to copy the variable-length songs into a fixed-length array you could rearrange the arrays to be one row per note:

const byte Song1N[Song1C][rowCount] PROGMEM = { // Write the song to flash-memory instead of SRAM
  {6,0,4,0,0,1},
  {0,0,4,0,0,0},
  {5,0,4,0,0,2},
  {0,0,4,0,0,0},
  {4,0,4,0,0,3},
  {0,0,4,0,0,0},
  {3,0,4,0,0,4},
  {0,0,0,0,0,0},
  {2,0,0,0,0,5},
  {0,0,0,0,0,0},
  {1,0,0,0,0,6},
  {0,0,0,0,0,0},
  {1,2,3,4,5,6}};

ToddL1962:
Your array in PROGMEM has a song length of 13 whereas your array in RAM has a song length of 40. You are doing a memory copy and since the dimensions don't match it doesn't line up in RAM.

You can accomplish what you want by performing the copy this way:

  for (int i=0; i<6; i++)

{
memcpy_P(SongN[i], Song1N[i], sizeof(Song1N[0]));
 }

Thnx - applied your fixed (need to think of these as arrays-in-arrays, I guess), worked like a charm. The rewrote the whole thing since it is indeed a waste of memory to use a 6x40 array when I can manage with just 1x40 - 1 byte is more than enough to store the on/off state of 6 horns. The resulting 2 bits are not enough though to store the timing so for now I'll keep the 2nd array for timing, for ease of use.

The footprint is back to 822 bytes (40%), which is great! One problem though is that writing a song in bit-code is not very human-friendly... I guess I'll make some simple excel-sheet to convert human-readable notes to their bit-variant... or does anyone have a nice suggestion?

Btw, future-wishes are implementing bluetooth-control, and triggering a 2nd arduino to control the lights of the car in sync with the trainhorns.

The resulting code so far:

int const Horns[] = {13, 14, 15, 16, 17, 10, 11};   // array of pins to which horns are attached. Horns[0] is the builtin led, used for debugging / pauses (blank notes)

byte const hornCount = 7; // number of horns, inlcuding the builtin led
byte const maxLength = 40; // The maximum notes a song should get - is used to define max array size

int noteCount = -1; // the number of notes to play (i.e. the length of the song-array to play)

byte SongN[maxLength] = {}; // Array to store the current playing song
int SongL[40] = {}; // Array to store the current playing song's timing (the amount of time that each note should play)

int currentHorn; // holds the horn to play while playing
int unsigned long currentLength; // holds the length that the currentHorn should play

int unsigned long currentMillis; // will store the actual time in millis()
int unsigned long startMillis; // Will store the time the timerloop is started in millis()

int abortDetect = 0; // Variable that stores wether the abort-key has been pressed

//----------------------------------------------
//SONGS TO PLAY
//----------------------------------------------
//Song 1 = Test song
int const Song1C = 13; // Length of the song will never change
byte const Song1N[maxLength] PROGMEM = {1, 0, 2, 0, 3, 0, 4, 0, 5, 0, 6, 0, 6}; // Write the song to flash
int const Song1L[] PROGMEM = {250, 250, 250, 250, 250, 250, 250, 250, 750, 250, 750, 250, 1000}; // Write the timing to flash

//----------------------------------------------
//Song 2 = Test song
int const Song2C = 11;
byte const Song2N[maxLength] PROGMEM =  {6, 5, 4, 3, 2, 1, 2, 3, 4, 5, 6};
int const Song2L[] PROGMEM = {500, 500, 500, 500, 500, 500, 500, 500, 500, 500, 1000};
//----------------------------------------------

void setup() {

  Serial.begin(9600); // For debugging purposes

  // Initiate the LCD:
  lcd.init();
  lcd.backlight();

  for (int thisPin = 0; thisPin < hornCount; thisPin++) { // Set the pins to which the relays for the horns are attached to OUTPUT
    pinMode(Horns[thisPin], OUTPUT);
  }
}

void loop() {
  lcd.setCursor(0, 0);
  lcd.print("Press a key...");
  char key = keypad.getKey();
  // just print the pressed key
  if (key) {
    Serial.println(key);
    lcd.clear();
    lcd.print("Key pressed: ");
    lcd.print(key);
  }

  noteCount = -1; // reset to a zero-length song

  if (key == '1') {                                     // copy song 1 to the play-arrays
    noteCount = Song1C;
    lcd.setCursor(0, 1);
    lcd.print("Notes: ");
    lcd.print(noteCount);

    memcpy_P(SongN, Song1N, sizeof(Song1N)); // Copy the array to play from flash to SRAM array
    memcpy_P(SongL, Song1L, sizeof(Song1L)); // Do the same for the corresponding note-length-array, but simpler since it's one-dimensional

  }

  else if (key == '2') {                                // copy song 2 to the play-arrays
    noteCount = Song2C;
    lcd.setCursor(0, 1);
    lcd.print("Notes: ");
    lcd.print(noteCount);
    memcpy_P(SongN, Song2N, sizeof(Song2N));
    memcpy_P(SongL, Song2L, sizeof(Song2L));
  }


  // play time - play the song that has just been copied

  for (int thisNote = 0; thisNote <= noteCount; thisNote++) { // For each note to play (ie for each column)

    currentLength = SongL[thisNote]; // get the length that the current note (column) should play

      Serial.print(SongN[thisNote]); //Debugging: Print the note that is played
      Serial.print("(");

  if (SongN[thisNote] & 0b0000000) {digitalWrite(Horns[0], HIGH); Serial.print("0");}
  if (SongN[thisNote] & 0b0000001) {digitalWrite(Horns[1], HIGH);Serial.print("1");}
  if (SongN[thisNote] & 0b0000010) {digitalWrite(Horns[2], HIGH);Serial.print("2");}
  if (SongN[thisNote] & 0b0000100) {digitalWrite(Horns[3], HIGH);Serial.print("3");}
  if (SongN[thisNote] & 0b0001000) {digitalWrite(Horns[4], HIGH);Serial.print("4");}
  if (SongN[thisNote] & 0b0010000) {digitalWrite(Horns[5], HIGH);Serial.print("5");}
  if (SongN[thisNote] & 0b0100000) {digitalWrite(Horns[6], HIGH);Serial.print("6");}
  Serial.print(")");

    Serial.print(" with delay:"); //Debugging: Print the amount of msec that they will play
    Serial.print(currentLength);
    Serial.println(" msec");

    abortDetect = 0; // Reset the abortvariable to 0
    // Store the time in millis() when we entered the timingloop

    startMillis = millis(); // Store the time when the timerloop is entered
    currentMillis = millis(); // Store the current time

    while (currentMillis - startMillis < currentLength) { // Timerloop - loop while current time minus starting time is shorter than the time the note should play
      char key = keypad.getKey(); //check if the song should be aborted
      if (key == '*') { // If STAR is pressed, we're aborting
        Serial.println("ABORT! ABORT!");
        abortDetect = 1;
        lcd.clear();
        lcd.print("ABORT SONG"); // Acknowledge the abort
        delay(1500); // Give some time to let it sink in
        startMillis = startMillis - currentLength; // End the timerloop
      } // End of key-reading

      currentMillis = millis();

    } // End of the timerloop

  if (SongN[thisNote] & 0b0000000) {digitalWrite(Horns[0], LOW);}
  if (SongN[thisNote] & 0b0000001) {digitalWrite(Horns[1], LOW);}
  if (SongN[thisNote] & 0b0000010) {digitalWrite(Horns[2], LOW);}
  if (SongN[thisNote] & 0b0000100) {digitalWrite(Horns[3], LOW);}
  if (SongN[thisNote] & 0b0001000) {digitalWrite(Horns[4], LOW);}
  if (SongN[thisNote] & 0b0010000) {digitalWrite(Horns[5], LOW);}
  if (SongN[thisNote] & 0b0100000) {digitalWrite(Horns[6], LOW);}
  
    if (abortDetect == 1) {
      thisNote = noteCount; // Fake that we've played all notes. 
    }
    if (thisNote == noteCount) {
      lcd.clear(); // Clear the LCD after playing the song
    }
  }
}

I'm wondering though: would there be an easy way of entering a song bit-wise, in such way that I can see what I'm doing when writing it down, ie make it 'readable' for humans too?

  • 1 byte is more than enough to store the on/off state of 6 horns. The resulting 2 bits are not enough though to store the timing so for now I'll keep the 2nd array for timing, for ease of use.

Honestly 6 types of notes use 3 bits, the remaining 5 are easily enough for timing since nearly all melodies use quarter notes or half or double of that (or double and a half etc. there is no real need to store the timing in ms. But you have so much space it doesn't matter all that much.

The footprint is back to 822 bytes (40%), which is great!

I thought you wanted to use PROGMEM ? pcbbc is correct !

I guess I'll make some simple excel-sheet to convert human-readable notes to their bit-variant... or does anyone have a nice suggestion?

do you have a DAW ? any midi-editor will do Ohm studio is free for use, but the midi editor is not my favorite (still)

I'm wondering though: would there be an easy way of entering a song bit-wise, in such way that I can see what I'm doing when writing it down, ie make it 'readable' for humans too?

It helps a lot already if you store the values in a visible way in your case as it is now0b01000 for horn nr. 4 (or 5) mind you in that case it would be easier if a note would just occupy 3 bits and they look like '1' & '3' mind you in that way a chord would take up more space.
Anyway, if you take the trouble to have a look at what i suggested in #2 that used up a bit over 800 bytes for the Marseillaise in total in flash (on an Attiny13, which has almost no bootloader, but still)
Anyway without to much effort i think you can make it more readable to use the note (or horn-bit) as you have it now, and turn on bit 7 to indicate a timing bit and use the other 7 bits to show how long that should be, that way you can have a single array (const progmen) read a byte , if it is a note change the relays to the horns, if it is a 'delay' convert that value to ms and so on. And rather tha n having a maximum size just use a NULL terminator.

Deva_Rishi:
Honestly 6 types of notes use 3 bits, the remaining 5 are easily enough for timing since nearly all melodies use quarter notes or half or double of that (or double and a half etc. there is no real need to store the timing in ms. But you have so much space it doesn't matter all that much. I thought you wanted to use PROGMEM ? pcbbc is correct !do you have a DAW ? any midi-editor will do Ohm studio is free for use, but the midi editor is not my favorite (still)It helps a lot already if you store the values in a visible way in your case as it is now0b01000 for horn nr. 4 (or 5) mind you in that case it would be easier if a note would just occupy 3 bits and they look like '1' & '3' mind you in that way a chord would take up more space.
Anyway, if you take the trouble to have a look at what i suggested in #2 that used up a bit over 800 bytes for the Marseillaise in total in flash (on an Attiny13, which has almost no bootloader, but still)
Anyway without to much effort i think you can make it more readable to use the note (or horn-bit) as you have it now, and turn on bit 7 to indicate a timing bit and use the other 7 bits to show how long that should be, that way you can have a single array (const progmen) read a byte , if it is a note change the relays to the horns, if it is a 'delay' convert that value to ms and so on. And rather tha n having a maximum size just use a NULL terminator.

I should have used 'chords' as a term: my notes are multi-horn tunes. So a single note can consist of sounding horn 1, 2 and 3 together. Thus my possible notes are 65432*1=720 possibilities - definitely not fitting into 3 bits unless using the Sloot Digital Coding System :wink:
Also, you might imagine these tunes are pretty rudimentary: they are to be played during a car rally, and are ment to resemble famous tunes. I'll check into the DAW-route if free time allows me to. This setup however, now controlling trainhorns, will also be used to control car lights. I might need the flexibility in timing then.

Concerning the NULL terminator - will do that, all, bytes are valuable. For now I'm in the comfort zone again concerning free memory. Fun to learn al these good practices by making mistakes!

[offtopic] Found myself googling what a 'pcbbs' is :smiley:

Why even bother copying it to RAM? Just read the notes as you need them with pgm_read_byte_near()...

Thus my possible notes are 65432*1=720 possibilities

Eh !!! nope, i mean at the moment you store them in 6 bits, so 64 possibilities. My train of thought was to actually see them as 6 possible individual notes, with a 5 bit duration, but i think now that a 1 byte 'chord' followed by a 1 byte interval (or 7 bit) interval would be the most efficient. a chord can the be represented by the 6 bits as they are now, but including bit 7 'on' , If bit 7 is 'off' it can identify an interval, which leaves room for a 'null' terminator (intervals are never zero), and you even have room for a bpm change / set using both bits 6 & 7 'on' . Anyway like that you can store the whole thing in the same array, and you can make it look sort of like this.
{0b10010011, 30, 0b10001011, 45, 0b10001000, 30, etc}which gives you a more decent visual representation of what is going on.

Deva_Rishi:
Eh !!! nope, i mean at the moment you store them in 6 bits, so 64 possibilities. My train of thought was to actually see them as 6 possible individual notes, with a 5 bit duration, but i think now that a 1 byte 'chord' followed by a 1 byte interval (or 7 bit) interval would be the most efficient. a chord can the be represented by the 6 bits as they are now, but including bit 7 'on' , If bit 7 is 'off' it can identify an interval, which leaves room for a 'null' terminator (intervals are never zero), and you even have room for a bpm change / set using both bits 6 & 7 'on' . Anyway like that you can store the whole thing in the same array, and you can make it look sort of like this.

{0b10010011, 30, 0b10001011, 45, 0b10001000, 30, etc}

which gives you a more decent visual representation of what is going on.

Basic math has been a while back... definitely 64 yeah [oops].

Writing it down like you did indeed gives a nice visual representation, definitely do-able for writing tunes 'on the fly'. Thnx!

Why even bother copying it to RAM? Just read the notes as you need them with pgm_read_byte_near()...

Why bother? "because you only just told me about pgm_read_byte_near() just now" :slight_smile:
Last night I was a bit triggerhappy when I learnt about PROGMEM, and thus immediately rewrote the whole thing to copy to flash and then back to RAM. Only to learn now that I can read bytes at a time. Back to the drawing table!

because you only just told me about pgm_read_byte_near() just now

If you would have investigated the link i gave you in #2 (to the melody player) you would have found it. Check the final code on page 2 for additional ideas.

Deva_Rishi:
If you would have investigated the link i gave you in #2 (to the melody player) you would have found it. Check the final code on page 2 for additional ideas.

No harm ment, I was being sarcastic about my own ignorance. Actually did read your link. I've got maybe 24 hours of combined codingexperience in C - reading someone elses code during eveninghours after an intensive day of work might not go as naturally as you might think :slight_smile: Easy to miss stuff...
Really thanx for the pointers though!
Also got interested in exploring struct - several Arduino-players use it for more complex note-coding. Right now, I'm thinking of staying on the less efficient way of using arrays like I'm already doing, but combining it with using pgm_read_byte_near(). Flash isn't going to be an issue, and RAM usage stays below 50% this way, while mantaining the possivility to easily upscale the program to control 12 lights instead of 6 trainhorns, for example.