problem with c code for pic

[little_carlos]

Hello guys
im doing a counter from 1 to 4 using one push button to increase the count and another to decrease and a 7 segment display
the problem i have is that it only displays either 1 or 4, but never 2 or 3 when increasing or decreasing, it jumps directly after pushing the button a few times, what is wrong with the code?
btw, i have the problem on proteus and on board, its a pic 16f628a using the internal clock at 4mhz.

#define uno PORTB = 0x06;
#define dos PORTB = 0x5B;
#define tres PORTB = 0x4F;
#define cuatro PORTB = 0x66;
#define cinco PORTB = 0x6D;
#define seis PORTB  = 0x7D;
#define siete PORTB = 0x07;
#define ocho PORTB = 0x7F;
#define nueve PORTB = 0x6F;
#define cero PORTB = 0x3F;
int i;
void main() {
 TRISB = 0;
 PORTB = 0;
TRISA = 1;
PORTA = 0;
 i = 0;

 while(1){
if(RA0_bit == 1){
i ++;

}
if(RA1_bit == 1){
i --;
}
if(i == 1){
uno;
}
if(i == 2){
dos;
}
if(i == 3){
tres;
}
if(i == 4){
cuatro;
}

}
}

[Feynman]

Did you debounce your button?

[Paul Moir]

Physical switch contacts bounce when they're closed.  The micro is fast enough to count the switch bounces.

An easy way to resolve this is to ignore the button for a few milliseconds after it's state changes.  Many other solutions exists.  The google search term you want is "debounce"

Minor and you probably already know, but you haven't accounted for i below 1 or above some number.  Also c has a feature called "case" for testing a variable against a bunch of possibilities.  For example

switch(i) {
case 1:  uno;
break;
case 2: dos;
break;
}

Or you can use an array:

Code: [Select]

const byte numbers[] = {0x00, 0x06, 0x5B 0x4F, 0x66};

...

if ((i >= 0) && (i <= 4)) {
PORTB = numbers[i];
}


[little_carlos]

Physical switch contacts bounce when they're closed.  The micro is fast enough to count the switch bounces.

An easy way to resolve this is to ignore the button for a few milliseconds after it's state changes.  Many other solutions exists.  The google search term you want is "debounce"

Minor and you probably already know, but you haven't accounted for i below 1 or above some number.  Also c has a feature called "case" for testing a variable against a bunch of possibilities.  For example

switch(i) {
case 1:  uno;
break;
case 2: dos;
break;
}

Or you can use an array:

Code: [Select]

const byte numbers[] = {0x00, 0x06, 0x5B 0x4F, 0x66};

...

if ((i >= 0) && (i <= 4)) {
PORTB = numbers[i];
}


oh now i get it! thanks man I owe you some tacos 

[Howardlong]

Also, I assume you want

TRISA=3;

assuming you want RA0 up and RA1 down.

[dannyf]

1 I would change the macros defined at beginning of your code.
2. A switch case statement would be much better.

[hamdi.tn]

it's too obvious ... with your code, the while loop will run so many times between the time you press the button and the moment you release it, even if no bouncing occur. basiclly it will count correctly from 1 to 4 in few micro second without giving you the time to notice that .
what you should do is
1- press detection based on interrupts and denouncing
2- blocking code , so when you press the button your code block the loop until you release the button.

[nctnico]

NO press detection using interrupts! Never ever use interrupts for that because a flaky switch will make your microcontroller stop into a grinding halt producing elusive problems. Whoever advices to use interrupts for buttons directly should be shot to death and then banned from this forum forever!

The best way is to have a timer running, use the counter from that timer in the main loop to see if a certain time period has elapsed (say 50 times per second). When the switch is pressed, have a counter count up, when the switch is open reset that counter. When the counter reaches a certain count (say 5 or 10) you can be certain the button has been pressed.

[Paul Moir]

it's too obvious ...

You're right, it was too obvious.  I completely missed that! 

[hamdi.tn]

NO press detection using interrupts! Never ever use interrupts for that because a flaky switch will make your microcontroller stop into a grinding halt producing elusive problems. Whoever advices to use interrupts for buttons directly should be shot to death and then banned from this forum forever!

The best way is to have a timer running, use the counter from that timer in the main loop to see if a certain time period has elapsed (say 50 times per second). When the switch is pressed, have a counter count up, when the switch is open reset that counter. When the counter reaches a certain count (say 5 or 10) you can be certain the button has been pressed.

now that am banned and dead  man you left nothing for isis to do to me this been discussed  so many times in this forum particularly  in a thread about interrupt vs pooling one.

1- it's always case depending
2- if there is no drama about making a small holding time after an interrupt been detected then test the button level by pooling it .it will have the same effect of what you said
3- i will find this a good idea on two cases : first if you can couple a switch interrupt with a timer interrupt that test the switch logic level after a period elapsed as you suggested. Or two if you can couple the button to a timer with gate control so where the timer reach a certain value is consider the button pressed ( for this pic you have only one timer (Timer 1) and one gate control. for both cases mcu will have minimum involvement.
4- this debate will continue as long as interrupt and pooling exists so no need to waste time about it really

[nctnico]

If you use interrupts for a button (or any switch) the interrupt will never be activated for an actual keypress. First of all all switches bounce so you'll have a nice switch bounce detector and secondly the interrupt will fire for every spike induced in the wiring. All in all using interrupts for a slow event like a button just causes extra overhead (asynchronous processes!) and wasted CPU cycles to serve completely useless interrupts.

[hamdi.tn]

If you use interrupts for a button (or any switch) the interrupt will never be activated for an actual keypress. First of all all switches bounce so you'll have a nice switch bounce detector and secondly the interrupt will fire for every spike induced in the wiring. All in all using interrupts for a slow event like a button just causes extra overhead (asynchronous processes!) and wasted CPU cycles to serve completely useless interrupts.

well  is true. but wasting cpu cycle is inevitable. what you propose is to make it in a well know controlled fashion and i can't denies the benefit of that.

[dannyf]

Code: [Select]

PORTB = numbers[i];
I would say that that is generally not the right approach. In most cases, you may have other pins on the same port so writing to that entire port may not be desirable.

If you have to store the information in an array, I would use set or clear the pins to be lit:

Code: [Select]

LED_PORT |= led_font[index];
Obviously, you will have to clear LED_PORT first.