My first guess is that you are blasting through you latchPin() function too many times while pressing a button.  I know you are complaining about the code running too slow but it is only too slow when you are concerned about your output.  When a human presses a button they will typically hold it down for 10's or 100's of milliseconds.  How many times does your loop() function get called per second while someone is pressing a button?  Hundreds or maybe thousands of times.  You need to track that.

It looks like you are printing information from all 4 (4051) mux chips on every pass through the loop.  So you are probably getting information from the same pin on each of the four chips instead of just one like you are expecting.

[Read this before posting a programming question ...]

I am new here and I will try to follow the forum protocol as much as I can.

That's good but you missed the very first step in the protocol.   There's a sticky post at the top of this forum that explains how to ask your question.  Read this before posting a programming question ... http://forum.arduino.cc/index.php?topic=97455.0 and more specifically item 6. Getting help on the forum.

But when we combine the two codes into 1 it does not work well.  Any general ideas?
Thanks!

Sorry, but this can't be answered generally without getting into a very long article about writing code.  Besides, if you search the forum for combining two sketches (or similar) you will see that the answers are unique for each situation.

One of the drawbacks to this setup is you need to run a Mega, or some other board that has at least 19 digital I/O ports (6 for ethernet, 7 for my keypad, 6 for the LCD).

Well, since you are using the Adafruit lcd library you can share the data pins of the LCD with the ROW pins of the keypad.  That will let you recover four more pins.

[really cool]

The code's like 1K+ lines long

That's OK.  It still only took 5 minutes to find a problem.  When you first wrote you said that readButtons() was peppered all through the code.  That isn't really much of a problem.  The real problem is that it is stuck inside a while(true) loop:

Code:

while (1)   //  Do forever loop.
{
    readButtons();
    // Do more stuff.
}

This means that getKey() isn't going to be called at all and you won't get any keypresses.  To fix it (I think) you will need to change this:

Code:

#include "keypad.h"
//  kpd = keypad setup, blah...

//  char key;    <------- Remove this.

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

void loop ()  {
    //    key = kpd.getKey();     <----- Remove this.
    //    readButtons();              <----- Remove this.
    mainMenu();    <----- This is already in your loop and should stay.
}

 void readButtons()  {
    // Clear the button states before checking for a new keypress.
    buttons[buttonUp] = LOW;
    buttons[buttonDown] = LOW;
    buttons[buttonSelect] = LOW;
    buttons[buttonBack] = LOW;

    char key = kpd.getKey();     // <---------- Add this.

    switch (key) {
        case 'U':       // <-- Change all these to match your keypad characters.
            buttons[buttonUp] = HIGH;
            break;
        case 'D':
            buttons[buttonDown] = HIGH;
            break;
        case 'S':
            buttons[buttonSelect] = HIGH;
            break;
        case 'B':
            buttons[buttonBack] = HIGH;
            break;
    }
}

OK, I've been going through the code as I write and it needs to be said that it's not a very good idea to include screen updates (lcdPrint), button testing, and menu selection all in the same function.  The problem is that the keypad library expects to be called quite frequently, at least a few hundred times per second, but that will cause the LCD to flicker or more likely to fade out.  Looking through the code you'll notice delay()'s of 150 and 250 milliseconds are sprinkled everywhere.  This will make the keypad buttons very sluggish. But if you remove the delay()'s it will mess with the LCD.  The trick is to move the LCD display code to its own function and only update the LCD when the information to be displayed changes.

I truly hope I am not discouraging you as this looks like a really cool project and I would hate to see you give up on it.    I just need to warn you against using all those delay()'s.  But if you following my warning then it is going to mess up your display.   You might be able to reach a happy balance for testing by adjusting the length of the delay()'s but rewriting how the information is displayed would make it a lot easier to change the code in the future without breaking it.

and it threw an error: "  'key' was not declared in this scope".

key is expected to be a char.  Since I didn't have the rest of your code I was only guessing that you might have something like the following:

Code:

#include "keypad.h"
//  kpd = keypad setup, blah...

char key;    // Temporarily stores the active key.

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

void loop ()  {
    key = kpd.getKey();
    readButtons();    // Uses 'key' internally. It might be better to pass a parameter.

    // back button
    if (buttons[buttonBack] == HIGH) return;
    // up button 
    if (buttons[buttonUp] == HIGH) CursorPrevious(DiagMenu, 5);
    // down button
    if (buttons[buttonDown] == HIGH) CursorNext(DiagMenu, 5);
}

 void readButtons()  {
    // Clear the button states before checking for a new keypress.
    buttons[buttonUp] = LOW;
    buttons[buttonDown] = LOW;
    buttons[buttonSelect] = LOW;
    buttons[buttonBack] = LOW;

    switch (key) {
        case '2':       // <-- Change all these to match your keypad characters.
            buttons[buttonUp] = HIGH;
            break;
        case '8':
            buttons[buttonDown] = HIGH;
            break;
        case '6':
            buttons[buttonSelect] = HIGH;
            break;
        case '4':
            buttons[buttonBack] = HIGH;
            break;
    }
}

And thanks for the very nice PM.

I'm currently trying to retrofit an existing program to use a keypad instead of buttons. I know the keypad works, thanks to everyone who assisted me in a previous thread. 

Right now, this function is being called just about everywhere:

Code:

void readButtons()
{
  buttons[buttonUp] = digitalRead(A3);
  buttons[buttonDown] = digitalRead(A0);
  buttons[buttonSelect] = digitalRead(A2);
  buttons[buttonBack] = digitalRead(A1);
}

You might be able to make all the rest of the code work correctly if you just change readButtons() to:

Code:

void readButtons()
{
    // Clear the button states before checking for a new keypress.
    buttons[buttonUp] = LOW;
    buttons[buttonDown] = LOW;
    buttons[buttonSelect] = LOW;
    buttons[buttonBack] = LOW;

    switch (key) {
        case 'U':       // <-- Change all these to match your keypad characters.
            buttons[buttonUp] = HIGH;
            break;
        case 'D':
            buttons[buttonDown] = HIGH;
            break;
        case 'S':
            buttons[buttonSelect] = HIGH;
            break;
        case 'B':
            buttons[buttonBack] = HIGH;
            break;
}

OK, so here's the problem.  keypadEventListener, a callback routine, was written for the keypad library when only a single key was allowed.  If you pressed more than one key at a time then everything after the first key was ignored.

When I rewrote the library to support multiple keypresses I created a Key class that lets each key store its own character, code, changed status, and state.  So now if you use getKeys() in your loop instead of getKey() then you will have a list that stores up to 10 active keys.

I mainly kept keypadEventListener for backwards compatibility with sketches that were written before the multi-key update.  If I were to change

Code:

keypadEventListener(key[0].kchar);

to

Code:

keypadEventListener(key[n].kchar);

then any sketches using keypad events and the single-key getKey() function would be broken.  But I still wanted the callback to occur on every state change for every key.  So if you use the multi-key version, getKeys(), you can still use keypad events but you'll just have to ignore the key character.

Another option is to create a different addEventListener function that works with multiple key presses.

Why is allways key[0] passed and not the key generating the event ?

Well, probably because somebody made a mistake when they refactored the library. The original library only allowed single key presses and I later modified it to allow multiple key presses at the same time.

I'm off to bed right now so I will take a look at your suggested changes tomorrow.

I'm so glad I'm done for the week.

I'm really glad to hear it.    Let me know if you have any more trouble.  It's been kind of interesting.

However, I still have the same problem. Now, I'm reading about some keypads needing pull up resistors, and I'm going to try that and report back.

No need.  The library automatically uses the internal pull-up resistors on the uController.  What I'm really concerned about is why are you getting the strange characters.  I've seen that problem when someone uses the wrong baud rate in their serial terminal.  Can you make sure you have your code and serial monitor set to the same value?

Also, did you check to see which Arduino version you are using?

Now, the oddness as seen off the USB:

• Most of the buttons don't work.
• When I press 4, I get ~
• When I press 7, I get '
• When I press 8, I get f
• When I press 9, I get x

I know the keypad works because I tested continuity on all pins. The odd results are what's puzzling. Where is it getting that information? Did I use the wrong library? Any help would be appreciated. 

This is not a normal failure mode for the keypad library.  Even if you wired it up completely wrong you would not get those strange characters. Unless of course if you are plugged into the serial pins 0 or 1.  Also, are you running version 1.0.3 of the Arduino software?  If so, could you download 1.0.4 and give it a try.  http://arduino.cc/en/Main/Software

Here's a first attempt at making the code a bit more logical.  It's not nearly as optimized as it could be but I wanted to start off with something at least slightly understandable.  Be warned, I haven't tried to run it. 

Code:

#include <Keypad.h>

const byte ROWS = 4; //four rows
const byte COLS = 3; //three columns
char keys[ROWS][COLS] = {
    {'1','2','3'},
    {'4','5','6'},
    {'7','8','9'},
    {'*','0','#'}
};
byte rowPins[ROWS] = {2, A5, A4, A3}; //connect to the row pinouts of the kpd
byte colPins[COLS] = {A2, A1, A0}; //connect to the column pinouts of the kpd

Keypad kpd = Keypad( makeKeymap(keys), rowPins, colPins, ROWS, COLS );

int Out1=3;
int Out2=4;
int Out3=5;
int Out4=6;
int Out6=7;
int Out7=8;
int Out8=9;

byte output_pins = B11111111;  // Clear game controller pin control buffer.
byte old_output_pins;    // Optimization for setControllerPins().

void setup() {
    Serial.begin(9600);
    kpd.setDebounceTime(1);  // Set debounce to minimum.  Should improve key response.

    pinMode(Out1, OUTPUT);
    pinMode(Out2, OUTPUT);
    pinMode(Out3, OUTPUT);
    pinMode(Out4, OUTPUT);
    pinMode(Out6, OUTPUT);
    pinMode(Out7, OUTPUT);
    pinMode(Out8, OUTPUT);
   
    old_output_pins = output_pins;
    setControllerPins(output_pins, old_output_pins);    // Initialize all controller pins HIGH.
}

void loop() {
   
    // In here we fill the pin control buffer with 8 bits where each bit represents a pin on the
    // game controller. For example the fourth bit equals pin 4 on the game controller, etc.
    // Bits that are 0's are the active pins.
    if (kpd.getKeys())
    {
        if( kpd.findInList('1') ) {
            output_pins & B11010111;  // controller pins 4 and 6, outpin 5 is never used and will be ignored.
        }
        if( kpd.findInList('2') ) {
            output_pins & B10110111;    // controller pins 4 and 7
        }
        if( kpd.findInList('3') ) {
            output_pins & B01110111;    // controller pins 4 and 8
        }
        if( kpd.findInList('4') ) {
            output_pins & B11011011;    // controller pins 3 and 6
        }
        if( kpd.findInList('5') ) {
            output_pins & B10111011;    // controller pins 3 and 7
        }
        if( kpd.findInList('6') ) {
            output_pins & B01111011;    // controller pins 3 and 8
        }
        if( kpd.findInList('7') ) {
            output_pins & B11011101;    // controller pins 2 and 6
        }
        if( kpd.findInList('8') ) {
            output_pins & B10111101;    // controller pins 2 and 7
        }
        if( kpd.findInList('9') ) {
            output_pins & B01111101;    // controller pins 2 and 8
        }
        if( kpd.findInList('*') ) {
            output_pins & B11011110;    // conrtoller pins 1 and 6
        }
        if( kpd.findInList('0') ) {
            output_pins & B10111110;    // conrtoller pins 1 and 7
        }
        if( kpd.findInList('#') ) {
            output_pins & B01111110;    // conrtoller pins 1 and 8
        }
       
        setControllerPins(output_pins, old_output_pins);
        old_output_pins = output_pins;    // remember last controller pin states.
        output_pins = B11111111;  // Clear pin control buffer.
    }
} // End loop

// This function must be optimized to reduce the number of calls to digitalWrite().
// That is what old_state does. Also, a time limit of 1 mSec would really help, too.
void setControllerPins(byte new_state, byte old_state)
{
    // If bitRead(newState) returns a 1 then the pin will be set HIGH, else LOW.
    if( bitRead(new_state, 0) != bitRead(old_state, 0) ) {
        digitalWrite(Out1, bitRead(new_state, 0));
    }
    if( bitRead(new_state, 1) != bitRead(old_state, 1) ) {
        digitalWrite(Out2, bitRead(new_state, 1));
    }
    if( bitRead(new_state, 2) != bitRead(old_state, 2) ) {
        digitalWrite(Out3, bitRead(new_state, 2));
    }
    if( bitRead(new_state, 3) != bitRead(old_state, 3) ) {
        digitalWrite(Out4, bitRead(new_state, 3));
    }
    if( bitRead(new_state, 5) != bitRead(old_state, 5) ) {
        digitalWrite(Out6, bitRead(new_state, 5));
    }
    if( bitRead(new_state, 6) != bitRead(old_state, 6) ) {
        digitalWrite(Out7, bitRead(new_state, 6));
    }
    if( bitRead(new_state, 7) != bitRead(old_state, 7) ) {
        digitalWrite(Out8, bitRead(new_state, 7));
    }
}

OK, I'm back and ready to play.

I'll take a look at the code again tonight but I think you were right about ANDing the keys together.  I think it will work to find which keys are active on the list, build an 8 bit value from all the keys ANDed together, and then have a function manage the HIGH/LOW states of each of each of the 8 controller lines.

It looks like it might require a little more creative thinking than that.  The problem is the controller pins are shared and you just found out that means you can't just send a pin HIGH without first checking if it is being used by another button.

I have a chem2 final monday night so I can't give much effort until afterwords.

However, the controller has 8 pins so first I would create an 8 bit register where each bit represents a pin. Then the buttons could be read to see which ones are pressed. Use that info to build a bitmask.  Finally, write a function that uses that bitmask to decide which pins get set HIGH or LOW.

Sorry, best I can do at the moment.  If you haven't figured it out by tuesday then I'll be back after that to help out some more.

[thought]

I'm using the keypad as input for Intellivision video game and some people might press the buttons very quickly. Any thoughts on how to adjust for that?

You might need to set the debounceTime to a lower value.  The default is 10 mSec which I thought would be fast enough.  A fast typist can only hit a key once every 20 mSec. You may have just proved me wrong.   Try the following:

Code:

void setup() {
      kpd.setDebounceTime(1);  // Set debounce to minimum.  Should improve key response.
}

If that doesn't work then we can take a closer look at what is happening before trying anything else.