Timer PIC16 not working.. is it my code?

Question

I'm new here as an OP, but I have been visiting this site for years. This time, just looking at other problems hasn't helped me. Hence, I sign up and just ask!

I'm creating a countdown timer with 30 lights that shut off one after the other for an adjustable time (for my girlfriends elementary school). I'm using pic16f628a and MPLAB with XC8 compiler. The hardware isn't an issue, I've had quite some practice. The software on the other hand..

The timer works as I can use buttons to increase the set time, press start, all lights light up, and watch the lights slowly (or sometimes fast) counting down. However, I cant fix the timing. For example 2 minutes set lasts 3, 4 minutes set lasts 5. Whatever I've done to all the relevant variables, is not working. The timing changes, but never correctly. For example, doubling the number of interrupts per light does not (even nearly!) double the time and I never get it right. I've calculated and tried dozens of variables and corrective measures. Absolute precision is not necessary. I've been at this problem for more than 2 weeks and getting desperate. Please Help! Here's my (I think relevant) code:

include <xc.h>
pragma config FOSC = INTOSCIO // Oscillator Selection bits
pragma config WDTE = OFF // Watchdog Timer Enable bit
pragma config PWRTE = OFF // Power-up Timer Enable bit
pragma config MCLRE = ON // RA5/MCLR/VPP pin function is MCLR
pragma config BOREN = ON // Brown-out Detect Enable bit
pragma config LVP = OFF // Low-Voltage Programming Enable bit
pragma config CPD = OFF // Data EE Memory Code Protection bit 
pragma config CP = OFF // Flash Program Memory Code Protection bit 
define _XTAL_FREQ 4000000 
volatile int intr = 0; //number of interrupts
void interrupt TimerOverflow(void) 
{
 if(TMR0IE && TMR0IF)
 {
 TMR0 = 0; //clear timer 0
 TMR0IF=0; //clear flag
 intr++; //count 1 interrupt
 }
}
void buzzer() // this part just buzzes on start and on end, not included for cleanliness 
void display2(int c) //multiplexing, this function displays the last sections' digits. Working so not included. int c is the number of minutes, as received from display()
void display(int b) //multiplexing, this function displays the first sections. Working so not included. int b is the number of minutes, as received from init() and teller()
void teller(int a) //a is the number of minutes as received from init(), 'teller' is counter in dutch
{
int perlight = a*67 //number of minutes times a variable (now 67) to calculate number of interrupts per light-shut-off
PS0 = 1; // Prescaler (1:256) is assigned to the timer TMR0
PS1 = 1;
PS2 = 1;
T0CS = 0; //internal clock selector
PSA = 0; //assign prescalar
TMR0IE = 1; //timer0 interrupt active
PEIE = 1; //peripheral interrupt active
GIE = 1; //global interrupt active
TMR0IF = 0; //clear timer0 flag
TMR0 = 0; // clear timer0
int d = 30; //turn all lights on starting the count
while(1) //endless loop
{
 if (intr>=perlight) //if number of interrupts is more than 'needed' for one-light-shutoff
 {
 intr = 0; //reset interrupts
 d--; //decrease count (turn one light off)
 }
 display(d); //send count to display
 if(d==0) //if count is done
 {
 buzzer(); //beep 3 times
 __delay_ms(500);
 buzzer();
 __delay_ms(500);
 buzzer();
 break;
 }
}
}
void init()
{
int min = 0; //number of minutes that the clock needs to run
while(1) //endless loop
{
 display(min); //send number of minutes to display function
 if(RA2==0) //if 'add minute' button has been pressed
 {
 __delay_ms(50); //debouncer
 if(RA2==0) 
 {
 min++; //add minute to count
 display(min); //send to display
 }
 } 
 if(RA3==0) //if start is pressed
 {
 __delay_ms(50); //debouncer
 if(RA3==0)
 {
 if(min!=0) //if start is pressed, buzz and start teller()
 {
 buzzer();
 teller(min);
 }
 if(min==0) // if start is pressed without timing set, buzz 3 times
 {
 buzzer();
 __delay_ms(500);
 buzzer();
 __delay_ms(500);
 buzzer();
 }
 }
 }
}
}
void main()
{
TRISB = 0b00000000; //RB as Output PIN
TRISA = 0b00101100; //RA as Input/Output PIN
CMCON = 0b00000111; //disable comparators
while(1)
{
 init(); //start init()
}
}

I really hope someone can help this total C-amateur.. Thanks in advance!!
EDIT: I've tried using timer2 as well with a changed interrupt-counter. This was even stranger than before, with 1 and 2 minute set finishing in under a second, and 3 minutes and beyond taking very long..
EDIT2 I've tried replacing variable 'perlight' to a integer (61 (2 minutes), second time 122(4 minutes)). Perfect timing! But now its not adjustable, so I figure something is wrong with 'perlight'?

• Triak: @line 49 from the current code display(d); //send count to display may be causing problems(making delays). Remove it and post the results
• user79496: Hi Triak, Thanks for your reply. I've implemented & tested your suggestion, and find that it does not influence the general timing of this countdown timer. The timing is still very unreliable. EDIT2 in my post is what confuses me; somehow it doesn't calculate 'perlight' appropriately..

Answer 1

I tested your code in MPLab Sim and on real hardware, getting the expected results in both cases (4 minutes with perlight = a*61 and time set to '2 minutes'). Perhaps your problem is caused by some code you haven't shown us, or you have a hardware issue. However I can see some things that might affect the timing:-

1. You are incrementing the 16 bit variable intr in the ISR, then comparing it to perlight in your main code. The 16F628 can only compare 8 bits at a time, and an interrupt can occur at any time, so intr could change in the middle of the comparison and corrupt the result. To prevent this possibility you should disable interrupts whenever intr is accessed from the main code.

2. After the timing session finishes your timer interrupt keeps going, and you don't reset intr before starting another timing session. This causes subsequent timing sessions to be up to 4 seconds short. At the beginning of each timing session you should disable interrupts, reset the timer hardware, reset intr to zero, then re-enable interrupts to start counting.

3. You are clearing TMR0 in the ISR. This is not necessary because the timer automatically wraps to zero when it overflows. However any write to TMR0 also resets the prescaler, losing the cycles it counted between the timer overflow and write to TMR0. Letting the timer run free allows it to keep consistent time even if interrupt latency changes.

4. With a 4MHz clock and 1:256 prescaler you cannot get an accurate 2 second time delay because 2/0.065536 is not an integer. However you can get very close by changing the prescaler to 128 and using a `perlight' multiplier of 61, which equates to 1.998848 seconds (128*256*61). For best timing accuracy you should use a crystal or ceramic resonator rather than the internal oscillator (which can be out by several percent).

Answer 2

I haven't fully analyzed your code, but I suspect that what may be happening is that your timer is overflowing faster than your code loop is running. And, if so, then you are overflowing your intr register which is leading to strange results.

• user79496: Bitsmack, Thanks for your quick answer! But isn't the timer supposed to overflow every 256 counts of timer0? So one count is 1us, overflow at 256us, with prescaler 0,065s, times 67 = 4s?
• user79496: Just to be sure, I replaced 67 with 15745 to get te time per light to 4 s. This move has made the clock very slow.. I've also replaced the int 'intr' with a long 'intr' just in case. Still, no predictable timing.
• bitsmack: @SjorsLiebregts You are right, of course :) I've corrected my post. I'll come back tomorrow and look again. Hopefully someone will have an answer for you before then!