Skip to content

Extended ShiftRegisterKeys to support multiple data pins with shared clock and latch #8143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jul 19, 2023
Merged

Extended ShiftRegisterKeys to support multiple data pins with shared clock and latch #8143

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5fcd90d
Extended to support multiple data pins
Jul 6, 2023
4ba4c2a
Renamed 'datas' to 'data_pins' to keep codespell happy
Jul 6, 2023
6dab35e
Assign key_count to MP_ROM_NONE on deinit (for gc)
Jul 8, 2023
21d0864
updated documentation
Jul 10, 2023
2860593
fixed missing comma
kolkmvd Jul 10, 2023
f8edecf
corrected formatting
Jul 10, 2023
0c606c5
Documentation: Sequences are supported, not just Lists
Jul 11, 2023
bdf9336
Improved help text
Jul 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 54 additions & 9 deletions shared-bindings/keypad/ShiftRegisterKeys.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@
//| self,
//| *,
//| clock: microcontroller.Pin,
//| data: microcontroller.Pin,
//| data: Union[microcontroller.Pin, Sequence[microcontroller.Pin]],
//| latch: microcontroller.Pin,
//| value_to_latch: bool = True,
//| key_count: int,
//| key_count: Union[int, Sequence[int]],
//| value_when_pressed: bool,
//| interval: float = 0.020,
//| max_events: int = 64
Expand All @@ -53,23 +53,25 @@
//| Create a `Keys` object that will scan keys attached to a parallel-in serial-out shift register
//| like the 74HC165 or CD4021.
//| Note that you may chain shift registers to load in as many values as you need.
//| Furthermore, you can put multiple shift registers in parallel and share clock and latch.
//|
//| Key number 0 is the first (or more properly, the zero-th) bit read. In the
//| 74HC165, this bit is labeled ``Q7``. Key number 1 will be the value of ``Q6``, etc.
//| With multiple data pins, key numbers of the next pin are sequentially to the current pin.
//|
//| An `EventQueue` is created when this object is created and is available in the `events` attribute.
//|
//| :param microcontroller.Pin clock: The shift register clock pin.
//| The shift register should clock on a low-to-high transition.
//| :param microcontroller.Pin data: the incoming shift register data pin
//| :param Union[microcontroller.Pin, Sequence[microcontroller.Pin]] data: the incoming shift register data pin(s)
//| :param microcontroller.Pin latch:
//| Pin used to latch parallel data going into the shift register.
//| :param bool value_to_latch: Pin state to latch data being read.
//| ``True`` if the data is latched when ``latch`` goes high
//| ``False`` if the data is latched when ``latch`` goes low.
//| The default is ``True``, which is how the 74HC165 operates. The CD4021 latch is the opposite.
//| Once the data is latched, it will be shifted out by toggling the clock pin.
//| :param int key_count: number of data lines to clock in
//| :param Union[int, Sequence[int]] key_count: number of data lines to clock in (per data pin)
//| :param bool value_when_pressed: ``True`` if the pin reads high when the key is pressed.
//| ``False`` if the pin reads low (is grounded) when the key is pressed.
//| :param float interval: Scan keys no more often than ``interval`` to allow for debouncing.
Expand All @@ -91,29 +93,72 @@ STATIC mp_obj_t keypad_shiftregisterkeys_make_new(const mp_obj_type_t *type, siz
{ MP_QSTR_data, MP_ARG_KW_ONLY | MP_ARG_REQUIRED | MP_ARG_OBJ },
{ MP_QSTR_latch, MP_ARG_KW_ONLY | MP_ARG_REQUIRED | MP_ARG_OBJ },
{ MP_QSTR_value_to_latch, MP_ARG_KW_ONLY | MP_ARG_BOOL, {.u_bool = true} },
{ MP_QSTR_key_count, MP_ARG_KW_ONLY | MP_ARG_REQUIRED | MP_ARG_INT },
{ MP_QSTR_key_count, MP_ARG_KW_ONLY | MP_ARG_REQUIRED | MP_ARG_OBJ },
{ MP_QSTR_value_when_pressed, MP_ARG_REQUIRED | MP_ARG_KW_ONLY | MP_ARG_BOOL },
{ MP_QSTR_interval, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} },
{ MP_QSTR_max_events, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 64} },
};
mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
mp_arg_parse_all_kw_array(n_args, n_kw, all_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);

size_t num_data_pins;

if (mp_obj_is_type(args[ARG_data].u_obj, &mcu_pin_type)) {
num_data_pins = 1;
} else {
num_data_pins = (size_t)MP_OBJ_SMALL_INT_VALUE(mp_obj_len(args[ARG_data].u_obj));
}

const mcu_pin_obj_t *data_pins_array[num_data_pins];

if (mp_obj_is_type(args[ARG_data].u_obj, &mcu_pin_type)) {
const mcu_pin_obj_t *datapin = validate_obj_is_free_pin(args[ARG_data].u_obj, MP_QSTR_data);
data_pins_array[0] = datapin;
} else {
for (size_t pin = 0; pin < num_data_pins; pin++) {
const mcu_pin_obj_t *datapin =
validate_obj_is_free_pin(mp_obj_subscr(args[ARG_data].u_obj, MP_OBJ_NEW_SMALL_INT(pin), MP_OBJ_SENTINEL), MP_QSTR_data);
data_pins_array[pin] = datapin;
}
}

size_t num_key_counts;

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

mp_arg_validate_length(num_key_counts, num_data_pins, MP_QSTR_key_count);

size_t key_count_array[num_key_counts];

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;
} else {
for (size_t kc = 0; kc < num_key_counts; kc++) {
mp_int_t mpint = mp_obj_get_int(mp_obj_subscr(args[ARG_key_count].u_obj, MP_OBJ_NEW_SMALL_INT(kc), MP_OBJ_SENTINEL));
const size_t key_count = (size_t)mp_arg_validate_int_min(mpint, 1, MP_QSTR_key_count);
key_count_array[kc] = key_count;
}
}

const mcu_pin_obj_t *clock = validate_obj_is_free_pin(args[ARG_clock].u_obj, MP_QSTR_clock);
const mcu_pin_obj_t *data = validate_obj_is_free_pin(args[ARG_data].u_obj, MP_QSTR_data);
const mcu_pin_obj_t *latch = validate_obj_is_free_pin(args[ARG_latch].u_obj, MP_QSTR_latch);
const bool value_to_latch = args[ARG_value_to_latch].u_bool;

const size_t key_count = (size_t)mp_arg_validate_int_min(args[ARG_key_count].u_int, 1, MP_QSTR_key_count);
const bool value_when_pressed = args[ARG_value_when_pressed].u_bool;
const mp_float_t interval =
mp_arg_validate_obj_float_non_negative(args[ARG_interval].u_obj, 0.020f, MP_QSTR_interval);
const size_t max_events = (size_t)mp_arg_validate_int_min(args[ARG_max_events].u_int, 1, MP_QSTR_max_events);

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

return MP_OBJ_FROM_PTR(self);

#else
mp_raise_NotImplementedError_varg(translate("%q"), MP_QSTR_ShiftRegisterKeys);
#endif
Expand Down Expand Up @@ -155,7 +200,7 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(keypad_shiftregisterkeys___exit___obj
//| ...

//| key_count: int
//| """The number of keys that are being scanned. (read-only)
//| """The total number of keys that are being scanned. (read-only)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the user code, would it be more useful to return a tuple in the case of multiple shift registers? Instead of doing a gc_alloc, you could create a tuple, and you can index into that, and return it for .key_count. (I was thinking about a new .key_counts, but the input arg is still key_count, so it would not be consistent.)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How important is it to have a different key count per shift register? Is this needed for a real in-the-wild design, or is it more a hypothetical?

Another alternative would be to have a single argument giving the maximum count of keys. "shift register 0" would be 0..(n-1), "shift register 1" would be n..(2*n-1) and so on. Unused positions would need to be hard-wired to the non-pressed state (not an undefined state or the pressed state), and some key numbers would be unused if the real number of keys per shift register is "ragged" (e.g., 3, 5, 3, 6, or whatever).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current use case i have is ragged, but I could live in my case with a single key_count and ignore unused keys (which would i my case be signalled as pressed and end up in the event queue). I believe the key_count() function is also used to allocate the currently_pressed / previously_pressed stuff for the toplevel keypad code, so a tuple might not work.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is needed to support already existing hardware then I'm not opposed to incorporating a feature that supports ragged shift registers.

//| """

//| events: EventQueue
Expand Down
2 changes: 1 addition & 1 deletion shared-bindings/keypad/ShiftRegisterKeys.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

extern const mp_obj_type_t keypad_shiftregisterkeys_type;

void common_hal_keypad_shiftregisterkeys_construct(keypad_shiftregisterkeys_obj_t *self, const mcu_pin_obj_t *clock_pin, const mcu_pin_obj_t *data_pin, const mcu_pin_obj_t *latch_pin, bool value_to_latch, size_t key_count, bool value_when_pressed, mp_float_t interval, size_t max_events);
void common_hal_keypad_shiftregisterkeys_construct(keypad_shiftregisterkeys_obj_t *self, const mcu_pin_obj_t *clock_pin, mp_uint_t num_data_pins, const mcu_pin_obj_t *data_pins[], const mcu_pin_obj_t *latch_pin, bool value_to_latch, size_t num_key_count, size_t key_counts[], bool value_when_pressed, mp_float_t interval, size_t max_events);

void common_hal_keypad_shiftregisterkeys_deinit(keypad_shiftregisterkeys_obj_t *self);

Expand Down
104 changes: 77 additions & 27 deletions shared-module/keypad/ShiftRegisterKeys.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,30 +44,54 @@ static keypad_scanner_funcs_t shiftregisterkeys_funcs = {
.get_key_count = shiftregisterkeys_get_key_count,
};

void common_hal_keypad_shiftregisterkeys_construct(keypad_shiftregisterkeys_obj_t *self, const mcu_pin_obj_t *clock_pin, const mcu_pin_obj_t *data_pin, const mcu_pin_obj_t *latch_pin, bool value_to_latch, size_t key_count, bool value_when_pressed, mp_float_t interval, size_t max_events) {
void common_hal_keypad_shiftregisterkeys_construct(keypad_shiftregisterkeys_obj_t *self, const mcu_pin_obj_t *clock_pin, mp_uint_t num_data_pins, const mcu_pin_obj_t *data_pins[], const mcu_pin_obj_t *latch_pin, bool value_to_latch, mp_uint_t num_key_counts, size_t key_counts[], bool value_when_pressed, mp_float_t interval, size_t max_events) {

digitalio_digitalinout_obj_t *clock = m_new_obj(digitalio_digitalinout_obj_t);
clock->base.type = &digitalio_digitalinout_type;
common_hal_digitalio_digitalinout_construct(clock, clock_pin);
common_hal_digitalio_digitalinout_switch_to_output(clock, false, DRIVE_MODE_PUSH_PULL);
self->clock = clock;

digitalio_digitalinout_obj_t *data = m_new_obj(digitalio_digitalinout_obj_t);
data->base.type = &digitalio_digitalinout_type;
common_hal_digitalio_digitalinout_construct(data, data_pin);
common_hal_digitalio_digitalinout_switch_to_input(data, PULL_NONE);
self->data = data;

digitalio_digitalinout_obj_t *latch = m_new_obj(digitalio_digitalinout_obj_t);
latch->base.type = &digitalio_digitalinout_type;

common_hal_digitalio_digitalinout_construct(latch, latch_pin);
common_hal_digitalio_digitalinout_switch_to_output(latch, true, DRIVE_MODE_PUSH_PULL);
self->latch = latch;
self->value_to_latch = value_to_latch;

mp_obj_t dios[num_data_pins];

for (size_t i = 0; i < num_data_pins; i++) {
digitalio_digitalinout_obj_t *dio = m_new_obj(digitalio_digitalinout_obj_t);
dio->base.type = &digitalio_digitalinout_type;
common_hal_digitalio_digitalinout_construct(dio, data_pins[i]);
common_hal_digitalio_digitalinout_switch_to_input(dio, PULL_NONE);
dios[i] = dio;
}

// Allocate a tuple object with the data pins
self->data_pins = mp_obj_new_tuple(num_data_pins, dios);

self->key_counts = (mp_uint_t *)gc_alloc(sizeof(mp_uint_t) * num_key_counts, false, false);
self->num_key_counts = num_key_counts;

// copy to a gc_alloc() 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;

self->value_to_latch = value_to_latch;
self->value_when_pressed = value_when_pressed;
self->key_count = key_count;
self->funcs = &shiftregisterkeys_funcs;

keypad_construct_common((keypad_scanner_obj_t *)self, interval, max_events);
Expand All @@ -85,18 +109,28 @@ void common_hal_keypad_shiftregisterkeys_deinit(keypad_shiftregisterkeys_obj_t *
common_hal_digitalio_digitalinout_deinit(self->clock);
self->clock = MP_ROM_NONE;

common_hal_digitalio_digitalinout_deinit(self->data);
self->data = MP_ROM_NONE;

common_hal_digitalio_digitalinout_deinit(self->latch);
self->latch = MP_ROM_NONE;

for (size_t key = 0; key < self->data_pins->len; key++) {
common_hal_digitalio_digitalinout_deinit(self->data_pins->items[key]);
}
self->data_pins = MP_ROM_NONE;
self->key_counts = MP_ROM_NONE;

common_hal_keypad_deinit_core(self);
}

size_t shiftregisterkeys_get_key_count(void *self_in) {
keypad_shiftregisterkeys_obj_t *self = self_in;
return self->key_count;

size_t total = 0;

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

return total;
}

static void shiftregisterkeys_scan_now(void *self_in, mp_obj_t timestamp) {
Expand All @@ -105,28 +139,44 @@ static void shiftregisterkeys_scan_now(void *self_in, mp_obj_t timestamp) {
// Latch (freeze) the current state of the input pins.
common_hal_digitalio_digitalinout_set_value(self->latch, self->value_to_latch);

const size_t key_count = shiftregisterkeys_get_key_count(self);
// Scan for max_key_count bit
for (mp_uint_t scan_number = 0; scan_number < self->max_key_count; scan_number++) {
common_hal_digitalio_digitalinout_set_value(self->clock, false);

for (mp_uint_t key_number = 0; key_number < key_count; key_number++) {
// Zero-th data appears on on the data pin immediately, without shifting.
common_hal_digitalio_digitalinout_set_value(self->clock, false);

// Remember the previous up/down state.
const bool previous = self->currently_pressed[key_number];
self->previously_pressed[key_number] = previous;
// Loop through all the data pins that share the latch
mp_uint_t index = 0;

// Get the current state.
const bool current =
common_hal_digitalio_digitalinout_get_value(self->data) == self->value_when_pressed;
self->currently_pressed[key_number] = current;
for (mp_uint_t i = 0; i < self->data_pins->len; i++) {

// When this data pin has less shiftable bits, ignore it
if (scan_number >= self->key_counts[i]) {
continue;
}

mp_uint_t key_number = scan_number + index;

// Remember the previous up/down state.
const bool previous = self->currently_pressed[key_number];
self->previously_pressed[key_number] = previous;

// Get the current state.
const bool current =
common_hal_digitalio_digitalinout_get_value(self->data_pins->items[i]) == self->value_when_pressed;
self->currently_pressed[key_number] = current;

// Record any transitions.
if (previous != current) {
keypad_eventqueue_record(self->events, key_number, current, timestamp);
}

index += self->key_counts[i];
}

// Trigger a shift to get the next bit.
common_hal_digitalio_digitalinout_set_value(self->clock, true);

// Record any transitions.
if (previous != current) {
keypad_eventqueue_record(self->events, key_number, current, timestamp);
}
}

// Start reading the input pins again.
Expand Down
6 changes: 4 additions & 2 deletions shared-module/keypad/ShiftRegisterKeys.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,11 @@
typedef struct {
KEYPAD_SCANNER_COMMON_FIELDS;
digitalio_digitalinout_obj_t *clock;
digitalio_digitalinout_obj_t *data;
digitalio_digitalinout_obj_t *latch;
size_t key_count;
mp_obj_tuple_t *data_pins;
mp_uint_t *key_counts;
mp_uint_t num_key_counts;
mp_uint_t max_key_count;
bool value_when_pressed;
bool value_to_latch;
} keypad_shiftregisterkeys_obj_t;
Expand Down