0
\$\begingroup\$

I'm writing a bit of code for a keyboard. I've written this before, but it was inefficient and I figured as I was learning bare metal, I'd rewrite the code again. However, the said code wasn't working, so I've written the main in C to see what went wrong. My output pins (columns) are connected to pins PC0 - PC8 and input pins (rows) are connected to PD0 - PD6.

Initialization code:

 RCC->AHB1ENR |= RCC_AHB1ENR_GPIOCEN | RCC_AHB1ENR_GPIODEN; GPIOC->MODER |= GPIO_MODER_MODER0_0 | GPIO_MODER_MODER1_0 | GPIO_MODER_MODER2_0 | GPIO_MODER_MODER3_0 | GPIO_MODER_MODER4_0 | GPIO_MODER_MODER5_0 | GPIO_MODER_MODER6_0 | GPIO_MODER_MODER7_0 | GPIO_MODER_MODER8_0; GPIOD->PUPDR |= GPIO_PUPDR_PUPDR0_1 | GPIO_PUPDR_PUPDR1_1 | GPIO_PUPDR_PUPDR2_1 | GPIO_PUPDR_PUPDR3_1 | GPIO_PUPDR_PUPDR4_1 | GPIO_PUPDR_PUPDR5_1 | GPIO_PUPDR_PUPDR6_1;

Definitions, structures, typedefs, variables

typedef enum { KEY_UP, KEY_DOWN } keystate_t; #define NUM_COLS 9 #define NUM_ROWS 7 #define NUM_KEYS (NUM_COLS * NUM_ROWS) keystate_t key_state [NUM_KEYS] = {KEY_UP}; static const struct { GPIO_TypeDef* port ; uint16_t pin ; } keyscan_col_io[NUM_COLS] = { {GPIOC, 0}, {GPIOC, 1}, {GPIOC, 2}, {GPIOC, 3}, {GPIOC, 4}, {GPIOC, 5}, {GPIOC, 6}, {GPIOC, 7}, {GPIOC, 8} }; static const struct { GPIO_TypeDef* port ; uint16_t pin ; } keyscan_row_io[NUM_ROWS] = { {GPIOD, 0}, {GPIOD, 1}, {GPIOD, 2}, {GPIOD, 3}, {GPIOD, 4}, {GPIOD, 5}, {GPIOD, 6} };

While Loop

 while (1) { /* USER CODE END WHILE */ /* USER CODE BEGIN 3 */ for (int cols = 0; cols < NUM_COLS; cols++) { for (int c = 0; c < NUM_COLS; c++) { if (c == cols) HAL_GPIO_WritePin(GPIOC, keyscan_col_io[c].pin, GPIO_PIN_SET); else HAL_GPIO_WritePin(GPIOC, keyscan_col_io[c].pin, GPIO_PIN_RESET); } for (int rows = 0; rows < NUM_ROWS; rows++) { if (HAL_GPIO_ReadPin(GPIOD, keyscan_row_io[rows].pin)) key_state [rows * NUM_ROWS + cols] = KEY_DOWN; else key_state [rows * NUM_ROWS + cols] = KEY_UP; } } }

Like I said, I initially tried writing the while loop section in bare metal but the code didn't work as expected, so I wrote it in HAL to see if it would.

On checking the status registers, all the bits are set as expected. I set key_state as a live expression, so that I could view the states of all the keys. However, the keys won't trigger the right variables. I checked the connections and can confirm they're connected properly. Also, on checking with the previous, program I wrote, the keys trigger the right variables and I cannot see any difference between the code.

Where am I going wrong?

\$\endgroup\$
3
  • \$\begingroup\$ How does it make sense to have a nested loop iterating over columns inside the loop already iterating over columns? At a glance it seems you should ditch the inner for loop since it does nothing meaningful. \$\endgroup\$ Commented Apr 29, 2021 at 13:46
  • \$\begingroup\$ Also what about GPIO activation delays? Have you verified that the GPIO is faster than your MCU? That isn't always the case, you might need to give the pins some time between activation until reading. Especially if you have external RC filters etc. How do you debounce the keys? \$\endgroup\$ Commented Apr 29, 2021 at 13:48
  • \$\begingroup\$ @Lundin the outer loop just repeats the two inner loops for each column. The first inner loop for column ensures the correct pin is set and all the other pins are reset. Once that is done, the second inner loop for rows iterates through each row scanning for a pressed button. This process is repeated several times by the outer loop depending on the number of columns. As for the speed, the pins are by default set to low and I haven’t changed it. \$\endgroup\$ Commented Apr 29, 2021 at 15:26

1 Answer 1

Reset to default
0
\$\begingroup\$

Replace

for (int rows = 0; rows < NUM_ROWS; rows++) { if (HAL_GPIO_ReadPin(GPIOD, keyscan_row_io[rows].pin)) key_state [rows * NUM_ROWS + cols] = KEY_DOWN; else key_state [rows * NUM_ROWS + cols] = KEY_UP; }

with

for (int rows = 0; rows < NUM_ROWS; rows++) { if (HAL_GPIO_ReadPin(GPIOD, keyscan_row_io[rows].pin)) key_state [rows * NUM_COLS + cols] = KEY_DOWN; else key_state [rows * NUM_COLS + cols] = KEY_UP; }

So multiply by NUM_COLS instead of NUM_ROWS.

Or it may be also cols * NUM_ROWS + rows depending on your mapping. The way it is now is incorrect.

\$\endgroup\$
7
  • \$\begingroup\$ I tried this and now the input does trigger a variable but not the right ones. \$\endgroup\$ Commented Apr 29, 2021 at 12:22
  • \$\begingroup\$ can you give an example? did you try cols * NUM_ROWS + rows also? \$\endgroup\$ Commented Apr 29, 2021 at 12:23
  • \$\begingroup\$ When I press the button on (R1, C1) (PC0 as output and PD0 as input) which corresponds to key_state [0], it triggers key_state [16] instead. \$\endgroup\$ Commented Apr 29, 2021 at 12:27
  • \$\begingroup\$ With cols * NUM_ROWS + rows, on pressing the button on (R1, C1), key_state [50, 52, and 54] are triggered. \$\endgroup\$ Commented Apr 29, 2021 at 12:32
  • \$\begingroup\$ double check the connections it seems what you are considering R1,C1 is effectively R2,C8. by the way how come it gives three different states? \$\endgroup\$ Commented Apr 29, 2021 at 13:15

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.