keypad.ShiftRegisterKeys has wrong key_count

[aseanwatson]

CircuitPython version

Adafruit CircuitPython 9.1.2 on 2024-08-22; Raspberry Pi Pico W with rp2040
Board ID:raspberry_pi_pico_w

Code/REPL

import board
import keypad

keys = keypad.ShiftRegisterKeys(
    data               = board.GP0,
    clock              = board.GP1,
    latch              = board.GP2,
    key_count          = 8,
    value_when_pressed = True,
    value_to_latch     = True,
    )

print(f"There are a total of {keys.key_count} keys.")

currentEvent = keypad.Event()

while True:
    # this if could be a while loop; that would let you handle every keypress
    # even if it means you don't call update as frequently
    while keys.events.get_into(currentEvent):
        if currentEvent.pressed:
            # TODO: handle key press
            print(f'key {currentEvent.key_number} was pressed')
        elif currentEvent.released:
            # TODO: handle key release
            print(f'key {currentEvent.key_number} was released')

Behavior

Actual:

code.py output:
There are a total of 17 keys.

It looks like

Expected:

code.py output:
There are a total of 8 keys.

Description

See the original Adafruit Forum Post.

Just trying different values, it seems like the effective value of the key_count parameter is key_count * 2 + 1 (so 8 => 17; 1 => 3)

Additional information

I don't have a good way to step through this in the debugger, but I looked at the source. I don't see an obvious bug.

keypad_shiftregisterkeys_make_new does this:

    mp_arg_parse_all_kw_array(n_args, n_kw, all_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);

to populate args and, specifically, args[ARG_key_count].

Then it does:

    if (mp_obj_is_int(args[ARG_key_count].u_obj)) {
        num_key_counts = 1;
    } 

And this:

    if (mp_obj_is_int(args[ARG_key_count].u_obj)) {
        const size_t key_count = (size_t)mp_arg_validate_int_min(args[ARG_key_count].u_int, 1, MP_QSTR_key_count);
        key_count_array[0] = key_count;
    } 

And finally this:

    common_hal_keypad_shiftregisterkeys_construct(
        self, clock, num_data_pins, data_pins_array, latch, value_to_latch, num_key_counts, key_count_array, value_when_pressed, interval, max_events, debounce_threshold);

common_hal_keypad_shiftregisterkeys_construct does this:

    self->key_counts = (mp_uint_t *)m_malloc(sizeof(mp_uint_t) * num_key_counts);
    self->num_key_counts = num_key_counts;

    // copy to an m_malloc() and on the fly record pin with largest Shift register
    mp_uint_t max = 0;

    for (mp_uint_t i = 0; i < self->num_key_counts; i++) {
        mp_uint_t cnt = key_counts[i];

        if (cnt > max) {
            max = cnt;
        }

        self->key_counts[i] = cnt;
    }

    self->max_key_count = max;

Taken together, I expect self->num_key_counts to be 1 and self->key_counts[0] to be 8 and self->max_key_count to be 8.

But the logic analyzer seems to indicate max_key_count is 17.

[aseanwatson]

For what it's worth, this test also fails the same way:

 import board
 import keypad
 
 keys = keypad.ShiftRegisterKeys(
-    data               = board.GP0,
+    data               = (board.GP0),
     clock              = board.GP1,
     latch              = board.GP2,
-    key_count          = 8,
+    key_count          = (8),
    value_when_pressed = True,
    value_to_latch     = True,
    )

[dhalbert]

+    key_count          = (8),

Try (board.GP0,) and (8,) to make these actual tuples. Just putting them in parentheses without the comma does not make a tuple (a syntactic trap of Python's).

[aseanwatson]

Thanks! I confirmed the workaround works.

[dhalbert]

An aside: (8) is not a tuple; (8,) is a tuple; (8,8) without a trailing comma is also a tuple; (8, 8,) is also a tuple. The trailing comma is ignored, but it convenient when you have tuple members on multiple lines:

t = (
    8,
    9,
    10,
)

[dhalbert]

I have a fix for this in #9719. If you are interested in testing, the build artifacts are here:
https://github.com/adafruit/circuitpython/actions/runs/11337244998?pr=9719

[aseanwatson]

The fix looks good.

• I went to https://github.com/adafruit/circuitpython/actions/runs/11337244998/artifacts/2055704506 and installed the UF2 for my pico_w.
• I confirmed the boot_out.txt was `Adafruit CircuitPython 9.2.0-beta.1-3-gfa7573d9b2 on 2024-10-15; Raspberry Pi Pico W with rp2040
• The print(f"There are a total of {keys.key_count} keys.") now reflects the key_count parameter
• The logic analyzer trace shows the right number of clock pulses

Thank you for the quick turnaround.