brettmclean / pad4pi

Interrupt-based matrix keypad library for Raspberry Pi
GNU Lesser General Public License v3.0
51 stars 19 forks source link

Debouncing or noise problems with a self-made 2x4 layout #7

Closed Wikinaut closed 6 years ago

Wikinaut commented 6 years ago

Thanks for your code, I found it used here http://codelectron.com/how-to-make-a-soundboard-using-raspberry-pi-step-by-step-guide-memebox/ (code: https://github.com/codelectron/Soundboard ).

I noticed that sometimes a wrong key is decoded for one of my 2 x 4 keypads, see image. Each micropad with 4 keys has one "row" and four "columns". Each cable is about 70 cm long; perhaps the debouncing can be fine-tuned? img_20180603_001946

My layout:

# Setup Keypad
KEYPAD = [
        ["1","2","3","4"],
        ["5","6","7","8"]
]

COL_PINS = [11,23,24,25] # BCM numbering
ROW_PINS = [17,27] # BCM numbering
Wikinaut commented 6 years ago

test.py:

from pad4pi import rpi_gpio
import RPi.GPIO as GPIO
import time

def processKey(key):
  print(key)
  return

# Setup Keypad
KEYPAD = [
        ["1","2","3","4"],
        ["5","6","7","8"]
]

COL_PINS = [11,23,24,25] # BCM numbering
ROW_PINS = [17,27] # BCM numbering

factory = rpi_gpio.KeypadFactory()

keypad = factory.create_keypad(keypad=KEYPAD, row_pins=ROW_PINS, col_pins=COL_PINS)
keypad.registerKeyPressHandler(processKey)

while 1:
    time.sleep(10)
keypad.cleanup()

Output, ony one row and one column connected, key 5 connected and pressed

python test.py
/home/pi/.local/lib/python2.7/site-packages/pad4pi/rpi_gpio.py:132: RuntimeWarning: This channel is already in use, continuing anyway.  Use GPIO.setwarnings(False) to disable warnings.
  GPIO.setup(self._col_pins[j], GPIO.OUT)
5
5
6
5
.....
5
5
5
5
8
5
Wikinaut commented 6 years ago

@brettmclean I think the problem may be caused by a mistake in https://github.com/brettmclean/pad4pi/blob/8894a2f84a35ce11c5a61ec827396d4d54b3f526/pad4pi/rpi_gpio.py#L127

The debouncing time is something different than self._key_delay which is setup from key_delay=DEFAULT_KEY_DELAY in init.

Please can you check whether my view is correct (I am not fully sure)?

I used instead

GPIO.add_event_detect(self._row_pins[i], GPIO.FALLING, callback=self._onKeyPress, bouncetime=50)

which apparently solved my problem.

Wikinaut commented 6 years ago

I am going to send a formal pull request later, as I recognized a second problem.

brettmclean commented 6 years ago

Hi @Wikinaut,

I'm sorry to see that you're having a weird experience with this. I am far from a hardware/Raspberry Pi/Python expert, but let's see what we can figure out.

Just so we have the full picture, what model of Raspberry Pi are you using? Just in case it's something specific to your hardware. In #5, we weren't able to figure out the underlying problem but the user found that switching from a Rpi Zero W to RPi 3 resolved the problem he was experiencing.

You mentioned when running test.py that "only one row and column are connected" (i.e. only the row and column that correspond to the 5 key are wired to the RPi). Test.py will still be listening for events on the other pins. If the other row/column pins are not connected to anything, I wonder if they would enter a "floating" state which could cause it to randomly register key presses in those rows and columns as voltage is interpreted as jumping between high and low.

That might explain why you see a 6 and an 8 in the output when you're only pressing the 5 key. If you change COL_PINS and ROW_PINS to each contain only one pin (the row and column pins for the 5 key), do you still get this problem? If you physically connect the rest of the wires to the correct pins, do you still get this problem?

I took a look at #8. I allow users to configure key_delay as a parameter in the Keypad constructor. By setting bouncetime to a constant, I'm concerned that debouncing would not work the way that users expect it to. Is there something I'm missing here about what this change is meant to do?

Your other change in getKey looks like it's a shortcut to break out of the loop early and avoid processing other columns. Is this change part of the solution to your debouncing/noise problem or is it just a performance optimization? I think there would need to be a large number of columns before this optimization makes a notable difference and I do value keeping code simpler.

Please let me know the model of RPi you're using, whether Test.py still produces incorrect values when you reduce the number of pins to the bare minimum, and help me understand the purpose of setting bouncetime to a constant.

Thanks, Brett

Wikinaut commented 6 years ago

Hi @brettmclean , thanks for your swift reply.

I worked the whole night and figured out, that the early-exit (using the break) was the far most effective way to highly (but unfortunately not fully) mitigate the problem. In percent I tend to say that my early-exit decreases debounce errors by a factor of about 100, but leave a few.

I will come back now to the debounce delay: my parameter has an impact of about 5 (it reduces debounce errors to about one fifth when you test debounce delay 10 compared to 20 and 50 and 100, what I tested).

Of course, everyone wants the quickest key-response as possible (lowest debounce delay), so values between 30..50 are fine. This depends on the quality of the switches used, of course, and I am also testing simple uninsulated wire ends as "switches". I changed the code because it looked to me that this parameter was by mistake mixed up with the key-repeat(!)-delay parameter: the time until an automatic key repeat rate is fired ... though I may me wrong.

Please can you check especially this code part again? Perhaps I am wrong.

I have

The 2B is my test target; the 3B showed less debouncing errors - with your software, but I haven't had the time yet to test with my branch.

Wikinaut commented 6 years ago

So, coming back to this part:

You mentioned when running test.py that "only one row and column are connected" (i.e. only the row and column that correspond to the 5 key are wired to the RPi). Test.py will still be listening for events on the other pins.

This is normal in a key matrix, isn't it? So we have to deal with this somehow ⟶

If the other row/column pins are not connected to anything, I wonder if they would enter a "floating" state which could cause it to randomly register key presses in those rows and columns as voltage is interpreted as jumping between high and low.

⟶ this is my current thinking, perhaps all unused lines, not active in the i-th scan have to be tied to low or high.... or the scanning should take place with inverted levels (to compensate for induced noise and floats...) .... do you have an idea? I am a newbie to Raspi hardware, not to hardware as such.

brettmclean commented 6 years ago

Hi @Wikinaut,

The idea about "floating" states was pure speculation on my part. On second look, I'm not as sure that this would actually be an issue because the Python code sets the input pins to use a pull-up resistor, which I believe is supposed to prevent this.

Now that I look it over again, setting bouncetime to _key_delay may not have been necessary given that pad4pi does its own debouncing at the beginning of _onKeyPress.

Given that #5 was resolved by moving to a (presumably) more powerful RPi model and that you had fewer problems after exiting from the column scanning loop early, it looks like keypad behavior can be sensitive to Python script/library performance.

Thank you for your detailed research. Given how starkly your situation improved after making these changes, I'm going to go ahead and accept your pull request #8.

Thank you for your contribution, Brett

brettmclean commented 6 years ago

I've released pad4pi 1.1.4 containing your changes.

Wikinaut commented 6 years ago

@brettmclean thanks. I am currently further investigating what can be improved, or what is the underlying problem, and in the Raspberry-Pi-Book https://kofler.info/old/raspberry-pi/ , Kofler wrote on page 724 regarding RPi.GPIO module:

"Unfortunatley, [the] bouncetime [parameter] has been found very [sic] unreliable in our tests."

Wikinaut commented 6 years ago

So I will try to write a bounce-handler inside your module.

Wikinaut commented 6 years ago

Here's the link to the underlying rpio.gpio library: https://github.com/metachris/RPIO (just for completeness). RPIO is a GPIO toolbox for the Raspberry Pi. https://pypi.python.org/pypi/RPIO https://sourceforge.net/projects/raspberry-gpio-python/

Kofler later in his book abandoned the use of the RPi.GPIO library - because of the bad event handling - and uses pigpio http://abyz.me.uk/rpi/pigpio/ instead.

updated

Wikinaut commented 6 years ago

I was wrong.

CaptClaude commented 6 years ago

It's unfortunate that metachris/RPIO is no longer maintained. I thank you for pointing me to that library though, I have used pad4pi in a couple of projects and in one I still have some lingering errors I haven't been able to nail down. I don't think it's a debouncing issue as the keypad is a high quality one in a Polycom conferencing phone, but it is 5x4 and the PiZW is also running Flask and managing a bunch of audio with pygame. Improving the reliability and speed of the keypad handling with a better GPIO library would improve the performance.