0

Im at my wits end here. I'm writing a program for a keyboard with each key having multiple functions. So far, I've managed to implement single functions for the keys without a debouncing routine. The code:

int keymat(void)
{
    for (int c = 0; c < 9; c++)                                             //COL 1 - 9
    {
        for (int i = 0; i < 9; i++)
        {
            if (i == c)
                col[i] = 1;
            else
                col[i] = 0;
            HAL_GPIO_WritePin(GPIOC, gpio_pin_struct_lut[0][i], col[i]);    //SET ONE PIN TO HIGH AND 
                                                                              OTHERS TO LOW
        }

            for (int r = 0; r < 7; r++)                                     //ROW 1 - 7
            {
                if ((HAL_GPIO_ReadPin(((GPIO_TypeDef *) gpio_port_struct_lut[r]), gpio_pin_struct_lut[1][r])))      //READ EACH ROW
                    return keyboard_struct_lut[0][r][c];

                }
            }
    }
    return 0;
}

where gpio_port_struct_lut[r] and gpio_pin_struct_lut[1][r] are arrays which include the addresses of the port and the pins and keyboard_struct_lut[0][r][c] is a 3D array that contains numbers that correspond to switch cases for each switch. This much works, and I've confirmed it.

The problem arises when I try to implement multiple functions within the switch:

for (int r = 0; r < 7; r++)                                 //ROW 1 - 7
{
if (button_down != (HAL_GPIO_ReadPin(((GPIO_TypeDef *) gpio_port_struct_lut[r]), gpio_pin_struct_lut[1][r])))       //READ EACH ROW
{
   button_down = !button_down;
   if( button_down )
   {
       button_down_ts = now;  // Timestamp button-down
   }
   else
   {
       button_up_ts = now;   // Timestamp button-up
       if( double_pending )   // If double decision pending...
       {
           return keyboard_struct_lut[1][r][c];
           double_pending = false;
       }
       else
       {
           double_pending = true;
       }
       long_press_pending = false;   // Cancel any long press pending
   }
}
// If button-up and double-press gap time expired, it was a single press
if( !button_down && double_pending && now - button_up_ts > DOUBLE_GAP_MILLIS_MAX )
{
    return keyboard_struct_lut[0][r][c];
    double_pending = false ;
}
// else if button-down for long-press...
else if( !long_press_pending && button_down && now - button_down_ts > LONG_MILLIS_MIN )
{
    return keyboard_struct_lut[2][r][c];
    button_down = false;
    long_press_pending = false;
    double_pending = false;

Note: Multiple functions code was provided to me by Clifford, however, that was for a single switch.

On implementing this code within my program, microcontroller keeps outputting the first function and won't go past it. I understand the logic behind the code, but I just cannot progress from here. Where am I going wrong? How would I implement this code within the first section of code? I would be very grateful for all of your help.

Joshua John
  • 55
  • 1
  • 7
  • Your code appears to make extensive use of global variables the declarations of which you have not shown. Not only does that make it very difficult to understand your code, it is probably unnecessary. The `col` array in `keymat()` seems particularly unnecessary. You just need `HAL_GPIO_WritePin(GPIOC, gpio_pin_struct_lut[0][i], i == c ? 1 : 0 );` in the column set-up loop - `col` serves no purpose. You are perhaps overcomplicating your code, then cannot understand how it works - or rather why it does not in this case. – Clifford Mar 29 '21 at 16:43
  • You really need to post _real_ code conventionally formatted/indented. Your `keymat()` fragment has mismatched braces - the `return` statement is outside of the function. – Clifford Mar 31 '21 at 17:36

1 Answers1

0

I suggest you modularise this for clarity of thought, maintainability and general good practice. So to that end first implement (and test) a low-level keypad scan, that simply provides access to the KEY_UP/KEY_DOWN state of each keypad switch. You appear to have that in keymat, but it utilises variables and structures you have not provided, and which frankly suggest unnecessary complexity, so I have started again. So the first module will be keyscan with a header:

// keyscan.h
#if !defined KEYSCAN_H
#define defined KEYSCAN_H

#define SCANCODE_MAX 62

typedef enum 
{
    KEY_UP, 
    KEY_DOWN
} tKeyState ;

tKeyState getKeyState( unsigned scancode ) ;
void keyscan( void ) ;

#endif

Then the implementation:

// keyscan.c
#include keyscan.h


// Keypad charactaristics
#define KEYPAD_COLUMNS 9
#define KEYPAD_ROWS 7
#define KEYPAD_SCANCODES (KEYPAD_ROWS * KEYPAD_COLUMNS)
#define DEBOUNCE_MILLIS 20

// Key pad I/O assignment
static const struct
{
    GPIO_TypeDef* port ;
    uint16_t pin ;
} keyscan_row_io[KEYPAD_ROWS] = { {GPIOC, 0},       // C1
                                  {GPIOC, 1},       // C2
                                  {GPIOC, 2},       // C3
                                  {GPIOC, 3},       // C4  
                                  {GPIOC, 4},       // C5
                                  {GPIOC, 5},       // C6
                                  {GPIOC, 7} } ;    // C7

static const struct
{
    GPIO_TypeDef* port ;
    uint16_t pin ;
} keyscan_column_io[KEYPAD_COLUMNS] = { {GPIOC, 8},     // R1
                                        {GPIOC, 9},     // R2
                                        {GPIOC, 10},    // R3
                                        {GPIOC, 11},    // R4 
                                        {GPIOC, 12},    // R5
                                        {GPIOC, 13},    // R6
                                        {GPIOC, 14}     // R7
                                        {GPIOC, 15}     // R8
                                        {GPIOD, 0} } ;  // R9


// Key state array
static tKeyState keystate[KEYPAD_SCANCODES] ; // Index by key scancodes 0 to 62 
                                              // being row * KEYPAD_ROWS + column

// Access function (hiding keystate)
tKeyState getKeyState( unsigned scancode )
{
    return scancode < KEYPAD_SCANCODES ? keystate[scancode] : KEY_UP ;
}

// Scanner
void keyscan( void )
{
    static uint32_t last_scan_ts = 0 ;
    uint32_t now = HAL_GetTick() ;

    if( now - last_scan_ts > DEBOUNCE_MILLIS  )
    {
        // For each row...
        for( int scan_row = 0; scan_row < KEYPAD_ROWS; scan_row++ )
        {
            // Set scan-row
            for( int r = 0; r < KEYPAD_ROWS; r++ )
            {
                HAL_GPIO_WritePin( keyscan_row_io[r].port, 
                                   keyscan_row_io[r].pin, 
                                   r == scan_row ? GPIO_PIN_SET : GPIO_PIN_RESET  ) ;    
            }

            // Read columns
            for( int scan_col= 0; scan_col < KEYPAD_COLUMNS; scan_col++ )
            {
                keystate[scan_row * KEYPAD_ROWS  + scan_col] = HAL_GPIO_ReadPin( keyscan_column_io[scan_col].port, 
                                                                                 keyscan_column_io[scan_col].pin ) == GPIO_PIN_SET ?
                                                               KEY_DOWN : KEY_UP ;
            }
        }
    }
}

A few things to note comparing with your keymat() function.

  • I have scanned rows rather than columns simply because it is more conventional.
  • No two or three dimensional arrays, which clearly overcomplicate the issue.
  • No magic numbers - a bad habit of yours.
  • It has rudimentary debouncing by ensuring the keypad cannot be scanned at less that 20ms intervals, regardless of how frequently it is called.
  • I have put in dummy I/O assignments - you should change to match your hardware.
  • the code could be simplified a lot if your GPIO for row and column are all adjacent in the same port - you can then use a walking-one and write/read the entire port in one go, removing at least one loop, and reducing the number of I/O accesses to just two per row. Unfortunately the stupid STM32 HAL layer does not provide access to the while port in one read.

Now we have a function getKeyState() that works rather like buttonState() in your previous question Implementing a single press, long press and a double press function in HAL for STM32 except now it handles 62 switched instead of just one.

So using the same technique as in the earlier single button question but for each of the 63 keys - so you need to keep state variables for 63 instead of one key and iterate through the state of all 63 keys. The interface needs to change somewhat because you need to return up to 63 events. The simplest solution is probably to use a "callback" a function that the scanner calls when it detects an event. Alternatives include putting the events in a queue and then reading the buffered events after the event detection scan, or (my preferred solution) use an RTOS and run the event scan in a separate task and pass the events to a consumer in a message queue. Anyway, for illustration I am using the callback method. So lets have a keyevent module with header:

// keyevent.h
#if !defined KEYEVENT_H
#define defined KEYEVENT_H

typedef enum
{
    NO_PRESS,
    SINGLE_PRESS,
    LONG_PRESS,
    DOUBLE_PRESS
} eKeyEvent ;

typedef void (*tKeyEventCallback)( unsigned scancode, eKeyEvent keyevent ) ;
void scanKeyEvent( tKeyEventCallback event_callback ) ;
#endif

And the implementation:

// keyevent.c
#include "keyscan.h"
#include "keyevent.h"

void scanKeyEvent( tKeyEventCallback event_callback )
{
    static const uint32_t DOUBLE_GAP_MILLIS_MAX = 250 ;
    static const uint32_t LONG_MILLIS_MIN = 800 ;
    static struct
    {
        uint32_t button_down_ts ;
        uint32_t button_up_ts ;
        bool double_pending ;
        bool long_press_pending ;
        bool button_down ;
    } key_event_state[SCANCODE_MAX] = {0} ;

    eKeyEvent key_event = NO_PRESS ;
    uint32_t now = HAL_GetTick() ;

    // Do a keyscan - it does not matter if this is also done elsewhare, 
    // it handles its own update interval, and merely needs to be called 
    // frequently.  This ensures valid key state if it has not been called.
    keyscan() ;

    // for each key...
    for( unsigned key = 0; key <= SCANCODE_MAX; key++ )
    {
        // If state changed...
        if( key_event_state[key].button_down != getKeyState( key ) )
        {
            key_event_state[key].button_down = !key_event_state[key].button_down ;
            if( key_event_state[key].button_down )
            {
                // Timestamp button-down
                key_event_state[key].button_down_ts = now ;
            }
            else
            {
                // Timestamp button-up
                key_event_state[key].button_up_ts = now ;

                // If double decision pending...
                if( key_event_state[key].double_pending )
                {
                    key_event = DOUBLE_PRESS ;
                    key_event_state[key].double_pending = false ;
                }
                else
                {
                    key_event_state[key].double_pending = true ;
                }

                // Cancel any long press pending
                key_event_state[key].long_press_pending = false ;
            }
        }

        // If button-up and double-press gap time expired, it was a single press
        if( !key_event_state[key].button_down && 
            key_event_state[key].double_pending && 
            now - key_event_state[key].button_up_ts > DOUBLE_GAP_MILLIS_MAX )
        {
            key_event_state[key].double_pending = false ;
            key_event = SINGLE_PRESS ;
        }
        // else if button-down for long-press...
        else if( !key_event_state[key].long_press_pending && 
                 key_event_state[key].button_down && 
                 now - key_event_state[key].button_down_ts > LONG_MILLIS_MIN )
        {
            key_event = LONG_PRESS ;
            key_event_state[key].long_press_pending = false ;
            key_event_state[key].double_pending = false ;
        }

        if( key_event != NO_PRESS )
        {
            event_callback( key, key_event ) ;
        }
    }
}

See that this is the same algorithm as in the previous question, but it iterates for each key and has a state array rather than a single set of state variables. Also rather the returning a single event, it invokes the supplied callback for each event detected.

     ____________________________
____|                            |_____________
     <----long press min-->
                           ^
                           |_Long press detected
     ______     _____
____|      |___|     |_________________________
                     ^
                     |_Double press detected
     ______
____|      |___________________________________
           <------->
              ^    ^
              |    |_Single press detected
              |_ Double press gap max.

So as an example of how all that might be used in a "big-loop" executive:

#include "keyscan.h"
#include "keyevent.h"

// Key codes:
// Probably put all these in keyscan.h
// with an entry for each key name - for illustration only not exhaustive
// or even matching the actual keypad layout.
#define SCANCODE_ENTER 26 
#define SCANCODE_Q     9  
#define SCANCODE_W     10  
// ...


void keyEventCallback( unsigned scancode, eKeyEvent keyevent )
{
    if( scancode == SCANCODE_Q && keyevent == LONG_PRESS )
    {
        // Q key held down - do something
    }
    else if( scancode == SCANCODE_W && keyevent == DOUBLE_PRESS )
    {
        // W doubble tap - do something
    }
    else
    {
        // ignore any other events
    }
}

int main()
{
    for(;;)
    {
        // do a key scan
        keyscan() ;

        // Handle simple key presses (button down)
        if( getKeyState( SCANCODE_ENTER ) == KEY_DOWN )
        {
            // Enter key held down - do something
        }

        // Scan for button events
        // keyEventCallback() will be invoked for all events detected of any key
        scanKeyEvent( keyEventCallback )

        // Do other work...
    }
}

Note that all the above code is illustrative - none of it has been compiled or tested and will certainly need to be adapted to your hardware implementation. If it works out of the box without a bit of fettling I'd be amazed.

Clifford
  • 88,407
  • 13
  • 85
  • 165