New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Does your debounce have an issue with tmr.now() rollover? #2

Open
marcelstoer opened this Issue Nov 16, 2015 · 10 comments

Comments

Projects
None yet
4 participants
@marcelstoer

marcelstoer commented Nov 16, 2015

Your debounce code at https://github.com/hackhitchin/esp8266-co-uk/blob/master/tutorials/introduction-to-gpio-api.md was (indirectly) discussed at http://www.esp8266.com/viewtopic.php?f=24&t=4833&start=5#p29127. It essentially says

tmr.now() rolls over at 2147 seconds - the NodeMCU API page indicates uint31, not uint32, which means it wraps to zero when it reaches the max value for int32. The value (now - last) will be negative when (last) gets really big and (now) wraps around.

And it goes on to propose adding if delta < 0 then delta = delta + 2147483647 end; as a remedy.

-> https://github.com/nodemcu/nodemcu-firmware/wiki/nodemcu_api_en#tmrnow

@paulcuth

This comment has been minimized.

Member

paulcuth commented Nov 17, 2015

Awesome, cheers for the heads-up; I had no idea. I'll get that sorted when as I can (might be a couple of days).

Thanks.

@marcelstoer

This comment has been minimized.

marcelstoer commented Nov 17, 2015

NP.

I'm not too familiar with debounce methods but I didn't initially get yours because you react on the first event and then wait rather than trying to react only once the contact has settled. Anyway, good stuff!

What I've been using for a while is the following:

function start()
    gpio.mode(pin, gpio.INT, gpio.PULLUP)
    gpio.trig(pin, "down", doorLocked)
end
function doorLocked()
    -- doStuffOnLocked()
    tmr.delay(50)                       -- time delay for switch debounce
    gpio.trig(pin, "up", doorUnlocked)  -- change trigger on falling edge
end
function doorUnlocked()
    -- doStuffOnUnlocked()
    tmr.delay(50)
    gpio.trig(pin, "down", doorLocked)  -- trigger on rising edge
end

Works quite reliably and is easy to understand.

@paulcuth

This comment has been minimized.

Member

paulcuth commented Nov 17, 2015

You're quite right; strictly it is a throttle function, rather than debounce. I'll take a look at what's common elsewhere and fall in line.

@ali1234

This comment has been minimized.

ali1234 commented Dec 30, 2015

This happens because nodemcu does a bitwise AND with 0x7fffffff on the system timer:

https://github.com/nodemcu/nodemcu-firmware/blob/master/app/modules/tmr.c#L129

This wouldn't be a problem if nodemcu passed through the real bits, even though Lua ints are signed:

last = 0xffffffff
now = 1
now - last == 1 - 0xffffffff == 1 - (-1) == 2 == correct.

But because of the AND we get instead:

last = 0xffffffff & 0x7fffffff == 0x7fffffff
now = 1
now - last == 1 - 0x7fffffff == 0x80000002 == -2147483646 == wrong.

The easiest, cleanest, and probably most efficient fix is to always AND the result of comparing two tmr.now() values with 0x7fffffff:

diff = bit.band((now - last), 0x7fffffff)
0x80000002 & 0x7fffffff == 2 == correct
@marcelstoer

This comment has been minimized.

marcelstoer commented Dec 30, 2015

Thanks for the valuable feedback!

The easiest, cleanest, and probably most efficient fix is to always AND the result of comparing two tmr.now() values with 0x7fffffff:

Except that you need the NodeMCU bit module which you might otherwise not have included in your firmware.

@ali1234

This comment has been minimized.

ali1234 commented Dec 30, 2015

The default firmware has it, or at least mine does. If you build your own firmware I assume you can just remove the AND?

@marcelstoer

This comment has been minimized.

marcelstoer commented May 16, 2016

Forget the code I posted above at Nov 17, 2015 - nothing I should be proud of. Using tmr.delay() is really a bad idea as it stops the world and the networking stack (and other things) can fall over.
I keep a Gist with an IMHO improved version at https://gist.github.com/marcelstoer/75ba30a4aec56d1b3810

@quozl

This comment has been minimized.

quozl commented Sep 8, 2016

@marcelstoer, actually your Gist at https://gist.github.com/marcelstoer/75ba30a4aec56d1b3810 does suffer from a rare problem; if the button is in the "wrong" state when the timer fires, the next transition will be missed. I've been using the same technique for a while. To see the problem, set the debounceDelay to half a second or so, but press the button for less than that time. The next button press will be ignored.

@marcelstoer

This comment has been minimized.

marcelstoer commented Sep 8, 2016

I prefer this technique which I keep at https://gist.github.com/marcelstoer/59563e791effa4acb65f (including the fix proposed in this issue).

@quozl

This comment has been minimized.

quozl commented Sep 8, 2016

Hmm, thanks.

Although
https://gist.github.com/marcelstoer/59563e791effa4acb65f#file-debounce-with-tmr-lua-L21 could read from the gpio pin a different value than what caused the interrupt, because the read from gpio happens later.

The level value given to the trigger callback may be of use. https://nodemcu.readthedocs.io/en/master/en/modules/gpio/#gpiotrig

level comes from the underlying SDK and NodeMCU just passes it on. Reference: https://github.com/nodemcu/nodemcu-firmware/blob/master/app/modules/gpio.c#L35

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment