Absolute Beginner Question/Help with PIC 12F683

[----AJ----]

Hi Guys,

I'm just starting out in the world of C and micro-controllers and I set myself a simple beginners task of using an external switch + counter + LED, all I want to do is:

When the external switch gets pressed, increment the counter by 1, when the counter gets to five blink the LED.

The code is below, and the problem seems to be that the counter is not behaving as expected, the LED does come on but at very random counts, sometimes after the button is pressed 3 times, then other times after maybe 10 times.

Can you have a look and point me in the right direction please as I think that I've tied myself up in knots somewhere,:-D

// PIC 12F683
  // One external switch, on rising edge only count 1, when counter = 5 then blink LED

int i = 0;
void blink(void){
                 for(i = 0; i<4; i++)
                 {
                 GPIO.B0 = 1;
                 Delay_ms(100);
                 GPIO.B0 = 0;
                 Delay_ms(100);
                 }
}
void interrupt(void){ // ISR
                      if (TMR0 == 5)
                      {
                      INTCON.GIE = 0;
                      blink();
                      INTCON.GIE = 1;
                      INTCON.T0IE = 1;
                      TMR0 = 1;
                      INTCON.T0IF = 0;
                      }
}
void main(void){
     TRISIO = 0x04;
     CMCON0 = 0x07;
     CMCON1 = 0x07;
     ANSEL = 0;
     ADCON0 = 0;
     INTCON.GIE = 1;
     INTCON = 0xB1;
     INTCON.T0IE = 1;
     INTCON.T0IF = 0;
     OPTION_REG = 0x20;
     OPTION_REG = 0b10110000;
     TMR0 = 5;
     IOC.IOC2 = 1;
while(1){//Do nothing
}
}

Cheers
Andy

 

[betwixt]

It's basically OK except for a few minor bugs:

1. in main() you change the values of some registers then change them again in the next instruction. Only the last value will be used. Set all the INTCON bits and all the OPTION_REG bits in one instruction each.
2. Dont change the GIE bit inside the interrupt routine. The PIC automatically clears it when an interupt occurs and re-enables it when you leave the interrupt routine.
3. Set the Timer0 interrupt enable in main and leave it set. There's no need to turn it on again in the interrupt routine.
4. It isn't technically wrong to use delay routines inside an interrupt routine but it is considered bad practice. The delay will prevent it leaving the ISR so you could miss other interrupts being triggered.

The reason you are seeing intermittent results is almost certainly due to two factors, the switch contacts will be bouncing and producing more than one pulse to the counter and also you will be missing some presses while in the IRS because of the delays.

A better stategy is to do it like this:

1. Clean up the code as mentioned earlier
2. Declare another variable, I suggest "char Trigger" but the name is up to you to pick.
3. In the ISR simply reset the value in TMR0 ready for the next time, make "Trigger = 1;" then clear the T0IF bit. Remove all the other lines.
4. Inside the while(1) loop, add

if(Trigger == 1) blink();
Trigger = 0;

Now, the while loop will monitor for the switch being pressed five times (or at least, five pulses coming from it) then run the blink code. The Trigger variable will be set when the interrupt is called but actually do the flashing in the main part of the program.

Brian.

 

[----AJ----]

Hi Brian,

thanks for the reply. I've done the changes as suggested but at the moment I can't get it to do anything apart from switch on the LED as soon as I power up, I'm sure I must have misunderstood the changes or I've got the syntax wrong somewhere, here's the code:

 int i = 0;
 char TRIGGER = 0;
void blink(void){
                 for(i = 0; i<4; i++)
                 {
                 GPIO.B0 = 1;
                 Delay_ms(100);
                 GPIO.B0 = 0;
                 Delay_ms(100);
                 }
}
void interrupt(void){ // ISR
                      if (TMR0 == 5) TRIGGER = 1;
                      TMR0 = 5;
                      INTCON.T0IF = 0;
}
void main(void){
     TRISIO = 0x04;
     CMCON0 = 0x07;
     CMCON1 = 0x07;
     ANSEL = 0;
     ADCON0 = 0;
     INTCON = 0b11110000;
     OPTION_REG = 0b11110000;
     TMR0 = 5;
     IOC.IOC2 = 1;
     if (TRIGGER == 1) blink();
     trigger = 0;
while(1){//Do nothing
}
}

 

[----AJ----]

i've just noticed something else which is a bit weird. If i start with the code at the top of the page and upload it to the PIC, it works as described (randomly)

So I change the program code to that which I put into the reply to brian, it works as decribed (or not as stated)

I then change the code back to original code and upload to the PIC. But it dosn't go back to how it was. What i have to do to get it back to the original (random) opperation is delete all the code and then copy and paste from my post above and upload. I just don't get it???????

I'm using 'mikroC', is this program O.K or does it have bugs in it?

Cheers
Andy

 

[betwixt]

You didn't quite follow my instructions:

1. the call to the blink routine goes in the while loop like this:

while(1)
{
 if(Trigger == 1) blink();
 Trigger = 0;
}

Also you need to reset the value in TMR0 to zero not 5 so it can count up to 5 again. The way you are using it, TMR0 increases it's count by one at each pulse on it's input so when it has reached the value to trigger the blinking you have to set it back to the starting value again or it will contine to count upward until it reaches 255, roll over to zero and up to 5 again. It would take 256 pulses to retrigger the blinking!

Be careful with capitalization, I'm not sure about MikroC but most compilers will see "trigger", "Trigger" and "TRIGGER" as different variables.

Brian.

 

[----AJ----]

Still no luck I'm afraid, there has got to be some sort of bug in the compiler, but I have no idea,

Here's the code,

int i = 0;
char trigger = 0;
void blink(void){
                 for(i = 0; i<4; i++)
                 {
                 GPIO.B0 = 1;
                 Delay_ms(100);
                 GPIO.B0 = 0;
                 Delay_ms(100);
                 }
}
void interrupt(void){ // ISR
                      if (TMR0 == 5)
                      {
                      trigger = 1;
                      TMR0 = 0;
                      INTCON.T0IF = 0;
                      }
}
void main(void){
     TRISIO = 0x04;
     CMCON0 = 0x07;
     CMCON1 = 0x07;
     ANSEL = 0;
     ADCON0 = 0;
     INTCON = 0b11110000;
     OPTION_REG = 0b11110000;
     TMR0 = 5;
     IOC.IOC2 = 1;
while(1)
{
if(trigger == 1) blink();
trigger = 0;
}
}