Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Contradiction in the RTTTL specification #10

Closed
StarsoftAnalysis opened this issue Jan 13, 2021 · 2 comments · Fixed by #11
Closed

Contradiction in the RTTTL specification #10

StarsoftAnalysis opened this issue Jan 13, 2021 · 2 comments · Fixed by #11
Labels
enhancement New feature or request

Comments

@StarsoftAnalysis
Copy link
Contributor

I've been struggling to get a tune to sound right, and eventually realised that NonBlockingRTTTL doesn't parse RTTTL in the way I expected.

The problem occurs with dotted notes that are not in the default octave, which I thought should be written as a4. but this library expects a.4

The confusion seems to arise because the 'language' is poorly specified. I don't know if there's a canonical definition anywhere, but I found a Backus-Naur specification which includes:

<note> := [<duration>] <note> [<scale>] [<special-duration>] <delimiter>

That puts the dot (the 'special-duration') at the end. But the same document includes g.6 in its example, cleverly contradicting itself.

Wikipedia says 'Dotted rhythm patterns can be formed by appending a period (".") character to the end of a duration/beat/octave element.'

I suggest that NonBlockingRTTTL should allow for either dot position -- simply by repeating the short bit of code that consumes the dot and extends the note's duration after the section that deals with the octave. That would avoid breaking anyone's existing tunes, and make it easier for people to get new tunes to work.

Cheers,

Chris

@end2endzone
Copy link
Owner

end2endzone commented Jan 13, 2021

Hi.
Thank you for you interest in NonBlockingRTTTL.

You are right, the library is expecting notes in a4. format as showed in the arkanoid melody from the examples with the note 16g.6

I can't remember where I took the specification for the RTTTL format. Looking at my original articles on my blog about AnyRtttl and NonBlockingRTTTL libraries, I only mentioned the Wikipedia article which refers to this specification. Looking at the specification, I do remember the awkward yellow background.

If I understand your proposition, you are suggesting to copy lines 212 to 217 to line 231. I see no problem with this and I think it would make the application "more robust" (in a way).

Its been a while since I have worked with my arduino. It must be lying on a shelve somewhere and quite dusty now. In other words, to test the change, I would have to rebuild my arduino setup to work on this. Since you proposed this change, I would be happy if you would create a pull request and contribute to the project. Is that something that is appealing to you? I know the change is relatively small and simple but like I said, I currently have no way to verify if the change is working as expected. If you do, please add a comment in the code that refers this issue so that people know why we are parsing the . twice.

If you do not want to create a pull request, that is also fine. I can also commit the change myself but I would need your help to validate if the change is working as expected. Your call.

@StarsoftAnalysis
Copy link
Contributor Author

OK, I'll do some more testing and create a pull request.

@end2endzone end2endzone linked a pull request Jan 15, 2021 that will close this issue
end2endzone added a commit that referenced this issue Jan 15, 2021
Resolve issue #10 -- allow 'dot' to be after octave number
end2endzone added a commit that referenced this issue Jan 15, 2021
Updated CHANGES and AUTHORS for issue #10.
end2endzone added a commit that referenced this issue Jan 15, 2021
@end2endzone end2endzone added the enhancement New feature or request label Jan 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants