Looking for a stylish way to read out a Keypad (Button-Matrix)

Hallo, Everyone.

I did a few steps forward with Arduino and family in the past. Now I want to go futher on and look for Beauty inside the code.

English isn’t my first language (maybe also not even the second) and I am not a programmer. So be indulgent with a - maybe - stupid problem:

I have a Keypad with 60+ Keys here. Every Key should send a different line to a RS485-Bus.
RS485 is running, everything (including hardware) is fine.

I made a similar project about one year ago with also about 70 buttons which arranged in two matrixes. Button-presses I read out with a switch-case monster, releases of those buttons also.

In this new project I want to prevent this monster of cases cause it’s a crime to debug this :slight_smile:

Here the short relevant detail of my code (the rest isn’t a secret, I just want to set a focus):

...
#include <Keypad.h>

const byte rows = 8; //eight rows
const byte cols = 9; //nine columns = 72 buttons ara available
char CDU[rows][cols] = { // upper-/lower-case DOES matter!!!
  {'1','2','3','4','5','6','7','8','9'},
  {'0','a','b','c','d','e','f','g','h'},
  {'i','j','k','l','m','n','o','p','q'},
  {'r','s','t','u','v','w','x','y','z'},
  {'A','B','C','D','E','F','G','H','I'},
  {'J','K','L','M','N','O','P','Q','R'},
  {'S','T','U','V','W','X','Y','Z','/'},
  {'%','&','(',')','=','?','ä','ö','ü'},
};
byte rowPins[rows] = {A6, A5, A4, A3, A2, A1, A0, 13}; //connect to the row pinouts of the keypad
byte colPins[cols] = {12, 11, 10, 9, 8, 7, 6, 5, 4}; //connect to the column pinouts of the keypad
Keypad keypad = Keypad( makeKeymap(keys), rowPins, colPins, rows, cols );

void keypadEvent(KeypadEvent OSB){	
	switch (CDU.getState()) {	// gives PRESSED, HOLD or RELEASED
	case PRESSED:	
	  switch(keys) { // following commands are fired unique if PRESSED
	    case '1': sendDcsBiosMessage("CDU_0", "1"); break; // CDU 0
	    case '2': sendDcsBiosMessage("CDU_1", "1"); break; // CDU 1
	    case '3': sendDcsBiosMessage("CDU_2", "1"); break; // CDU 2
		// here will follow 60+ other case(s)
	  }
	}
	case RELEASED:	
	  switch(keys) { // following commands are fired unique if RELEASED
	    case '1': sendDcsBiosMessage("CDU_0", "0"); break; // CDU 0
	    case '2': sendDcsBiosMessage("CDU_1", "0"); break; // CDU 1
	    case '3': sendDcsBiosMessage("CDU_2", "0"); break; // CDU 2
		// here will follow 60+ other case(s)
	  }
	}
}
...

Expressions inside case doesn’t make sense here without context, but trust me: It works. :slight_smile:
I did not compile nor uploaded this sketch to a AVR. But I know it will run - as the last one did.

Here is finally my problem or question:
Do you know a (much) better way to read out what happens inside the Keypad?
As you’ll see, it is just a small part of the code what changes. There could be 2 variables (which I have to declare somehow and somewhere).

I want just prevent this ugly swich-case-construction.

I know: While I wrote this post I could easy manage to write 2x 60 lines of code. But: Good code has(!) to be also beautiful.

Thank you in advance
DRESI

You have your keys in an array and it seems that what you do when you identify a key press is pretty easy to generate so just do a big for to look up for the key pressed and generate your command taking into account the getState() status... will be like 5 lines of code probably...

DRESI:
I know: While I wrote this post I could easy manage to write 2x 60 lines of code. But: Good code has(!) to be also beautiful.

That’s kind of ridiculous. Usually there are one or two ways to accomplish a task, and usually one of them is “cleaner” or “simpler”. But, in the end what’s important is that the code WORKS properly. It matters not if the code is “beautiful” (whatever that means). Do you mean “properly indented”, or “cute variable names” or “well commented”? What is “beautiful code”?

Now, I will post a small piece of code that I use to read a 3 X 4 keypad array. The point is not that you can “cut and paste” it into your project, but rather see how I did it and adapt the idea to your project.

I don’t know if my code is “beautiful”, and certainly there are several other ways to do it, but this is one example that may give you some ideas how to make your project work.

// Arduino pins connected to COLUMNS
#define COL3 18
#define COL2 19
#define COL1 20

// Arduino pins connected to ROWS
#define ROW1 14
#define ROW2 15
#define ROW3 16
#define ROW4 17

// call scan matrix and debounce whatever we got
uint8_t get_key (void)
{
   uint8_t k;
   uint16_t d;
   k = -1;
   d = 100;

   while (d--) { // debounce - key must be stable for 100 msec
      if (k != scan_matrix()) {
         k = scan_matrix();
         d = 100;
      }

      __builtin_avr_delay_cycles ((F_CPU * 1) / 1e3); // 1 msec
   }

   return k;
}

// scan keyboard matrix. return unique key code
// or "-1" if more than one key was pressed.
uint8_t scan_matrix (void)
{
   uint8_t n, keyCode, colCount, rowCount, column, row, multiplier;

   const int columns[] = {
      COL1, COL2, COL3,
   };

   const int rows[] = {
      ROW1, ROW2, ROW3, ROW4,
   };

   colCount = (sizeof (columns) / sizeof (*columns));
   rowCount = (sizeof (rows) / sizeof (*rows));
   // "multiplier" is the larger of the two array sizes (if they are different)
   // and is required so that each key code is unique.
   multiplier = (colCount < rowCount) ? rowCount : colCount;

   for (n = 0; n < colCount; n++) {
      pinMode (columns[n], INPUT_PULLUP);
   }

   for (n = 0; n < rowCount; n++) {
      pinMode (rows[n], OUTPUT);
   }

   keyCode = 0;

   for (row = 0; row < rowCount; row++) {
      digitalWrite (rows[row], LOW); // activate a row
      // 1 usec delay allows lines to stabilize - keypads seem to have
      // a LOT of capacitance and an immediate read after may return junk.
      __builtin_avr_delay_cycles ((F_CPU * 1) / 1e6); // 1 usec

      for (column = 0; column < colCount; column++) { // scan for which column

         if (digitalRead (columns[column]) == LOW) {

            if (keyCode == 0) {
               keyCode = ((column * multiplier) + (row + 1)); // return unique keycode

            } else {
               keyCode = -1; // more than 1 key pressed - error
            }
         }
      }

      digitalWrite (rows[row], HIGH);
   }

   return keyCode;
}

void setup (void)
{
   Serial.begin (115200);
}

void loop (void)
{
   uint8_t key, oldkey;
   char buffer[32];

   key = get_key();

   if (key != oldkey) { // only print when a different key code is received
      oldkey = key;
      sprintf (buffer, "keyscan: 0x%02X\r\n", key);
      Serial.print (buffer);
   }
}

Note that this code returns a unique code for each key, a 0 if no keys are pressed and -1 if more than one is pressed at a time. You will have to see what code each key generates and then run that through a lookup table to translate it into a real “key” value (easily done with a simple array - use the key code as the array index).

This should give you enough to get started. Whether or not it’s “beautiful” code… I have no idea and honestly it’s irrelevant.

I think the switch:case will be the most responsive as you're not doing a while of if-else tests, or looping thru a bunch of for tests. Who cares what it looks like if it runs fast?

The key is to capture the data requirements for all keys and store it as a data table, then build a relationship between a key and a position in the data table.

I would set up data tables (use structures to combine fields) to have all the data for each key: - The label eg, "CDU_0" - The string for on, eg "1" - The string for off, eg "0" - Whatever else you need

Then you can use a static initialiser for this data, even put it in PROGMEM to save RAM space.

When you detect the change in a key you look up the data in the static array and send that. This lookup/transmit mechanism should be the same for all keys, which means the code will be much more compact as you basically eliminate all the switch statements. This also makes it expandable as needed, just by changing the data table and not the code.

To optimise memory, consider that if every key label starts with CDU_ you do not need to store that. Similarly if a single character is all you need for the sending device, then only store that one character instead of including the terminating '\0' for the string. You can make more complicated strings/messages in the lookup/transmit function as needed just before transmission.

As an example of this separation of data/code (for a different purpose) you can look at the MultiBlink example found in my code libraries (link in signature block).

Thank you for your quick replies. (faster than I managed to come back to see it) :)

I think "data table" is the right key. PROGMEM I remember as I fought against fonts. Now I have to get some sleep (preparing for work tomorrow).

Will go for that next week and post my affords here. Right now I have still no idea how to make this. Will do some reading :)