search

ms-iot / lightning

MIT License
16 stars 20 forks source link

DebounceTimeout has no effect #44

Closed siochs closed 6 years ago

siochs commented 7 years ago

Hi, from my point of view, the current behaviour of debouncing is incorrect: When attaching a switch to ground with Pullup to Vcc and pressing the button, the ValueChangedEvent is fired each time the state is flapping (in my test scenario it is about 20 times per button press). Please refer to this thread: https://social.msdn.microsoft.com/Forums/en-US/dcb31681-9eda-4c97-9fd6-c5a7e94df147/gpio-input-pin-debouce-timeout-being-ignored?forum=WindowsIoT

I think, debouncing should filter the flapping from the input and firing the event only, when the state change (e.g. High -> Low) is still present after a certain time (DebounceTimeout). The following line in your code, I think, is not working correctly:

in Providers/GpioDeviceProvider.h line 127: if (eventTimeDiff >= pin->_DebounceTimeout.Duration || (eventTimeDiff > 0 && InfoPtr->NewState != pin->_lastEventState))

The first part of the if statement is okay. Second part: (eventTimeDiff > 0) will be almost always be true because flapping takes places in timeranges of a few milliseconds. (NewState != lastState) will also be true when an input flaps between High and Low. As a result, a "high frequency" bouncing pin will result in many event callbacks. I think, the line should be replaced by this one: if (eventTimeDiff >= pin->_DebounceTimeout.Duration || pin->_DebounceTimeout.Duration == 0) If no debounce Timeout is set, then the user is not interested in debouncing so the event can be fired very time the pin changes it state. Otherwise a debounce timeout should be passed before firing the event.

I tried this code in my scenario with a debounce timeout of 50ms and this was working like a charm.

MahmoudGSaleh commented 7 years ago

@siochs Thanks for the feedback (and apologies for the delay). Agree, this seems to be the right fix. I'll incorporate your feedback in a new bug for the debounce behavior.

Qowy commented 7 years ago

Any update on this? I've run into the same issue. Should we create a pull request?

MahmoudGSaleh commented 7 years ago

Yes, please feel free to create a PR.

MahmoudGSaleh commented 6 years ago

fixed now #50