Interfacing Keypad to PIC16F877A

Question

I am interfacing a 3X4 keypad to PIC16f877a uC on PORTD as:

In the first half of the C program I have set first four pins to Input (From LSB-RD0) and others to Output and store the value in a variable and in other half I have set last four pins to Input and others to Output and stored its value into another variable I have called this function in a continuous loop to get a pressed key. The code is as follows:

int getKey(void)
{
    int toRet = 0;
    int firstHalf = 0, secHalf = 0; 
    TRISD = 15, PORTD = 240;
    firstHalf = PORTD;
    TRISD = 240, PORTD = 15;
    secHalf = PORTD;
    switch (-((firstHalf + secHalf)-255))
    {
        case 20: toRet = 7;   break; //1
        case 36: toRet = 8;   break; //2
        case 68: toRet = 9;   break; //3
        case 18: toRet = 4;   break; //4
        case 34: toRet = 5;   break; //5
        case 66: toRet = 6;   break; //6
        case 17: toRet = 1;   break; //7
        case 33: toRet = 2;   break; //8
        case 65: toRet = 3;   break; //9
        case 40: toRet = 0;   break; //0
        case 24: toRet = 10;  break; //*
        case 72: toRet = 11;  break; //#
        default: toRet = 255; break; //null
    }
    return toRet;
}

The code is running fine on simulator but not on hardware it even doesn't generate any random key pressed. I am not using any pull up or pull down resistor because I think it may disturb the input mechanism. What should I do to solve this problem?

EDIT: After abdullah kahraman's answer I tried adding pull down resistors of 10k ohm on all keypad pins but the problem is stil the same. Also in simulation (Proteus ISIS simulator) the code runs fine with a warning Logic contention detected on net on both pins input and output(+5V from uC) at that time. I am not using any pull resistor in simulation.

Answer 1

The switch statement looks overly complicated, and at least to me wasn't immediately obvious what it was doing. I would go with a more straightforward implementation.

Like Abdullah said, delays after you set TRISD and PORTD registers may help, as well as debouncing the input.

Finally, I would get a o-scope and/or multimeter to probe on these pins to make sure everything is connected properly, toggling as expected, and make sure the keypad behaves as expected.

EDIT:

I noticed that there are internal Schmitt Triggers on PORTD inputs that are enabled by default. This will inject some latency, though I'm not sure how much, as the datasheet doesn't say.

EDIT2 :

Added hard delays before sampling and capacitive charge removal

int getKey(void)
{
    // Like Abdullah said, these can be bytes
    // Also, you can save image space by not initializing these
    unsigned char toRet;
    unsigned char firstHalf, secHalf;

    // Clear lines (attempt to get rid of capacitive charge)
    TRISD     = 0xff; // Hop off
    PORTD     = 0x00; // Prepare to drive low
    TRISD     = 0x00; // Drive all lines
    __delay_us(10000);// Wait arbitrary about of time
    TRISD     = 0xff; // Stop driving while we change values

    // Drive first half
    PORTD     = 0xf0;
    TRISD     = 0x0f; // Hex makes things easier to read
    __delay_us(10000);
    firstHalf = (PORTD & 0x0f) >> 0; // Mask only the stuff we care about
    TRISD     = 0xff;

    // Clear lines
    PORTD     = 0x00;
    TRISD     = 0x00;
    __delay_us(10000);
    TRISD     = 0xff;

    // Drive second half
    PORTD     = 0x0f;
    TRISD     = 0xf0;
    __delay_us(10000);
    secHalf   = (PORTD & 0xf0) >> 4; // Mask only the stuff we care about
                                     // and shift the nibble we care about down
    TRISD     = 0xff;

    // Simple selection logic
    // Keep in mind that if multiple keys are pressed, this function will return "null"
    switch (firstHalf) {
        case 0x1:
            switch (secHalf) {
                case 0x1: toRet =   7; break; //1
                case 0x2: toRet =   8; break; //2
                case 0x4: toRet =   9; break; //3
                default:  toRet = 255; break; //null
            }
            break;
        case 0x2:
            switch (secHalf) {
                case 0x1: toRet =   4; break; //4
                case 0x2: toRet =   5; break; //5
                case 0x4: toRet =   6; break; //6
                default:  toRet = 255; break; //null
            }
            break;
        case 0x4:
            switch (secHalf) {
                case 0x1: toRet =   1; break; //7
                case 0x2: toRet =   2; break; //8
                case 0x4: toRet =   3; break; //9
                default:  toRet = 255; break; //null
            }
            break;
        case 0x8:
            switch (secHalf) {
                case 0x1: toRet =  10; break; //*
                case 0x2: toRet =   0; break; //9
                case 0x4: toRet =  11; break; //#
                default:  toRet = 255; break; //null
            }
            break;
        default:          toRet = 255; break; //null
    }
    return toRet;
}

Answer 2

You are probably having a Read-Modify-Write issue. You can Google this and find a lot information about it. Please do this before reading my answer.

To tell what is going on here, my guess is like follows:

TRISD = 15, PORTD = 240;
firstHalf = PORTD;
TRISD = 240, PORTD = 15;
secHalf = PORTD;

In the above code, you set some pins as input and send HIGHs to them with some pins that are set output. However, when you are doing this, you are very fast. Assume that we are doing this for only one pin using below pseudo code.

1. TRISD0 = 1;  // D0 is input.
2. TRISD1 = 0;  // D1 is output.
3. PORTD1 = 1;  // Set D1 HIGH. D1 is connected to D0 via a switch.
4. firstHalf = PORTD0; // Read the port D0 to firstHalf variable.

The time passes between line 3 and line 4, where you set and then read may be very short, depending on your CPU clock frequency. So, maybe, you are reading the pin before it gets HIGH. This could be true if the time passes between these two lines is about 500ns and you have long cables that have too much parasitic capacitance on them that delays the line.

The solution is to add a delay that is enough to make it working and then set 5 times of this delay to be on the safe side, which is as follows:

TRISD = 15, PORTD = 240;
__delay_us(5);
firstHalf = PORTD;
TRISD = 240, PORTD = 15;
__delay_us(5);
secHalf = PORTD;

Please read answers of this question.

I would put pull-down resistors to all 7 of the port pins (RD0 to RD6). That should not interfere with your code.

Off topic: Why are you using integers when you are working with an 8-bit microcontroller? You do not need 16 bits for variables toRet, firstHalf and secHalf. Also the value that your function will return is maximum of 8 bits. I would use "unsigned character" type variables for them.

Edit:

I didn't understand your statement inside switch, so I've re-organized it and it now works in real world but not in simulation. Also, I've put pull downs for pins RD0-RD6. However, this code does not have debouncing. You will have to make the debouncing by calling it multiple times and checking if the result from it changed, or in other way; check it until it does not change. Here is your new code:

unsigned char getKey(void)
{
unsigned char toRet = 0;
unsigned char firstHalf = 0, secHalf = 0;

TRISD = 15, PORTD = 240;
__delay_us(5);
firstHalf = PORTD;

TRISD = 240, PORTD = 15;
__delay_us(5);
secHalf = PORTD;
switch ((unsigned int)(firstHalf + secHalf)-255)
{
    case 19: toRet = 7;   break; //1
    case 51: toRet = 8;   break; //2
    case 67: toRet = 9;   break; //3
    case 18: toRet = 4;   break; //4
    case 50: toRet = 5;   break; //5
    case 66: toRet = 6;   break; //6
    case 30: toRet = 1;   break; //7
    case 62: toRet = 2;   break; //8
    case 78: toRet = 3;   break; //9
    case 40: toRet = 0;   break; //0
    case 24: toRet = 10;  break; //*
    case 72: toRet = 11;  break; //#
    default: toRet = 255; break; //null
}
return toRet;
}

Answer 3

It might be because the compiler is not aware that PORTD is in bank 0 and TRISD is in bank 1. On this device, special function registers are arranged in banks, and PORTD and TRISD (which configures the bits as input or output) are at the same address, 0x08, in bank 0 and bank 1, respectively.

There are bits in the STATUS register (at 0x03, mapped to all banks) that select the bank. These are bits 5 & 6, referred to as RP0 and RP1. The compiler may expect the programmer to manipulate these bits, or it may do it itself.

It is easy enough to check by examining the assembly output listing. Between assignments to TRISD and PORTD, there should be an instruction that clears the RP0 bit in the STATUS register. If there isn't, the programmer has to add code to handlethe bank switching.