Improvement to non-blocking speaker

[jbrazio]

Non-blocking speaker now uses Arduino's tone() function.

Here is a comparison between all the possible three impementations for speaker.

• Blocking
• Non blocking with custom oscillator (RCBugfix)
• Non blocking using tone() (this PR)

https://www.youtube.com/watch?v=tsKZx0vHJ2o

[Blue-Marlin]

That's exactly what i feared. You are occupying a timer and an interrupt.

[Blue-Marlin]

A struct is not an enum.

[jbrazio]

I didn't want to create a new tone_t.h.

[jbrazio]

@Blue-Marlin for your pleasure: https://www.youtube.com/watch?v=tsKZx0vHJ2o

[jbrazio]

You are occupying a timer and an interrupt.

You're wrong, tone() only uses TIMER2_COMPA_vect.
Knock yourself out reading the code.

What is the difference between a timer and an interrupt you may ask..

• A timer is a counter, when it overflows a function is called.
• A interrupt is an event handler which calls a function when some state changes.

[Blue-Marlin]

The event is the timer overflow.
http://www.nongnu.org/avr-libc/user-manual/group__avr__interrupts.html
Read the manuals.

[jbrazio]

Exactly, that's what I wrote.

[Blue-Marlin]

No. You claimed a timer interrupt not to be an interrupt.
Does it break the normal program flow or not.
Is the 'function' called via the interrupt vector table, or not.

[Blue-Marlin]

Let's leave that. It's a bit unproductive. There are bigger problems we should care about.

No tone() please - is all what i want for now.

[jbrazio]

No. You claimed a timer interrupt not to be an interrupt.

No I claimed this:

A timer is a counter, when it overflows a function is called.
A interrupt is an event handler which calls a function when some state changes.

[thinkyhead]

What is the difference between a timer and an interrupt…?

The first is an interrupt based on a counter (overflow, apparently), while the second is every type of interrupt (including timer interrupts). All salmon are fish, but not all fish are salmon. All "timers" (in this sense) are interrupts, but not all interrupts are timers.

An interrupt is an event handler which calls a function when some state changes.

Such as a counter overflow or a pin state change. When it's a counter overflow, we call it a "timer." They both interrupt the flow of execution.

(Although I suppose it's possible with some CPUs to have a counter overflow just set a bit that you can check later, and not call any interrupt.)

[thinkyhead]

millis_t would also be appropriate here.

[thinkyhead]

No tone() please - is all what i want for now.

If tone() is a lesser option that few will want to use, then certainly. At least, if it taxes the CPU too much, it should be discouraged.

[jbrazio]

The starting point for the discussion was @Blue-Marlin's

You are occupying a timer and an interrupt.

Which implies tone() is using a timer AND an interrupt thus it is a wrong affirmation, they are conceptually and practically different concepts. I never said they are not both interrupts.

Let's be pragmatic and down to earth: It does not fucking matter.

The main objective of the buzzer is to emit sound feedback when people are navigating on the menu system; some people might want it to buzz when their print finish, others would prefer to play the Pokemon theme instead.

Why it doesn't matter are you still asking ?

Because the Pokemon theme will be played when the printer is idle, not when it's printing.
Who gives a fuck if a timer is running then ?

The best thing the printer has to do is to play the fucking Pokemon theme as it as been requested to do.
It's only a question of being pragmatic, understanding the real world application and usage cases before pissing their pants off.. scared.

I don't care if we use the exiting code or the tone() one, what I care is that both of them sound better than the blocking code (it turns out people have a small memory span). Once again, listen and take your own conclusions and choose which should be shipped with Marlin.

[Blue-Marlin]

Tone() was in the sound code at least twice. It was removed always after a short time, because of various problems. Our issue history is so full of buzzer, beeper, speaker, tone(), lcd.buzz(), lcd_buzz(), buzzer.h problems, that i currently can't find the relevant ones. Butt during the last 18 month always someone from KiteLab was involved. I'm soooooooooooo tired about it.

[jbrazio]

Please, let me show you why tone() was rejected:
//tone(BEEPER_PIN, freq, duration); // needs a PWMable pin

[Blue-Marlin]

The beginning was #3913
Just to remember:

It's not a big nor a feature.. who want a buzzer buzzing for 2S ?..

.

After looking into it, I have a way to improve it.. We could refactor the code into an object™ and get rid of all the delay() stuff by adding a tick() and some glue-logic behind it.
But (is) it worth the effort ?

Clear answers:
No. And it wont work.

[jbrazio]

Once again, listen and take your own conclusions.

[thinkyhead]

Once again, listen and take your own conclusions.

I'm happy with a tone that sounds better, and you're right that we don't care about long tones or long queues of tones playing during a print. And non-blocking is of course preferred in any case. Mainly I hope we make a positive impression on users, because in this instance user perception is what matters.