code advice on buzz()

I developed an hour chime function (buzz) two years ago for v1 which worked very well. After some firmware upgrades on v1, the chime had sometimes complete runaways and did not stop chiming. I had several attempts with completely different approaches to get rid of it and secured the buzz() in a way that it can only be executed once at a time. Now I tried the event triggered solution below on v2 - and there it is again. Works, works not. Does somebody see my error in the code? From my point of view it should be impossible that the buzz code is executed twice.

function planChime() {
  if (chimeTimer) {
    clearTimeout(chimeTimer);
  }
  chimeTimer = setTimeout(function() {
    chimeTimer = undefined;
    Bangle.buzz().then(() => {
      setTimeout(function(){
         planChime();
      }, 2000);
      });
  }, 3600000 - (Date.now() % 3600000));
}

after a buzzer runaway:

Uncaught Storage Updated!
Interpreter error: [
  "FIFO_FULL"
 ]
New interpreter error: FIFO_FULL
>

Why do you set chimeTimer to undefined in the callback function? Wouldnt that make it so that your clearTimeout is never called? if (chimeTimer) can't be true if chimeTimer is undefined. So seems like you are just getting an infinite recursion

I'm not quite sure I understand what @jgDev is suggesting... But from what I can see your code looks fine.

About the only thing that could cause it is if your planChime function was defined in another function where you then defined chimeTimer I guess you could end up with multiple different chimeTimers

The new Bangle.js v2 firmware also uses a different way of doing the buzz (using interrupts) so it's not prone to the issues 2v09 had where you could call clearInterval() with no arguments and stop the buzz from ending.

Maybe you could try putting some print statements in there? Might give you more of an idea what's going on?

To be exact: The shown code is derived from the Anton clock approach to time updates.

Jep, you're right. Thank you. That's not correct. The undefined would lead to one buzz in the first, two buzzes in the second hour and so on. But this does not explain why this leads to infinite buzzes in the first hour - due to the 2s delay.

It's working if it is changed to:

function planChime() {
  if (chimeTimer) {
    clearTimeout(chimeTimer);
  }
  chimeTimer = setTimeout(function() {
    Bangle.buzz().then(() => {
      setTimeout(function(){
         planChime();
      }, 2000);
      });
  }, 3600000 - (Date.now() % 3600000));
}

Nice. So the infinite buzzing issue is resolved?

Just a note of removing chimeTimer=undefined - chimeTimer will still be set to the ID of the timer that has now finished, and there's a high probability that subsequent timers could be given the same ID and then accidentally cleared by planChime.

... ok - next solution:

function planChime() {
  if (chimeTimer) {
    clearTimeout(chimeTimer);
   chimeTimer = undefined;
  }
  chimeTimer = setTimeout(function() {
    Bangle.buzz().then(() => {
      setTimeout(function(){
         planChime();
      }, 2000);
      });
  }, 3600000 - (Date.now() % 3600000));
}

Yes. Above is another solution, based on what Gordon wrote.

Okay, yeah that makes sense.

I still think there's a problem there. If you run planChime once and wait, then lets say chimeTimer gets set to the number 2. Now the setTimeout callback is called and the timer is finished, but chimeTimer is still set to 2.

Maybe now some other code calls setTimeout and gets a new timer with ID 2.

Next time you call planChime it'll then disable that new (unrelated) timeout - so I feel like the original code was still better.

Yes and it is the case - my next idea is:

function planChime() {
  chimeTimer = setTimeout(function() {
    Bangle.buzz().then(() => {
      setTimeout(()=>{
    clearTimeout(chimeTimer);
   chimeTimer = undefined;
         planChime();
      }, 2000);
      });
  }, 3600000 - (Date.now() % 3600000));
}

I think that's pretty broken too... If you run it twice you get two timeouts. Can you try adding some prints to the original code to see what's going on?

We use that pattern (if (x) clearTimeout(x);x = setTimeout(function() { x=undefined; ... },...)) all over the place so I don't think there's anything specifically wrong with it.

function planChime() {
  if (chimeTimer) {
    clearTimeout(chimeTimer);
  }
  chimeTimer = setTimeout(function() {
    chimeTimer = undefined;
    Bangle.buzz().then(() => {
      setTimeout(function(){
         planChime();
      }, 2000);
      });
  }, 3600000 - (Date.now() % 3600000));
}

I'm still struggeling on this.
Edit: Post deleted - I had another idea, still testing ..

Sorry Gordon that I have to come back with this.

if (x) clearTimeout(x);x = setTimeout(function() { x=undefined; ... },...)

This pattern leaves dangeling handles not only in theory, but also on the watch. clearTimeout is never called. If you have no timeout, then you create one. When the function is executed on time, you delete first your handle to this timer. But when the handle is lost the timeout never gets cleared, because there is no handle to it.

Just a note of removing chimeTimer=undefined - chimeTimer will still be set to the ID
of the timer that has now finished,

Shouldn't this be done by clearTimeout? I feel that the problem is here.

I tried multiple alternative solutions for this. All end up in a sudden infinite buzzing after 10-15 hours and FIFO_FULL. The FIFO_FULL also appears with the Anton clock, which also uses this pattern. I'm a JS developer, but I'm really struggeling to develop a pattern which does work. Any logical correct solutions to the timer problem do not work. If you use apps this will never become visible, because everything is reset with app change.

The strange thing is: All was fine also on v1 until approx a year ago. Did you change the behaviour of timers in the past?

What should work should look like this:

function planChime() {
  if (chimeTimer) {
    clearTimeout(chimeTimer);
  }
  chimeTimer = setTimeout(function() {
    Bangle.buzz().then(() => {
         planChime();
});
  }, 3600000 - (Date.now() % 3600000));
}

or at least

function planChime() {
  if (chimeTimer) {
    clearTimeout(chimeTimer);
   chimeTimer=undefined;
  }
  chimeTimer = setTimeout(function() {
    Bangle.buzz().then(() => {
         planChime();
});
  }, 3600000 - (Date.now() % 3600000));
}

But it does not.

But when the handle is lost the timeout never gets cleared, because there is no handle to it.

This is really not the case... x is just a number, clearing the variable has no effect to clearTimeout since a timeout from setTimeout only runs ONCE. Try it...

• Do reset() on your Bangle to ensure there's nothing running in the background
  
• Run global["\xff"].timers - this lets you see the internal list of timers, and it should be []
  
• Run var x then if (x) clearTimeout(x);x = setTimeout(function() { x=undefined; print("Hello") }, 60000)
  
• Run global["\xff"].timers again - you'll see a timer
  
• Wait 1 minute for the timer to execute and 'Hello' to be written
  
• Run global["\xff"].timers and you'll see there are no longer timers
  
  

Shouldn't this be done by clearTimeout?

Maybe in an ideal world, but it's just the way JS works. Try it on a desktop PC. setTimeout returns an integer, and that integer is never cleared.

I've tried to give you code that works above, what you suggest is broken for the reasons I already said.

Perhaps the issue is something else? You can verify it works - just upload this code:

var chimeTimer;

function planChime() {
  if (chimeTimer)
    clearTimeout(chimeTimer);
  chimeTimer = setTimeout(function() {
    chimeTimer = undefined;
    console.log("Buzz");
    Bangle.buzz().then(() => {
      console.log("Buzz done");
      setTimeout(function(){
         planChime();
      }, 2000);
    });
  }, 60000 - (Date.now() % 60000));
}

planChime();

It'll chime every minute. Now keep calling global["\xff"].timers - you'll see there is only ever one timer active at a time, which is the one for the next minute.

What did change a few months ago was how Bangle.buzz works - but the changes there were precisely to stop things like this from happening - it now uses a hardware timer, so people calling clearTimeout() can't accidentally stop the promise from working.