matrix_keypad: Add matrix-style keypad module #18733

bergzand · 2022-10-13T12:46:54Z

Contribution description

This module implements a simple matrix keypad driver where keys are connected
between GPIO columns and rows. It works best with diodes in series with the
switches to prevent key ghosting, but it can be used without.

For example one of these: https://learn.adafruit.com/matrix-keypad

Testing procedure

Test output with a custom board:

2022-10-13 13:54:23,675 # SEGGER J-Link V7.58 - Real time terminal output
2022-10-13 13:54:23,676 # J-Link EDU Mini V1 compiled Nov  2 2021 11:12:01 V1.0, SN=801025014
2022-10-13 13:54:23,676 # Process: JLinkExe
2022-10-13 13:54:23,676 # main(): This is RIOT! (Version: 2022.10-devel-1128-gfa62a-pr/driver/matrix_keypad)
2022-10-13 13:54:23,676 # Generated RIOT application: 'matrix_keypad'
2022-10-13 13:54:23,677 # [OK]
2022-10-13 13:54:23,677 #
2022-10-13 13:54:24,242 # Key switch at column 2 and row 1 is pressed!
2022-10-13 13:54:24,394 # Key switch at column 2 and row 1 is released!
2022-10-13 13:54:24,923 # Key switch at column 2 and row 2 is pressed!
2022-10-13 13:54:25,037 # Key switch at column 2 and row 2 is released!
2022-10-13 13:54:25,294 # Key switch at column 2 and row 3 is pressed!
2022-10-13 13:54:25,395 # Key switch at column 2 and row 3 is released!
2022-10-13 13:54:25,611 # Key switch at column 2 and row 4 is pressed!
2022-10-13 13:54:25,712 # Key switch at column 2 and row 4 is released!
2022-10-13 13:54:25,946 # Key switch at column 1 and row 3 is pressed!
2022-10-13 13:54:26,047 # Key switch at column 1 and row 3 is released!

Full Kconfig configuration is provided with the driver (to the best of my ability), I would love it if somebody could sanity check it.

Issues/PRs references

None

benpicco

Looks neat!

drivers/include/matrix_keypad.h

benpicco · 2022-10-19T23:35:58Z

drivers/include/matrix_keypad.h

+    /**
+     * @brief Device initialization parameters
+     */
+    matrix_keypad_params_t params;


Why not

Suggested change

matrix_keypad_params_t params;

const matrix_keypad_params_t *params;

I'm fine with either, afaik this is a memory latency vs ram usage tradeoff.

Is that measurable?
We are also running the code from flash after all.

I really don't remember the specifics.

But I'm fine with either here, so just confirm if you prefer it in ROM.

It depends on the MCU. E.g. AVRs are clocked so slow so that they do not need to wait for ROM access - it really is the flash that is the bottleneck for CPU clock there. But most other MCUs do implement the logic required to wait on bus accesses. E.g. Cortex M7 have to spend a few cycles waiting for a ROM access, even not every RAM region is accessible in a single CPU cycle. However, Cortex M7 also have quite an elaborate cache system to speed things up.

While I do like a to have an as low latency between the button press and the letter appearing in neovim on the terminal, I am pretty confident that the few CPU cycles spend waiting for the flash (on a cache miss) do not contribute noticeably to that latency.

Reworked it as suggested above.

tests/driver_matrix_keypad/main.c

benpicco · 2022-10-19T23:42:15Z

drivers/include/matrix_keypad.h

+/**
+ * @brief Debounce pattern high to low bits
+ */
+#ifndef CONFIG_MATRIX_KEYPAD_DEBOUNCE_PATTERN_BEGIN
+#define CONFIG_MATRIX_KEYPAD_DEBOUNCE_PATTERN_BEGIN  0xC0
+#endif
+
+/**
+ * @brief Debounce pattern low to high bits
+ */
+#ifndef CONFIG_MATRIX_KEYPAD_DEBOUNCE_PATTERN_END
+#define CONFIG_MATRIX_KEYPAD_DEBOUNCE_PATTERN_END  0x7
+#endif


How would one adjust these?

3 bits set -> 3 cycles of matrix_keypad_scan() needed to assert state?

Yes, see

RIOT/drivers/include/matrix_keypad.h

Lines 32 to 40 in ef33bd0

* The debouncing algorithm is a pattern style debounce where the switch must be

* in one position for a number of samples, then a set of "don't care" samples

* and then in the other position for a number of samples. The samples in the

* middle allow for a period where the switch can be either low or high without

* affecting the transition. The exact pattern is determined by

* @ref CONFIG_MATRIX_KEYPAD_DEBOUNCE_PATTERN_BEGIN and

* @ref CONFIG_MATRIX_KEYPAD_DEBOUNCE_PATTERN_END. These are used as mask where

* the switch must be in a determined state. The bits where neither pattern is

* set is used as the "don't care" set of samples.

It's a bit mask. I would love to use 0b00000111 here, but that's a gnu extension

It's also in C23 - but yea afaik we can't use it yet.

I'd still like to have the number of set bits -> number of required on/off cycles documented just to rule out any ambiguity.

I elaborated a bit on the requirements here and tried to remove the ambiguity. Let me know what you think

benpicco

Please squash

riot-ci · 2022-10-28T10:23:55Z

Murdock results

✔️ PASSED

1d33455 matrix_keypad: add test application

Success	Failures	Total	Runtime
2000	0	2000	07m:02s

Artifacts

Documentation preview

This only reflects a subset of all builds from https://ci-prod.riot-os.org. Please refer to https://ci.riot-os.org for a complete build for now.

bergzand added the Type: new feature The issue requests / The PR implemements a new feature for RIOT label Oct 13, 2022

bergzand requested review from leandrolanzieri, jia200x, MrKevinWeiss, smlng, aabadie, MichelRottleuthner and fjmolinas as code owners October 13, 2022 12:46

github-actions bot added Area: doc Area: Documentation Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: tests Area: tests and testing framework labels Oct 13, 2022

bergzand force-pushed the pr/driver/matrix_keypad branch from 863a85e to e95876c Compare October 13, 2022 13:02

benpicco requested a review from OlegHahm October 13, 2022 13:16

benpicco reviewed Oct 19, 2022

View reviewed changes

benpicco approved these changes Oct 27, 2022

View reviewed changes

bergzand added 2 commits October 28, 2022 10:03

matrix_keypad: Add driver for a matrix keypad

7f3cbfd

matrix_keypad: add test application

1d33455

bergzand force-pushed the pr/driver/matrix_keypad branch from dcc987b to 1d33455 Compare October 28, 2022 08:03

bergzand added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 28, 2022

benpicco merged commit 2a934c9 into RIOT-OS:master Oct 28, 2022

bergzand deleted the pr/driver/matrix_keypad branch October 29, 2022 21:00

bergzand restored the pr/driver/matrix_keypad branch January 17, 2023 12:06

kaspar030 added this to the Release 2023.01 milestone Jan 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

matrix_keypad: Add matrix-style keypad module #18733

matrix_keypad: Add matrix-style keypad module #18733

bergzand commented Oct 13, 2022

benpicco left a comment

benpicco Oct 19, 2022

bergzand Oct 20, 2022

benpicco Oct 20, 2022

bergzand Oct 20, 2022

bergzand Oct 20, 2022

maribu Oct 25, 2022

bergzand Oct 27, 2022

benpicco Oct 19, 2022

bergzand Oct 20, 2022

benpicco Oct 20, 2022

bergzand Oct 20, 2022

benpicco left a comment

riot-ci commented Oct 28, 2022

	matrix_keypad_params_t params;
	const matrix_keypad_params_t *params;

	* The debouncing algorithm is a pattern style debounce where the switch must be
	* in one position for a number of samples, then a set of "don't care" samples
	* and then in the other position for a number of samples. The samples in the
	* middle allow for a period where the switch can be either low or high without
	* affecting the transition. The exact pattern is determined by
	* @ref CONFIG_MATRIX_KEYPAD_DEBOUNCE_PATTERN_BEGIN and
	* @ref CONFIG_MATRIX_KEYPAD_DEBOUNCE_PATTERN_END. These are used as mask where
	* the switch must be in a determined state. The bits where neither pattern is
	* set is used as the "don't care" set of samples.

matrix_keypad: Add matrix-style keypad module #18733

matrix_keypad: Add matrix-style keypad module #18733

Conversation

bergzand commented Oct 13, 2022

Contribution description

Testing procedure

Issues/PRs references

benpicco left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benpicco left a comment

Choose a reason for hiding this comment

riot-ci commented Oct 28, 2022

Murdock results

Artifacts