clearInterval fails if button press happens too quickly in Spaceship2.js

[grgustaf]

If you tap and release the pushbutton in this sample very quickly, you can get the message:native_clear_interval_handler: timer not found

If you do it enough times, you can get to where you have two ore more of the timer functions running at once and competing with each other for control of the LEDs.

[jprestwo]

So I know what is happening but not sure of a way to fix it (in C that is). The GPIO callback is getting called, which starts an interval timer. When the GPIO callback gets called again on release, it clears this timer. But if the GPIO callback gets called in quick succession, it starts the interval timer twice, before it can clear it. This causes the original timer handle to be lost because it is overwritten by the second call. This can be fixed in JS by wrapping the setInterval call with an if statement and adding a variable to hold the last timers value. This way it will only set the interval timer if it has been cleared e.g.

if (last != timer || timer == undefined) {
    // start timer to toggle LED states every 250ms (1/4 second)
    timer = setInterval(function () {
        toggle = !toggle;
        led2.write(toggle);
        led3.write(!toggle);
    }, 250);
    last = timer;
} else {
...
...
    // remove the timer event
    clearInterval(timer);
    last = null;
}

This could be happening because of the way callbacks are prioritized any may be possible to fix in C, we would just have to rethink callbacks. I don't think this is what is happening but I could be wrong, its just a thought. Since callbacks are held in a static array, the time at which a callback is added is directly related to its priority. The higher the callback ID, the further down the list it is to get serviced. When you signal a callback, the servicing function still has to iterate though every callback until if finds one with the "signal" flag set. Perhaps we need to have a servicing queue, that contains callback ID's. This would give each callback equal priority.

[grgustaf]

Hmm, this would mean onchange is getting twice in a row with event.value == true then? That seems like a bug.

[grgustaf]

We ended up coming up with a plan to maintain a "ring buffer" of e.g. 1KB to hold all the pending callback types and their arguments; that way we can have more than one callback of a given type pending, with different arguments.

After testing that though, apparently that didn't solve this problem because we're actually getting multiple calls in a row with the same value from Zephyr. Looking more closely, Zephyr doesn't give us the new value, we have to read it from our callback, so it may be changing during that delay. To fix this we can record the last value we reported and drop duplicates; this is akin to debouncing the signal, and won't cause too many problems.

[jprestwo]

This is fixed by my ring buffer / callback queue change:
#287

This PR is in very early stages. We need to figure out if we want to use both pre/post callbacks AND ring buffer, or just the ring buffer.

Another thing with this issue is that is not entirely related to callbacks. The Zephyr GPIO driver will sometimes trigger twice at the same level when using rising and falling edge detection. This causes the top if block to get called two times in a row, and in turn never clearing the interval. The fix for this is to check if the timer is null, and set it to null after clearing it. This provides the needed synchronization.

[grgustaf]

Doing some more testing, I see with a Grove touch sensor as my button I can't seem to reproduce the problem. With a Grove pushbutton, it's very easy to reproduce. Perhaps the touch sensor is hardware debounced?

[xiaolongx]

This issue cannot reproduce on Arduino 101 with commit 2ff271b.