RTTTL fixed

[mgesteiro]

Adding a different startup sound to the Escornabot I discovered that the RTTTL implementation was faulty:

• note duration or octave were not being parsed (missing ' ')
• an octave goes by C C# D D# E F F# G G# A A# B ("A, A# and B at the end)
• reordered and reduced frequencies: only four octaves required (4 to 7)
• note to frequency mismatch
• BPM/duration calculation
• some ringtones (e.g. there is no B#)

TODO: extend the RTTTL parsing routine to implement the missing tone and a half (".") duration.

Surprisingly, the parsing/playing functions worked well-enough to keep in hiding for all these years... 🤪

With this pull-request the biggest issues should be fixed, and now you can hear the real tones: yes, the Escornabot sounds different now!

[rafacouto]

Why did you reduce octaves? Original values are covering audible freqs by humans (despite the qualitiy of the speaker that usually is mounted on hardware). Flash memory is quite free at this version with the minimal required MCU (avr328p).

New melodies are welcome, however the startup melody is not a RTTTL feature. It originally was a simple beep to indicate powering fact. Startup melody was discarded because it produced a crazy brownout effect when batteries are low.

BPS were used instead BPM because it simplifies the operations at machine code (1 divisions against 3). The trade-off is a bit complex understanding of the RTTTL spec (admissible?).

Thanks for your time contributing the Escornabot :-)

[mgesteiro]

Thanks for the comments 🙂
Here some more:

Why did you reduce octaves?

because of the RTTTL specification: https://www.mobilefish.com/tutorials/rtttl/rtttl_quickguide_specification.html (that is mentioned on the wikipedia itself)

Original values are covering audible freqs by humans

lower frequencies than 440 Hz (the minimum value in the original code) are audible: nine notes (C4 to G#4) were missing (as per the specification). Also, depending on the literature (or who you ask to) the commonly accepted maximum frequency is 20 kHz, where the original code goes 4 additional values (21096, 22350, 23679, 25087).

Startup melody was discarded because it produced a crazy brownout effect when batteries are low.

In my experience, if the batteries are so low:

1. the "bad sound" is a good indicator of that issue (and is nice to know)
2. the steppers would produce the same problem (aka the Escornabot is unusable anyway)

BPS were used instead BPM because it simplifies the operations at machine code (1 divisions against 3). The trade-off is a bit complex understanding of the RTTTL spec (admissible?).

the code doesn't reflect this: there is a direct conversion from the "b" in the RTTTL string to the "bps" variable where the "b" represents BPM (as per the original spec).

I haven't checked that optimization, but it doesn't seem to work: would you care to explain it?
In any case, isn't it a bit excessive?

Thanks for your time contributing the Escornabot :-)

When I patched this code, I did it for me, but also I did it following these criteria:

1. Don't break anything (unless completely necessary)
2. As it says "RTTTL", I stuck to the spec wherever possible (you won't find [many] ringtones out of it anyway)
3. It should "do the job", i.e., should be able to play RTTTL ringtones/melodies correctly (that now does)
4. Respect the original code/idea/structure as much as possible (didn't rebuild the whole thing)
5. KISS and clarity before performance

That being said, please tell me what corrections would you require to accept the pull-request and I'll do my best to fulfill them: the ultimate goal is to help contributing, not just discussing details 😅😅.

In any case, again, thanks for the time and all the effort put into the project.

[mgesteiro]

any update on this?

[rafacouto]

any update on this?

At first glance, reducing the octaves is not a good idea since some melodies could use them. A more exhaustive review should check limit of user values in order to become a more robust code.

I'm waiting to find some moment to review and test the changes in deep. Please, be patient 😉