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
}
}


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.

• What happens with your real hardware if you tie RD7 to ground via a 10K considering your TRIS registers mean you're trying to read it? – PeterJ Jan 2 '14 at 10:23

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
}
}

• Still not working :( still Logic contention occurs on pressed pins in simulation. – Farid-ur-Rahman Jan 18 '14 at 19:59
• Try with the latest code. In addition to adding delays for the Schmitt triggers, I also drive the lines low as a poor man's pulldown. – user35740 Jan 18 '14 at 21:31

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;


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
}
}

• Thanks for the answer. I have tried the code with delays of 5us and 5ms but problem is still the same. – Farid-ur-Rahman Jan 2 '14 at 9:58
• With the pull down resistors the code also not working in simulation – Farid-ur-Rahman Jan 2 '14 at 10:05
• @Farid-ur-Rahman I have made an edit to my answer. – abdullah kahraman Jan 2 '14 at 13:38

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.

• This is an extremely unlikely case. However, it's similarly extremely easy to check: look at the assembly output of the C compiler (.LST), and there you can quickly verify if the compiler used the correct banks. – Laszlo Valko Jan 15 '14 at 3:25
• Not knowing anything about the compiler, I wouldn't make any assumptions. However, this could explain the behavior described, and also the difference between the compiler and the simulator. Like you said, it's easy enough to check, and I would check it, if just to rule it out. – user28910 Jan 15 '14 at 15:05
• So Whats the solution you suggest? I am using Hi-Tech PICC Pro compiler. – Farid-ur-Rahman Jan 15 '14 at 19:33
• @Farid-ur-Rahman Could you paste the content of your compiled listing file to pastebin.com or similar website and add to your question then notice the answerer? – abdullah kahraman Jan 18 '14 at 19:00