search

adafruit / circuitpython

CircuitPython - a Python implementation for teaching coding with microcontrollers
https://circuitpython.org
Other
4.04k stars 1.19k forks source link

keypad.KeyMatrix does not properly support col_pin->switch->diode->row_pin #6401

Closed slashfoo closed 2 years ago

slashfoo commented 2 years ago

CircuitPython version

Adafruit CircuitPython 7.2.5 on 2022-04-06; Adafruit KB2040 with rp2040
Board ID:adafruit_kb2040

Code/REPL

import board
import collections
import neopixel
import time

from digitalio import DigitalInOut, Direction, Pull

ROW_PINS = [
    board.D5,
    board.A0,
    board.D6,
    board.SCK,
    board.D8,
    board.MOSI,
    board.D9,
    board.D10,
]

COL_PINS = [
    board.D2,
    board.A3,
    board.D3,
    board.A2,
    board.D4,
    board.A1,
]

if __name__ == '__main__':
    print("Doing setup")
    pixels = neopixel.NeoPixel(board.NEOPIXEL, 1)
    for _ in range(5):
        pixels.fill((0, 0, 6))
        time.sleep(0.1)
        pixels.fill((6, 0, 0))
        time.sleep(0.1)
    pixels.fill((0, 0, 0))

    import keypad

    km = keypad.KeyMatrix(list(ROW_PINS), list(COL_PINS),
                        columns_to_anodes = True,
                        interval = 0.2,
                        max_events = 64)

    print("Entering main loop")

    while True:
        pixels.fill((0, 6, 0))
        num_trig = 0

        event = km.events.get()
        while event is not None:
            num_trig += 1
            print(event)
            event = km.events.get()
            if not event: break
        # print('-')

Behavior

expected keystrokes are sent, but also undesired keystrokes are sent. The undesired keystrokes are always on the same column next row over.

Description

While browsing the code at https://github.com/adafruit/circuitpython/blob/main/shared-module/keypad/KeyMatrix.c#L126-L165, I saw that the loop modifies High/Low value of the row_dio, as well as the Input/Output setting for it. It doesn't touch the columns. This aligns with the fact that transposing the matrix appears to solve the issue.

I don't know how the code works in the original setup I have since the columns in my design are the ones that need to be outputs, and the rows are supposed to be the inputs. But there is no place in the code that looks like it'd support my use case.

Additional information

Setting up the KeyMatrix transposed appears to eliminate the problem, like so:

    km = keypad.KeyMatrix(list(COL_PINS), list(ROW_PINS),
                        columns_to_anodes = False,
                        interval = 0.2,
                        max_events = 64)

In the workaround above, row_pins is set to COL_PINS, and col_pins is set to ROW_PINS; the variable columns_to_anodes is set to False.

dhalbert commented 2 years ago

Could you post a schematic of your board? The definition of "row" and "column" is arbitrary, and the diode placement could be done either way. I tried to follow what I thought were typical examples. It sounds like some more documentation would be helpful.

slashfoo commented 2 years ago

@dhalbert I didn't have a schematic handy, but hopefully I'm able to make things clearer with a bit more descriptions below.

The board I'm using is the Adafruit kb2040 (based on the rp2040). The issue was discovered using kmk, which uses keypad.KeyMatrix. In the process of troubleshooting I tested KMK code, keypad.KeyMatrix code, and then created my own matrix scanning loop. This was done in the process of isolating the issue in software after I verified that my connections were all solid, that there were no unintentional connections, and that all diodes were in the correct orientation.

I initially asked on discord for KMK with my troubleshooting process and code examples in this gist: https://gist.github.com/slashfoo/2bdf703615d2279f3d7058f7bda4d34f ; if you're interested in additional examples of the code I used while investigating.

Board pinout is here: Adafruit KB2040 > Pinouts

My matrix is set up as follows:

Pins:

Connections:

The design/concept models GroupA as rows, and GroupB as columns. But as you mentioned, these are arbitrary. This is only one of two ways of modeling the circuits.

Conceptual models of the keyboard to write code

  1. GroupB as columns, GroupA as rows. With this definition, the diode direction is columns_to_anodes = True. In KMK this is called COL2ROW.
  2. GroupA as columns, GroupB as rows. With this definition, my circuit's diode direction is columns_to_anodes = False. In KMK, this would be ROW2COL.

From the documentation of keypad.KeyMatrix, I understand that the intent is to support column (diode)->| row as well as row (diode)->| column with columns_to_anodes being True, and False respectively. But in practice, only Model 2 is supported, because when modeled as Model 1 above, the code misbehaves.

I hope this makes my context clearer, please let me know if it's still confusing and I'll try to ellaborate further.

There's also the possibility that I'm missing something, are there other parameters that need to be changed besides columns_to_anodes in order to make these models give the same results and hence be interchangeable?

dhalbert commented 2 years ago

The intent of columns_to_anodes was whether the columns were connected to the anodes or the cathodes of the diodes, e.g. whether the diodes were flipped. But I see you could also interpret it as having the diodes on the rows rather than the columns.

Did you model your keyboard wiring on an existing schematic? Any example schematic that is wired the same way would be helpful for me to look at. Or even a picture, if it's point-to-point wiring. Thanks!

slashfoo commented 2 years ago

@dhalbert yes, I understand that the variable means if anodes or cathodes are econnected to the columns.

The way I interpret it is that when that value is true, columns are essentially outputs and when scanning the columns go high. When the variable is false, the columns are inputs, and it's the rows that are driven. I.E. columns_to_anodes = True means that conventional current goes from column pins to row pins; when set to false, the conventional current goes from the row pins to the column pins.

My matrix is in two PCBs from a kit that doesn't, to my knowledge, have schematics publicly available. I reproduced the connection in a diagram below. I hope the diagram makes it clearer.

D2              A3              D3              A2              D4              A1
*               *               *               *               *               *
│               │               │               │               │               │
├─/─>|┐         ├─/─>|┐         ├─/─>|┐         ├─/─>|┐         ├─/─>|┐         ├─/─>|┐
│     └───────────────┴───────────────┴───────────────┴───────────────┴───────────────┴──────────* D5
│               │               │               │               │               │
├─/─>|┐         ├─/─>|┐         ├─/─>|┐         ├─/─>|┐         ├─/─>|┐         ├─/─>|┐
│     └───────────────┴───────────────┴───────────────┴───────────────┴───────────────┴──────────* A0
│               │               │               │               │               │
├─/─>|┐         ├─/─>|┐         ├─/─>|┐         ├─/─>|┐         ├─/─>|┐         ├─/─>|┐
│     └───────────────┴───────────────┴───────────────┴───────────────┴───────────────┴──────────* D6
│               │               │               │               │               │
├─/─>|┐         ├─/─>|┐         ├─/─>|┐         ├─/─>|┐         ├─/─>|┐         ├─/─>|┐
│     └───────────────┴───────────────┴───────────────┴───────────────┴───────────────┴──────────* SCK
│               │               │               │               │               │
├─/─>|┐         ├─/─>|┐         ├─/─>|┐         ├─/─>|┐         ├─/─>|┐         ├─/─>|┐
│     └───────────────┴───────────────┴───────────────┴───────────────┴───────────────┴──────────* D8
│               │               │               │               │               │
├─/─>|┐         ├─/─>|┐         ├─/─>|┐         ├─/─>|┐         ├─/─>|┐         ├─/─>|┐
│     └───────────────┴───────────────┴───────────────┴───────────────┴───────────────┴──────────* MOSI
│               │               │               │               │               │
├─/─>|┐         ├─/─>|┐         ├─/─>|┐         ├─/─>|┐         ├─/─>|┐         ├─/─>|┐
│     └───────────────┴───────────────┴───────────────┴───────────────┴───────────────┴──────────* D9
│               │               │               │               │               │
└─/─>|┐         └─/─>|┐         └─/─>|┐         └─/─>|┐         └─/─>|┐         └─/─>|┐
      └───────────────┴───────────────┴───────────────┴───────────────┴───────────────┴──────────* D10

Fill in the gaps on the vertical lines as if they were under (or over) the horizontal lines without electrical connection. The / are the switches, and ─>|┐ are diodes w/ cathode to the right.

ladyada commented 2 years ago

hmm, it really shouldnt matter? all diode matrices wire the same way.... didya try maybe swapping cols/rows passed in?

slashfoo commented 2 years ago

@ladyada, I did swap cols<->rows. If I swap those (and also change the columns_to_anodes accordingly) the buggy behavior goes away.

The problem I'm trying to solve is that (to me) it seems like the code does not work if the output pins of the matrix are passed in as columns. The code appears to not be symmetrical, even though the documentation seems to indicate that it's a supported case.

I wrote my own python matrix scanner, and don't see the buggy behavior either.

I think if the problem is in the code, as is my hypothesis, it should be easy to replicate. I'll assemble another matrix to attempt to reproduce the issue as well, I have a few other keyboards that have socketed pro-micros, and a spare kb2040 that I can use to test. One such kb is my daily driver running QMK, i.e. known-good matrix.

This should give a bit more light, if the issue is able to be replicated on a different kb2040, and different matrix, it would add a bit more weight to the idea that the code may be lacking symmetry. Another idea I had is that there may be a problem with reading the pins too fast, but I haven't work on exploring that route.

I'll report back after the next experiment.

deshipu commented 2 years ago

If you swap the rows and columns, the key matrix is electrically the same as if you just flipped all the diodes to the opposite direction. There is nothing that the code could do to recognize the difference (and work in one case while not working in the other), both cases are electrically identical. It would really help to see the schematic of your matrix.

Also, what does it mean the code "does not work"? Do you get an error message? Do wrong keys register? No key presses register at all? Everything bursts into flames?

dhalbert commented 2 years ago

Could you post a link to the kit, and if they would help, photos of the board(s)?

dhalbert commented 2 years ago

The interval you are using is 0.2 secs or 200 msecs. That is a very long scanning time. 0.02 is the default. The errors you are seeing are:

expected keystrokes are sent, but also undesired keystrokes are sent. The undesired keystrokes are always on the same column next row over.

The code is just looking for a completed circuit between a row and a column pin, It could do that in either direction. I'm not sure why it should matter which is the input and which is the output.

Are there capacitors on the row or column lines on this board?

dhalbert commented 2 years ago

I am not saying you haven't detected a problem. But the lack of symmetry in the code is deliberate: I compensated by reversing the detection direction. It was smaller this way.

I am wiring up a 2x2 array of keyswitches with external diodes and connections so I can test it in either direction.

dhalbert commented 2 years ago

I made a small 2x2 test setup of four https://www.adafruit.com/product/4978, NeoKey breakout. On these boards, the A connection is the anode, and the C connection is the cathode. Here is the schematic. I only used the SWITCHA and SWITCHC pins.

neokey

I wired the SWITCHAs together as thecolumns, and the SWITCHCs together as the rows. I believe this corresponds exactly to your ASCII schematic above.

Here is my test program, running on a QT Py RP2040:

import board
import keypad

# left to right
COLS = (board.A0, board.A1)
# top to bottom
ROWS = (board.A2, board.A3)

km = keypad.KeyMatrix(row_pins=ROWS, column_pins=COLS,
                      columns_to_anodes=True)

while True:
    event = km.events.get()
    if event:
        print(event)

I do not see false events (wrong column, as you described) with this test program.

And finally, here is a photo of the wiring. The Anode pins are 2nd from the left on each breakout; the Cathode pins are the leftmost pins. The upper and lower rows of pins on each breakout are identical. IMG_2397

slashfoo commented 2 years ago

I tried the code on a different matrix that is wired in the same manner that I know to work, and the undesired behavior is not present.

@dhalbert thank you for looking into this and for putting time into attempting to replicate the problem. I think that by having the same piece of code work on the known-good matrix, I can safely assume that the code for which I opened this github issue is not the culprit.

To answer your previous question, there are no capacitors on the board.

I managed to get the schematic from the designer of the board, but I don't think I should continue my troubleshooting on this thread.

I'll close this issue and continue my troubleshooting out of band. Thanks again for the time and help with the investigation.

dhalbert commented 2 years ago

Thank you for your troubleshooting patience. If you figure out what is different about that board, let us know. I wondered if perhaps one of the diodes was backwards or shorted, but I'm not sure that would cause the problems you saw.

deshipu commented 2 years ago

A friend of mine had a similar problem, and we got it down to the diodes he used — instead of fast signal diodes like the 1N4148, he used some random rectifier diodes, that turned out to be too slow to switch properly during scanning. Replacing them with signal diodes fixed the problem.

slashfoo commented 2 years ago

@deshipu That could be the case, I'm going to try changing the diodes to a known part. This may align with the fact that I have the same kit, working properly with a Pro-Micro, but that one works at 16MHz rather than 120MHz, which may also align with the loop/transposition workaround.

When you mentioned that the circuits transposed were electrically the same, I recognized that it was the same intuition I had when trying it, the difference is in running the code, the issue with multiple keystrokes being sent appears when the pins being driven high/low are in the inner/tight loop, which I can only assume happens faster than the outer one.

Assuming that the diode switching speed is the problem, adding a delay or underclocking the microcontroller could also be workarounds. And it is an easy thing to test next.

@dhalbert I will definitely report back when i learn what the mystery is.