nodemcu / nodemcu-firmware

Lua based interactive firmware for ESP8266, ESP8285 and ESP32
https://nodemcu.readthedocs.io
MIT License
7.67k stars 3.13k forks source link

gpio module looses key presses #330

Closed gorootde closed 8 years ago

gorootde commented 9 years ago

Hi,

simple hardware setup: pin 3 <--> pusbutton (open if not pressed) <--> GND combined with the code

gpio.mode(3,gpio.INPUT,gpio.PULLUP)
gpio.trig(3,"down",function(level)
        local state=gpio.read(3)
        print(state,level)
end)

seems to loose some key presses. Continuously pressing the key may result in 1 or 0 printed as last message. I would expect that the last line always says 1 when nobody is pressing the button, and 0 as long as it is pressed.

Seems like some events get lost, maybe the callback mechanism does not return fast enough to catch the next event?

devsaurus commented 8 years ago

Please re-try with a recent firmware build (based on SDK 1.4.0) and report the results here. Otherwise it's going to be closed as per #719.

TerryE commented 8 years ago

I would expect that the last line always says 1 when nobody is pressing the button, and 0 as long as it is pressed.

Not correct. gpio.read(3) is synchronous. Callback delivery is aysnch. If a gpio pin is pulsed high then this will raise an interrupt and result in the Lua callback being scheduled. However, it is quite possible that by the time that the callback is executed, the signal has already returned to low.

gorootde commented 8 years ago

Tested it with the latest development firmware. The issue persists! Code to reproduce:

gpio.mode(3,gpio.INPUT,gpio.PULLUP)
gpio.trig(3,"both",function(level)
        print(level)
end)

@TerryE As you stated, it is quite clear that state maybe differs from level. But at least the last level value has to be 1, which is not the case after multiple button presses.

Could you please reopen this issue?

gorootde commented 8 years ago

Here is my workaround until this issue is fixed (also debounces the key):

gpio.mode(3,gpio.INT,gpio.PULLUP)
gpio.trig(3,"both",function(level)
    tmr.stop(3)
    tmr.alarm(3,10,0,function() --debounce 10ms
        print("gpio.read(3))  --use gpio.read, as 'level' does currently not provide the correct pin state
    end)
end)
TerryE commented 8 years ago

@kwave, The 'symptoms' won't have changed with the new release because we haven't changed this code. As I said, this is a known feature of the GPIO. We are in the process of reworking the GPIO logic, so can we please and evaluate the new code. There is no point in re-opening an issue which is already covered by other open issues.

gorootde commented 8 years ago

I was not talking about the gpio.read. Or does the level parameter implicitly also use this function? Would you mind posting the issue numbers of the issues that cover this issue? I'd like to track the progress to be always up to date.

TerryE commented 8 years ago

@kwave. the github help gives you ample explanation an how to find these. I have a life to lead, and would prefer to do this, rather that doing more work so that you have more time to live yours.

jmattsson commented 8 years ago

You're using a mechanical switch without debounce logic. That one is in your court.

Looking at the gpio code it appears that NodeMCU is sitting on top of a stack of SDK functions which wouldn't be improving the likelyhood of consistency. Seeing as the ESP supports ANYEDGE, it's not possible to synthesize the level from the interrupt type, which otherwise would have guaranteed consistency. What this sounds like to me is that NodeMCU should simply not provide the level argument, document the need to debounce (or derive the level from the trigger type if not using ANYEDGE mode), and leave it to the user.

nickandrew commented 8 years ago

'level' is set by reading the GPIO pin sometime after the interrupt occurred, and the GPIO state may have changed in the meantime. That doesn't make it a bug, but does mean it's not possible to determine exactly why an interrupt was triggered, with the arguments provided to the function.

If the pin_int_type[i] argument were added to the function call, then the function would know why it was called (1 for "up", 2 for "down", 3 for "both" i.e. ANYEDGE etc) and in the case of ANYEDGE, the former constraint applies, that 'level' shows what the pin state was at sometime after the interrupt occurring.

gorootde commented 8 years ago

@nickandrew that's exactly what I also would expect the level parameter to be. "The pin state at the time the interrupt was triggered". Even the documentation does not clearly define what the level parameter is.

nickandrew commented 8 years ago

@kwave No, that's not what I said. I said "sometime after the interrupt occurred", not simultaneously with the interrupt.

Since the documentation is unclear, I edited the wiki to point out that the pin is read "soon after" the interrupt is triggered.