Possible bug in tone() for ATTiny1604

[hamsterdave]

It seems there's a bug in the tone() function on the ATTiny1604 (possibly others, I don't have any others handy to test on at the moment). If you call the tone function with a duration specified:

tone(TONE_PIN, 440, 500);

It doesn't actually generate a tone output, it just toggles the specified pin high for the specified duration and then low again.

If you call the function with no duration, it generates the tone as it should, however if you then call noTone(); it stops generating the tone output, but it leaves the pin high, pumping DC into the speaker coil. The pin has to be manually set low again following noTone();

[SpenceKonde]

Issue created - described behavior is clearly wrong. I never use tone myself so it;s not surprising that a bug with certain applications of tone() got into the core.

[SpenceKonde]

Somehow I missed this over in discussions, Issue link is #369

[SpenceKonde]

Thanks for reporting this. I've fixed a few of the issues, issues with it leaving pin high; I don't think my fixes would have produced weird behavior though. The fix is currently in github master branch (2.3.0-dev) and can be installed via manual installation; it is not in a boards manager release).

[SpenceKonde]

Current changelog entry has grown to:

Correct large number of tone() bugs. 
Tone could leave the pin high. 
Frequency of 0 would leave the timer running and generating interrupts in the background for the requested duration while doing nothing.
(now it stops output if it's on the current pin, otherwise does nothing).
No attempt was made to handle invalid pins: it would get null pointer to port struct.... and proceed to use it without testing if it was valid, or if the bitmask was valid. 
Now we test for valid bitmask before requesting the port struct. 
Frequencies over 32768 would overflow an intermediate when called with duration and the resulting tone would be very short. 
Long durations can also overflow the intermediate, particularly at high frequency. 
The intermediate is found as `2 * frequency duration` and both multiplications can overflow (first one is as unsigned int, second as signed long.) 
Since the result is divided by 1000, remove 2* and change to /500 doubled the limit and removed the problem above 32768 Hz. 
If SUPPORT_LONG_TONES is defined as 1, then we will take additional steps to support tones longer than a minute; otherwise, when (frequency * duration) > 2^32, it'll overflow, and duration will be way short. 
It adds 106 bytes to the compiled binary - right now I've got it enabled by default for parts with 16k of flash and up.
Fixed at least two cases of variable that should be volatile not being volatile, and one of the opposite. 
Halved the time that interrupts have to be disabled for, and skip recalculating thing

It has clearly been many years since someone properly worked over that code.