How can i simplify this function?

[otuzsubat]

I use a 4x3 keypad in my project. I use C0, C1, C2 and C5 pins as output, and E0,E1,E2 as inputs. Can this function be more simply?

char one_char(void){
unsigned int row;
for (row=0; row<4;row++){//"row"
if (row==0){
output_low(pin_c0);
output_high(pin_c1);
output_high(pin_c2);
output_high(pin_c5);
}
if (row==1){
output_high(pin_c0);
output_low(pin_c1);
output_high(pin_c2);
output_high(pin_c5);
}
if (row==2){
output_high(pin_c0);
output_high(pin_c1);
output_low(pin_c2);
output_high(pin_c5);
}
if (row==3){
output_high(pin_c0);
output_high(pin_c1);
output_high(pin_c2);
output_low(pin_c5);
}
if(!input(PIN_E0)&&input(PIN_E1)&&input(PIN_E2))
return((row*3)+1+0x30;
else if (input(PIN_E0)&&!input(PIN_E1)&&input(PIN_E2))
return((row*3)+2+0x30);
else if (input(PIN_E0)&&input(PIN_E1)&&!input(PIN_E2))
return((row*3)+3+0x30);
else
return(0);
}

 

[checkmate]

Wouldn't help much to simplify it, since human fingers are very much slower than the uC.

Anyway, assuming that the other pins of port C are not output pins, you could simply output 0x17 to port C at the end of the for loop, instead of having so many output_high. (Provided you disable CCS's auto-TRIS setting of course)

When testing for columns, if you can ensure that the user only presses one key at a time, you can just test for that single pin.

Try using row+row+row or row+=(row<<1) instead of row*3.

BTW, have you tried the code before? Do you have an external switch debouncing circuit? Because it's not done in software.

Cheers!

 

[otuzsubat]

I use the other pins of C. In fact, i want to know how i can set or clear some pins at one time without changing the other pins.

Meanwhile, i applied your sugestion and my function runs faster now.

 

[otuzsubat]

BTW, have you tried the code before? Do you have an external switch debouncing circuit? Because it's not done in software.

I will add codes for debouncing, but before this i want to reduce the size of function.

 

[artem]

You can do some tricks .
As example for return part :

change

if(!input(PIN_E0)&&input(PIN_E1)&&input(PIN_E2))
return((row*3)+1+0x30;
else if (input(PIN_E0)&&!input(PIN_E1)&&input(PIN_E2))
return((row*3)+2+0x30);
else if (input(PIN_E0)&&input(PIN_E1)&&!input(PIN_E2))
return((row*3)+3+0x30);
else
return(0);

to :

const unsigned char conversion[8] = {0,0,0,1,0,2,3,0};

return (conversion[input(port_E)&0x07] == 0 ? 0 : (conversion[input(port_E)&0x07] + 0x30 +row*3));


Did not check for correctness, but concept is OK .

 

[barny451]

Assuming that you can output to all four bits of port c in one output operation, you could change from row++ between loops, to row<<=1, and output the bit-wise inverse of row onto port C:

for ( row = 1 ; row < 32 ; row <<=1 )
{
output( port_c, ~row )
...
}

The keycodes returned change, but you presumably have code to convert keycodes to digits (or whatever is on the key), so this could be adapted.

HTH
Barny

 

[otuzsubat]

I changed my function as below. I seperated it to two functions.
The basic changes are i dont use "for" loop for the variable "row" and instead of return((row*3)+3+0x30); i use tus_degeri=(row+=(row<<1)+0x33);

char tus_degerlendir(unsigned int row){
byte tus_degeri;
if(!input(PIN_E0)&&input(PIN_E1)&&input(PIN_E2)){
tus_degeri=(row+=(row<<1)+0x31);
if(tus_degeri==0x3A)
tus_degeri=0x2A;
return (tus_degeri);
}
if (input(PIN_E0)&&!input(PIN_E1)&&input(PIN_E2)){
tus_degeri=(row+=(row<<1)+0x32);
if(tus_degeri==0x3B)
tus_degeri=0x30;
return (tus_degeri);
}
if (input(PIN_E0)&&input(PIN_E1)&&!input(PIN_E2)){
tus_degeri=(row+=(row<<1)+0x33);
if(tus_degeri==0x3C)
tus_degeri=0x23;
return (tus_degeri);
}
return (0x00);
}
char tus_tara_degerlendir(void){
byte temp,row;
row=0;
output_low(pin_c0);
output_high(pin_c1);
output_high(pin_c2);
output_high(pin_c5);
temp=tus_degerlendir(row);
if (temp!=0)
return (temp);
row=1;
output_high(pin_c0);
output_low(pin_c1);
output_high(pin_c2);
output_high(pin_c5);
temp=tus_degerlendir(row);
if (temp!=0)
return (temp);
row=2;
output_high(pin_c0);
output_high(pin_c1);
output_low(pin_c2);
output_high(pin_c5);
temp=tus_degerlendir(row);
if (temp!=0)
return (temp);
row=3;
output_high(pin_c0);
output_high(pin_c1);
output_high(pin_c2);
output_low(pin_c5);
temp=tus_degerlendir(row);
if (temp!=0)
return (temp);
return(0x00);
}

 

[cherrytart]

You could also set all the output pins high then select which one to set low with a case statement.

But for this program that mostly saves typing. It may also be more compact code, but thats not always so when the number of cases is small.

 

[barny451]

After setting the pin_c ports to known state at the start of the function, you only have to write to the ones that change:

char tus_tara_degerlendir(void){
byte temp,row;
row=0;
output_low(pin_c0);
output_high(pin_c1);
output_high(pin_c2);
output_high(pin_c5);
temp=tus_degerlendir(row);
if (temp!=0)
return (temp);
row=1;
output_high(pin_c0);
output_low(pin_c1);
// NOT NEEDED output_high(pin_c2);
// NOT NEEDED output_high(pin_c5);
temp=tus_degerlendir(row);
if (temp!=0)
return (temp);
row=2;
// NOT NEEDED output_high(pin_c0);
output_high(pin_c1);
output_low(pin_c2);
// NOT NEEDED output_high(pin_c5);
temp=tus_degerlendir(row);
if (temp!=0)
return (temp);
row=3;
// NOT NEEDED output_high(pin_c0);
// NOT NEEDED output_high(pin_c1);
output_high(pin_c2);
output_low(pin_c5);
temp=tus_degerlendir(row);
if (temp!=0)
return (temp);
return(0x00);
}