pgm_read_ and PROGMEM - unexpected behaviour

Question

I made a program that plays a song (using buzzer) saved in array:

(...)
//function which plays a single note
void playNote(int8 wavelength, int duration) {
    if (wavelength != 1) {
        OCR0A = wavelength; /* set pitch */
        SPEAKER_DDR |= (1 << SPEAKER); /* enable output on speaker */
    }
    while (duration) { /* Variable delay */
        _delay_ms(1);
        duration--;
    }
    SPEAKER_DDR &= ~(1 << SPEAKER); /* turn speaker off */    

//function which plays song from array
void playSong( int8 *song,int tempo){
    int length_16_note = tempo;
    for( int8 i = 0;song[i]) ; i += 2){
        playNote(song[i],song[i+1]*length_16_note);
        _delay_ms(15);
    }
}
int main(void){
//array of macros and lenghts of notes
int8 song1[] = {
    C,4, E1,4, F1,3, E1,3, F1,2, 
    F1,2, F1, 2, B1,2, A1,2, G1,1, F1,2, G1,5,
    G1,4, B1,4, C1,3, F1,3, E1,2,
    B1,2, B1,2, G1,2, B1,2, 
    B1,3, C1,13, 0};
//initialize a timer
initTimer();
//play song
playSong(song1, 150)
}

I omitted some parts of it, but this code works just fine. And now I want to save my song into program memory so I change:

#include <avr/pgmspace.h>
(...)      
int8 song1[] PROGMEM = {...}

void playSong( int8 *song, int tempo){
    int length_16_note = tempo;
    for( int8 i = 0; pgm_read_byte(&(song[i])) ; i += 2){
        playNote(pgm_read_byte(&(song[i])), pgm_read_byte(&(song[i+1]))*length_16_note);
        _delay_ms(15);
    }
}

And when I run that code on Arduino, I get random beeps with random duration (much longer that expected). It looks like pgm_read_byte(&(song[i])) returns random values.

I tried to extract code from function playSong to main in order not to pass an array as an argument to the function and nothing changed. So what is wrong with this code?

Answer 1

I'm not sure this is the reason but my first guess is that you get longer durations than expected because your for loop looks like this:

for( int8 i = 0; pgm_read_byte(&(song[i])) ; i += 2)

This means every time the for loop is executed, the program checks the data in program space, as it is the loop condition.

There is a warning stated in the avr-libc user manual concerning the use of program memory reads in loops and functions, see below.

The macros and functions used to retrieve data from the Program Space have to generate some extra code in order to actually load the data from the Program Space. This incurs some extra overhead in terms of code space (extra opcodes) and execution time. (...) But you should be aware of this so you can minimize the number of calls within a single function that gets the same piece of data from Program Space.

Moreover, you make two additional calls to program flash inside the for loop. All this means, that to change the note, the program needs to wait for 3 byte reads from program memory and an additional 15 ms delay. As a result, the notes are much longer than expected.

I suggest the program should first read the whole song into some kind of a buffer in RAM, and play it afterwards directly from RAM.

Answer 2

Problem solved. The issue here was the fact that array song1 is declared in main() function. I had to declare array as global data before function definitions:

#include (...)
#define (...)

const int8 song1[] PROGMEM = {
C,4, E1,4, F1,3, E1,3, F1,2,
F1,2, F1, 2, B1,2, A1,2, G1,1, F1,2, G1,5,
G1,4, B1,4, C1,3, F1,3, E1,2,
B1,2, B1,2, G1,2, B1,2,
B1,3, C1,13, 0};

void playNote(int8 wavelength, int16 duration){...}
void playSong( int8 *song, int16 tempo){...}
int main(void){...}

It was hard to see the problem in my question above, because I wrote code in misleading way (in second quote I wrote array before function definition - but in original code it was in the same place as before). I'm sorry for that.