[Bug?] No sound from buzzer in IntervalTimer callback function

R

raffahacks

Member
I'm not sure if this is a bug of Teensyduino, or if it has already been solved, but on my Teensy 3.1 the tone() function works perfectly, except if it is contained in the IntervalTimer callback (for example, if I tell Teensy to beep every hour). Thanks in advance.
 
Hello,
Thank you for the quick answer, Paul, especially on Sunday! :)

The code itself is quite long (as it is a firmware for a smartwatch), but here's it: https://github.com/cunidev/cuni-os?files=1

I've written just a piece of code that should reproduce the problem, to avoid annoying research on a 80kb+ sketch. Thank you very much!

IntervalTimer interruptTimer;
interruptTimer.begin(interruptLoop,1000000);

void interruptLoop() {
tone(BuzzerPin,toneValue);
delay(1000);
noTone(BuzzerPin,toneValue);
}

(Of course, the buzzer works perfectly outside this function, as in the menus where it is used as a clicker)

Thanks!
 
Yes, this is a full program that reproduces the problem (I have just tested it):

Code: 
IntervalTimer interruptTimer;
const int BUZZER = 21; // set the pin here...

void setup() {
  Serial.begin(9600);
  // buzzer works here!
  interruptLoop();
  interruptLoop();
  interruptLoop();
  interruptLoop();
  delay(2000);
  
  interruptTimer.begin(interruptLoop, 10000);
}

void loop() {


}
void interruptLoop() {
  tone(BUZZER,100);
  delay(300);
  noTone(BUZZER);
  delay(3000);
  Serial.println("yes,this loop is triggered by the interrupt too...");
}
 
Hi!

I did not tried the code but the function interruptLoop seems odd to me. It is to be called every 10000 micro-seconds (10 ms) but its execution takes more than 3300 ms (300 ms from the first delay and 3000 ms from the second one). Besides, according to documentation (https://www.pjrc.com/teensy/td_timing_IntervalTimer.html) it should return true and your function has type void.
 
About the 10000ms, sorry, that is a mistake I did in this sketch (the original code was done to be triggered every second and not to last more than ~100ms to execute), but I think this doesn't change the situation... About returning "true", I didn't know that... But the example sketch on the page you linked uses a void as interrupt trigger, like in my sketch...
Well, the code can be corrected, but I'm quite convinced that the bug remains
 
This is the "corrected" version of the sketch, that I tested personally on my Teensy 3.1. The problem persists, as I expected:


Code: 
IntervalTimer interruptTimer;
const int BUZZER = 21;

void setup() {
  Serial.begin(9600);
  // buzzer works here!
  interruptLoop();
  delay(1000);
  interruptLoop();
  delay(1000);
  interruptLoop();
  delay(1000);
  interruptLoop();
  delay(2000);
  
  interruptTimer.begin(interruptLoop, 2000000);
}

void loop() {


}
void interruptLoop() {
  tone(BUZZER,100);
  delay(500);
  noTone(BUZZER);
  Serial.println("yes,this loop is triggered by the interrupt too...");
}

And I have tried using a boolean instead of a void for the interrupt (if I understand what you meant), but that threw a compiler error: it needs a void to work.
 
Interrupt functions return void - they have nothing to return a value to.

The bool to check is on the init of :: interruptTimer.begin to be sure it was allocated as specified.

from:: IntervalTimer.cpp
// ------------------------------------------------------------
// this function inits and starts the timer, using the specified
// function as a callback and the period provided. must be passed
// the name of a function taking no arguments and returning void.
// make sure this function can complete within the time allowed.
// attempts to allocate a timer using available resources,
// returning true on success or false in case of failure.
// period is specified as number of bus cycles
// ------------------------------------------------------------
bool IntervalTimer::beginCycles(ISR newISR, uint32_t newValue)
Click to expand...

Ideally you should rewrite the code such that the loop() portion does any time intensive code - calling tone() may be included in that - and delay() certainly is.

The code in interruptLoop() should perhaps only set a 'volatile bool isrFlag' to true. The loop() code can check for this each pass and then do the desired actions [tone with delay] and set the isrFlag to false on exit.
 
I second defragster. The code in an interrupt service routine should be short and avoid function calls and delays since these might have an impact on interrupt handling. Best use a volatile global flag to be set during the interrupt, then read it in the loop(), execute your code here and clear the flag.

Code: 
IntervalTimer interruptTimer;
const int BUZZER = 21;
volatile bool buzzRequest = false;

void setup() {
  Serial.begin(9600);
  // buzzer works here!
  interruptLoop();
  delay(1000);
  interruptLoop();
  delay(1000);
  interruptLoop();
  delay(1000);
  interruptLoop();
  delay(2000);
  
  interruptTimer.begin(interruptLoop, 2000000);
}

void loop() {
  if(buzzRequest) {
    tone(BUZZER,100);
    delay(500);
    noTone(BUZZER);
    Serial.println("yes,this loop is triggered by the interrupt too...");
    buzzRequest = false;
  }

}
void interruptLoop() {
  buzzRequest = true;
}
 
Last edited:
Thanks for your reply. But unfortunately using booleans won't probably be possible, as the sketch is a firmware, and the void loop() will be, in fact, abandoned after the very first initialization and the processor will execute several minor loops instead... But it'd be impossible to insert this boolean check in all these loops, I think...
 
I believe the point was to demonstrate whether tone() works from interrupt context. Perhaps a better example might have tried to start and stop the tone on every other interrupt, rather than using a delay.

This is now on my list of stuff to investigate. Realistically, it might be a while until I really look into this.
 
@Theremingenieur Hmm, no, actually it's a smartwatch, but you gave me a great idea on my next project :p
@PaulStoffregen thank you very much! The great support is the reason I'll stick to Teensy for my next projects ;)
 
@raffahacks
The tone() function has a third argument which is duration, if I change your interruptLoop() to be just
tone(BUZZER,100,500);

then sketch seems to work for me. (I used pin 13/LED for the "buzzer")
 
SOLVED:: The tone() cannot start or continue while in the interruptTimer routine [as the tone() does not have higher priority]! Posts #10 and #11 were right on general principle - even if we didn't say exactly why - and even if it took me a long time to see it - made more annoying with the sound of my quiet buzzer!

To demonstrate: Use the interruptLoop() below in post 8 and the tone is continuous once started in setup() until interruptTimer.begin(). Then the tone() plays while the LED is ON and stops when the LED goes off.

Nowhere is the tone turned off, or time limited (param 3) - but the buzzer goes quiet while the LED is off after "digitalWriteFast(LED_BUILTIN, 0 );" and comes on with "digitalWriteFast(LED_BUILTIN, 1 );" as interruptLoop() is exited.

The code shown in post 8 never makes a sound within interruptLoop() because this statement "noTone(BUZZER);" silences it before it could ever heard on leaving the interrupt.

This shows very well how trying to do anything of substance within an interrupt can cause great conundrums and hair loss. Also how the generally bad delay() is particularly confusing and bad in an interrupt routine [in this case the longer the delay() the longer the silence after tone() is turned on!].

Code: 
const int BUZZER = 14; // set the pin here... not LED_BUILTIN

void interruptLoop() {
  digitalWriteFast(LED_BUILTIN, 0 );
  tone(BUZZER, 100);
  delay(500);
  digitalWriteFast(LED_BUILTIN, 1 );
}

NOTE: This works to have a non-stop tone if in setup() you execute this statement: "interruptTimer.priority(250);". Doing that also makes the original post 8 code seem to work as well.
 
Last edited:
defragster said:
SOLVED:: The tone() cannot start or continue while in the interruptTimer routine...
Click to expand...

I'll disagree. As described in post 16, using tone(BUZZER,100,500); as the only statement in interruptLoop() works. I verified with other pins and and logic analyzer. Also loop() is running, as I toggled an LED in loop().

Tone.cpp for teensy 3 also uses the interval timer. If you delay() in the timer ISR (interruptLoop()), you will block the interrupts for Tone. The duration argument allows Tone to run asynchronously and exit interruptLoop(). The duration is implemented in Tone.cpp with a counter.
 
Last edited:
manitou said:
I'll disagree. As described in post 16, using tone(BUZZER,100,500); as the only statement in interruptLoop() works. I verified with other pins and and logic analyzer. Also loop() is running, as I toggled an LED in loop().

Tone.cpp for teensy 3 also uses the interval timer. If you delay() in the timer ISR (interruptLoop()), you will block the interrupts for Tone. The duration argument allows Tone to run asynchronously and exit interruptLoop(). The duration is implemented in Tone.cpp with a counter.
Click to expand...

Try this code please. It will show you post 17 is correct. Delay or any other time spent in a timer interrupt stops the tone - the sound pin will be HIGH or LOW but will not PWM during the interrupt. Exemplified by a slow tone interrupt interrupted by a faster no tone interrupt - without the overt delays in the interrupt it might seem to work better - but the tone will stop for the duration of these interrupts without changing the priority.

Yeah that post added some time to my getting to the problem - it is why I wrote "Nowhere is the tone turned off, or time limited (param 3)". That is a Red Herring.

One timer delays the other - the count should be 8 but you'll see 6. And the tone should be 1 sec , but you get BIPPS of "250ms - delay(175)" while the second timer interrupt code is out of play, and the LED blink is when the code is out of the interrupts.

Code: 
IntervalTimer interruptTimer;
IntervalTimer interruptTimer2;
const int BUZZER = 14; // set the pin here...
volatile int buzzC = 0;

void setup() {
  pinMode(LED_BUILTIN, OUTPUT);
  Serial.begin(9600);  // USB
  while (!Serial && millis() <= 3000) if (!(millis() % 500)) ;
  Serial.println("Hello World! ... ");
  // buzzer works here!
  interruptLoop();
  delay(1000);
  interruptLoop();
  delay(1000);
  interruptLoop();
  delay(1000);
  interruptLoop();
  delay(2000);

  interruptTimer.begin(interruptLoop,  2000000);
  interruptTimer2.begin(interruptLoop2, 250000);
  //  interruptTimer.priority(250);
}

void loop() {
}

void interruptLoop() {
  tone(BUZZER, 100, 1000);
  delay(500);
  Serial.print("\nyes,this loop is triggered by the interrupt too...");
  Serial.println(buzzC);
  buzzC = 0;
}
void interruptLoop2() {
  digitalWriteFast(LED_BUILTIN, 0 );
  buzzC++;
  delay(175);   // Remove this to complete the one second buzzer, Smaller delay is lo0nger tone is longer
  Serial.print("...:");
  Serial.print(buzzC);
  digitalWriteFast(LED_BUILTIN, 1 );
}
 
Last edited:
@defragster, your interruptLoop() has delay() and Serial.print(). I agree that is bad ISR programming and will mess up tone(). I argue that
Code: 
void interruptLoop() {
  tone(BUZZER,100,500);
}
works in raffahacks sketch in post 8.
 
Last edited:
No argument - that appears to and does work 'audibly' [it is a red herring bordering on solving a symptom - not a problem though] - the delay()'s were added purposefully to demonstrate exactly why the posts #1 and #8 code were failing, and by extension why code in the isr() must be well planned and necessary to do then, and not able to be done later as indicated in posts 9 and 11.

That doesn't detract from post 17 though: Any time spent in interrupt code will compromise similar or lower priority interrupts and the system at large or lead to a [Bug?].

Indeed post 19 doesn't demonstrate good usage - with delay() and Serial.print() - it is only there to allow one to visually (prints are missed) and audibly (sound is clipped) see the extent and nature of the problems such code can lead to fighting the symptoms of a known problem.
 
Last edited:
You must log in or register to reply here.
Back
Top