help needed with PIC programming

Thread Starter

Eyeru

Joined Mar 5, 2013
4
Hi all,

I am currently working on a PIC programming project. I am new to embedded programming, so please bear with me if I come across as an idiot. My project is to program a user interface for a power supply unit, this includes a 3x4 matrix keypad and multiplexed 7segment LEDs. My code includes a function that scans the keypad and verifies the key pressed using polling, and an array with the decoded values to drive the 7 segment displays. My code works fine and displays the correct key pressed on one digit but I can't seem to get how to store the keypresses and display them on the multiplexed displays and shift the displayed numbers to the left as the user enters the 2nd and 3rd...digit numbers

Below is the function that finds the key pressed and sends value to PORTC where the 7segment displays are connected

Rich (BB code):
unsigned char find_key(void)
{
    while (1)
    {
        PORTB &= 0x00;
        PORTBbits.RB5 = 1; //set row 1 high
        wait_1ms ();
        
        if (PORTBbits.RB1)
        {
            wait_5ms();
          if (PORTBbits.RB1)
            key = pat7seg[7]; //send '7' to portc to drive sevenseg display
        }
        if (PORTBbits.RB2)
        {
            wait_5ms();
          if (PORTBbits.RB2)
            key = pat7seg[8]; //send '8'
        }
        if (PORTBbits.RB3)
        {
             wait_5ms();
           if (PORTBbits.RB3)
            key = pat7seg[9]; //send '9'
        }
        PORTB &= 0x00;
        PORTBbits.RB4 = 1;  //set row 2 high
        wait_1ms ();
        
        if (PORTBbits.RB1)
        {
           wait_5ms();
          if (PORTBbits.RB1)
            key = pat7seg[4]; //send '4'
        }
        if (PORTBbits.RB2)
        {
            wait_5ms();
          if (PORTBbits.RB2)
            key = pat7seg[5]; //send '5'
        }
        if (PORTBbits.RB3)
        {
            wait_5ms();
           if (PORTBbits.RB3)
            key = pat7seg[6]; //send '6'
        }
        PORTB &= 0x00;
        PORTBbits.RB6 = 1;   //set row3 high
        wait_1ms ();
        
        if (PORTBbits.RB1)  //check column 1
        {
           wait_5ms();
          if (PORTBbits.RB1)
            key = pat7seg[1]; //send '1'
        }
        if (PORTBbits.RB2)
        {
            wait_5ms();
          if (PORTBbits.RB2)
            key = pat7seg[2]; //send '2'
        }
        if (PORTBbits.RB3)
        {
            wait_5ms();
           if (PORTBbits.RB3)
            key = pat7seg[3]; //send '3'
        }
        PORTB &= 0x00;
        PORTBbits.RB7 = 1;  //set row4 high
        wait_1ms ();
        
        if (PORTBbits.RB1)  //is column 1 pressed?
        {
           wait_5ms();
          if (PORTBbits.RB1)
           key = pat7seg[10]; //send dp
        }
        if (PORTBbits.RB2)
        {
             wait_5ms();
          if (PORTBbits.RB2)
            key = pat7seg[0]; //send '0'
        }
        if (PORTBbits.RB3)
        {
            wait_5ms();
           if (PORTBbits.RB3)
            key = ENTER; //send 'H'
        }
     }
    return key;
}
below is the part of the code I thought would display numbers on the 2 multiplexed 7segment displays but it doesn't work

Rich (BB code):
do
    {
        clearDisplay ();
        
        
        Keypressed = find_key();
        while (Keypressed != 0)
        if (Keypressed == ENTER)
                break;
            Keypressed++;
        Dig1 = Keypressed % 10;  //ones digit
        Dig1 = pat7seg[Dig1];
        Dig2 = Keypressed / 10;
        Dig2 = pat7seg[Dig2];
        for (i = 0; i <= 1; i++)
        {
            PORTC = Dig1;
            PORTDbits.RD6 = 0;
            PORTDbits.RD7 = 1;
            wait_5ms ();
            PORTC = Dig2;
            PORTDbits.RD6 = 1;
            PORTDbits.RD7 = 0;
            wait_5ms ();
        }
 
 }
    
    while (1);
Can you please give me some suggestions as to how I can improve this, thanks a lot in advance.
 
Last edited by a moderator:

John P

Joined Oct 14, 2008
2,025
First of all, congratulations on your excellent clear formatting. When we see someone who wants help with their code here, and it's a mess to wade through, it's so tempting to say, "Oh, this guy isn't doing anything to make it easy to help him. What else is there to look at?"

What your clear formatting makes it easy to see is that the button sensing is done in a while(1) loop which seems to have no exit. How's anything meant to happen? My suggestion is that in main() you should call the find_key() routine, which will return a code for any key pressed, or some null value (0 or 0xFF?) if nothing is pressed. Then if there's a keypress, act on it and if not, do other things and loop.
 

MrChips

Joined Oct 2, 2009
30,707
As far as I can tell I believe your code is stuck here:

Rich (BB code):
        while (Keypressed != 0)
        if (Keypressed == ENTER)
                break;
This is the same as:
Rich (BB code):
        while (Keypressed != 0) if (Keypressed == ENTER);
Use this instead:
Rich (BB code):
        while (1)
          {
            Keypressed = find_key();
            if (Keypressed)
            {

            }
           
          }
 
Last edited:

Thread Starter

Eyeru

Joined Mar 5, 2013
4
First of all, thanks for your encouraging words, JohnP. Secondly, that while(1) loop is in main (), it was meant to be a do...while() loop and does have a curly bracket after it if that's what you mean by an exit. And I do call find_key () in main() as follows:

Keypressed = find_key();

But how do I go about storing the next number enterred?

can while (Keypressed != 0) work as a condition to verify if a key has been pressed? as in the following code

Keypressed = find_key();
while (Keypressed != 0)
if (Keypressed == ENTER)
break;
Keypressed++;

and would the line Keypressed++; ensure that the system keeps on obtaining pressed keys till ENTER key is pressed?
 

Thread Starter

Eyeru

Joined Mar 5, 2013
4
Thanks @ Mr Chips, I see what you mean by my code being stuck at break;

Keypressed = find_key();

works for one digit, but as I said above, can you please help me with how to go on with the 2nd digit entered?
 

John P

Joined Oct 14, 2008
2,025
I meant that the find_key() routine has its own while(1) loop; that's the first line in the function, and I don't see any break or return statement to get you out of it. Also the variable "key" is presumably a global, since it isn't defined in the function, so there's no need to return it; all you need is to set it. But make sure you can tell the difference between a key-pressed situation and no key pressed.
 

MrChips

Joined Oct 2, 2009
30,707
Your problem is not just simply how to code a problem but it is also about creating an algorithm.

Before you start writing code I suggest you map out on a piece of paper the program algorithm using either pseudo-code or flowchart, or both.

What you need to do is:

1) establish the difference between key pressed and no key pressed,

2) convert from a key-code to a key digit and key function and detecting the differences,

3) establish a digit pointer indicating which digit (unit) is being addressed,

4) establish what to do when the ENTER key is pressed repeatedly.
 

Thread Starter

Eyeru

Joined Mar 5, 2013
4
Thanks a lot again, guys. I had a flow chart mapped out when I first started, but I am looking at expanding on it now, good tip there, thanks. @JohnP, if I try to build the code without the return key; statement, I get a warning of an implicit return to a function. What do you mean by I only need to set the global variable 'key'? set it like key = 1;?
And how can I differentiate between a key press and no key press state....I though the conditional statement;

if (Keypressed)
{

}

meant 'is there a key pressed? If so do something...' otherwise it's a no-key-pressed state by default

Another question can I use the following structure to store the values of the four digits to be displayed?

struct Digit_sel
{
unsigned Digit1 : 4;
unsigned Digit2 : 4;
unsigned Digit3 : 4;
unsigned Digit4 : 4;
}
Digit_selbits;
 

MrChips

Joined Oct 2, 2009
30,707
If you don't return key then find_key(void) should be declared as:

void find_key(void)

What you have written is ok except good programming practice would dictate that key should be declared as local:

Rich (BB code):
unsigned char find_key(void)
{
  unsigned char key;

 return key;
}
You need to remove the while(1) from inside find_key( ).
 
Top