search

ponty / arduino-rtttl-player

Arduino library to play RTTTL melodies
16 stars 12 forks source link

Bug in use of read_word, as demonstrated by failing Ram.pde #1

Closed cefn closed 11 years ago

cefn commented 11 years ago

The example testing the use of songs stored in Ram (Ram.pde) reveals a bug (which is implicit also in the other examples). In my environment at least, the Ram version of the examples doesn't play correctly, and never could.

That's because the notes array is declared as...

const prog_uint16_t notes[] PROGMEM

...and the read_word function is mistakenly invoked with a boolean which actually records whether the const char *p for the song should be interpreted as progmem or not, not the notes array.

This bug in the logic therefore confuses the reading of the notes[] array if the song is NOT stored in progmem.

The result is that if that the line in the _play(...) function which reads...

_tone(read_word(&notes[(scale - 4) * 12 + note], pgm));

...breaks when pgm is false, or in other words, mistakenly tries to access the PROGMEM notes array as if it was in ordinary RAM, just because pgm is false (the song itself happens to be stored in RAM). This creates junk notes on playback. Changing this line in rtttl.h so it reads...

_tone(read_word(&notes[(scale - 4) * 12 + note], true));

...should fix the problem for all playback types.

cefn commented 11 years ago

In case it's relevant, I've forked the repo and reimplemented the functionality of arduino-rtttl-player in an Object-Oriented sort of a way, as well as replacing the existing _play*() functions with a non-blocking implementation.

You can see the new examples at... https://github.com/cefn/non-blocking-rtttl-arduino/tree/master/rtttl/examples

ponty commented 11 years ago

Sorry for the late answer, thanks for the fix!