-2

I am using Javascript (no JQuery) and have a situation I'm trying to debug.

I'm using setInterval with a 1 second interval to monitor physical hardware that is being instructed to move. Once the hardware has moved to the correct position I call clearInterval with the ID returned from the setInterval call.

Everything works as anticipated - UNLESS... the hardware is not working properly.

When the hardware is not working properly, my first attempt to move the hardware returns an error (which is the way it should work).

When I receive an error, I want to stop the Interval by calling clearInterval.

However, what I'm seeing is this...

I call setInterval - which immediately fires off my first request to move the hardware which (in this case) returns an error (can not move hardware) very quickly.

It appears that the error is being returned faster the setInterval stores the Interval ID into the resulting variable. Thus, my call to clearInterval fails because the ID is zero, not the proper ID.

I've verified this... and indeed there is a significant delay on the setInterval call, to when it returns with the ID. So something like this is happening:

  • I call setInterval and expect an ID back
  • setInterval calls my function which tries to move the hardware
  • Hardware move fails, my function tries to call clearInterval, but the original setInterval has not yet returned the proper ID
  • clearInterval fails as the ID is zero, not the proper one
  • my function continues to be called

I've tried to clearInterval all the values between 0 and 100 (as suggested in other posts) - but that does not seem to stop the timer (the ID I'm getting back is almost always 2, so I'm unclear why it won't clear it).

What would be the suggested method for ensuring I get the ID back in a timely fashion?

Note... not using (and can not due to requirement) JQuery. Just pure javascript.

Many thanks!

Edit: Adding code...

Globally declared:

var drawIntervalID = 0;

Here is my code that sets the Interval:

drawIntervalID = setInterval(masks_animate,1000);

The masks_animate function is a simple straight-forward function that simply makes some decisions and calls other functions. Specifically it is calling moveCarousel to actually move the hardware. The moveCarousel is also the function trying to clear the interval if something goes wrong:

// This function will move the carousel
function moveCarousel(mask) {
   var res,i,ss;
   setLO("","#000000");
   if (training_mode) {
      alert("Attempt to move Carousel in Training Mode ignored");
   } else {
      res = sendRequest("MOVE:"+mask);
      i   = res.indexOf('OK:');
      if (i >= 0) {
          ss  = res.slice(i);
          res = ss.split(":");
          if (res.length == 2) {
              res[1]  = res[1].replace(/(\r\n|\n|\r)/gm,"");
              setLO(res[1],"#00ff00");
          } else {
              alert("Carousel MOVE request returned bad data ("+ss+")");
              clearTimer();
              masksDisplay();
          }
      } else {
          i = res.indexOf('ERR:');
          if (i >= 0) {
              ss  = res.slice(i);
              res = ss.split(":");
              if (res.length == 2) {
                  res[1]  = res[1].replace(/(\r\n|\n|\r)/gm,"");
                  setLO(res[1],"#ff0000");
              } else alert("Carousel MOVE request returned bad data ("+ss+")");
           } else alert("Carousel MOVE request returned bad data ("+res+")");
           clearTimer();
           masks_moving = 0;
           masksDisplay();
       }
   }
   return false;
}

The thing to look at is the lower case where I'm looking for "ERR:" (which is an error condition from the hardware. In that case I call clearTimer() which looks like this:

// This function will clear the timer
function clearTimer() {
    if (drawIntervalID != 0) {
        clearInterval(drawIntervalID);
        drawIntervalID = 0;
    } else {
        for (var i=0; i<100; i++) clearInterval(i);
        drawIntervalID = 0;
    }
}

Like I said - it all works perfectly except in the situation where the hardware immediately returns an error, in which case my drawIntervalID is not set yet.

EDIT: I've modified the code to set the initial drawIntervalID to NULL (not zero) and am checking for NULL. No difference in operation.

David Cook
  • 15
  • 5
  • 2
    WHERE is the code? – epascarello Apr 07 '16 at 19:30
  • Not possible. `setInterval` immediately returns the ID. It can't do anything else until it does. Show some code. – Matt Burland Apr 07 '16 at 19:32
  • @MattBurland `setInterval` won't immediately return anything if the application experiences an unhandled exception during the evaluation of `setInterval`. I suspect the OP is invoking his callback, rather than referencing it. – Scott Marcus Apr 07 '16 at 19:42
  • @ScottMarcus: Well, sure. If stuff breaks it breaks. – Matt Burland Apr 07 '16 at 19:44
  • @MattBurland Well, that's what the OP says is happening - - an immediate error with no return value. So no, not impossible. – Scott Marcus Apr 07 '16 at 19:45
  • @ScottMarcus: If you make an error in the line of code that has `setInterval` then yeah. But the I assume from the description that the *interval was set* or why would the OP be so concerned about clearing it? This is all wild speculation anyway because *the OP didn't include any code*. Which make it impossible to answer. – Matt Burland Apr 07 '16 at 19:47
  • I've added the code portions to the original post. – David Cook Apr 07 '16 at 19:48
  • `drawIntervalID` should be initialized to `null`, not `0` so that it doesn't get confused with an actual timer ID. – Scott Marcus Apr 07 '16 at 19:49
  • Where is this line `drawIntervalID = setInterval(masks_animate,1000);` in relation to everything else? Where is `moveCarosel` called? – Matt Burland Apr 07 '16 at 19:50
  • @ScottMarcus: That'll irrelevant. Although it might be cleaner. The clearing all intervals for 0 to 100 is just silly however. – Matt Burland Apr 07 '16 at 19:51
  • @MattBurland I think it is relevant because `0` could be a valid timer ID and the OP's `clearTimer` function is explicitly looking for `0` as an indicator of "no timer set". It's not a good idea and is best to remove it from the equation. – Scott Marcus Apr 07 '16 at 19:57
  • @DavidCook Where is that code? – Scott Marcus Apr 07 '16 at 19:58
  • Since your `clearTImer()` call, in the `else` (error) branch of your `if` is, itself, not conditional, have you tried moving it to the top of that branch? Have you tried logging `drawIntervalID ` at various points in your progression? – Scott Marcus Apr 07 '16 at 20:05

1 Answers1

0

You appear to be over managing and incorrectly intializing the timer ID in your code. And, since you are having a timer problem, this would be a logical place to start.

Just initialize your timer variable to null (not 0 or any other integer) and call clearInterval() directly (rather than passing control to your clearTimer function) when needed. There is no need for your clearTimer() function as it isn't providing any functionality that is actually useful to you - - it's just more code that is complicating matters.

It seems like you may be a victim of optimizing your code before you have working code.

Scott Marcus
  • 64,069
  • 6
  • 49
  • 71
  • I've added code to my original post. I'm calling it the same way you do in your second example. – David Cook Apr 07 '16 at 19:50
  • While the `clearTimer()` is unnecessary. It's certainly not the cause of the OP's problem. Also you can just `clearInterval` on any value and it won't cause an error if the timer either doesn't exist or wasn't running. So a simple `clearInterval(drawIntervalID)` should work even if `drawIntervalID` is `0` or `null` – Matt Burland Apr 07 '16 at 19:58
  • You are missing the point. A client could return `0` as the value of a timer and the OP is supposing that `0` means no timer. Why include this extra code that could be causing the issue when it provides no benefit? – Scott Marcus Apr 07 '16 at 20:00