[SOLVED] PIC16F1847 Timer1

[Veketti]

Dear All,



I’m stumbled with Timer1 in PIC16F1847 as it seems the initialization does make the microcontroller stuck. Especially this line: T1CON = 0b00111101; So when I comment that it will initialize correctly. I’m using 4MHz crystal connected to RA6 & 7. I try to have wake on sleep at RB0 and interrupt at the same pin as well. RA4 pin reads ADC voltage. Timer1 is configured to interrupt every 0.5 seconds where I set flags for display update and after 60 seconds commence sleep. Compiler is mikroC.



Here is the initialization and interrupts:

void InitMain(void) {
      DACCON0.b7 = 0;          // DAC disabled
      SRCON0.b7 = 0;           // SR latch disabled
      CM1CON0.b7 = 0;          // Disable comparator 1
      CM2CON0.b7 = 0;          // Disable comparator 2

      TRISA  = 0b00011100;     // Set RA2,3,4 as inputs, others are output 0 = output 1 = input
      // RA3 is VREF+, RA4 is ADC, RA2 tied to ground VREF-, RA5 transistor pull
      TRISB  = 0b10010011;     // Set all RB as output except RB0, RB1, RB4, RB7. RB0 button RB1 provision for button
      ANSELA = 0b00010000;     // Set all as digital except RA4 which is ADC
      ANSELB = 0b00000000;     // Set all B ports as digital

      INTCON = 0b11011000;    // Interrupt on change bit enabled
      OPTION_REG = 0b10000000;
      T1CON = 0b00111101;  //Timer1 Registers Prescaler= 8 - TMR1 Preset = 3035 - Freq = 2.00 Hz - Period = 0.500008 seconds
      TMR1H = 11;             // preset for timer1 MSB register
      TMR1L = 219;             // preset for timer1 LSB register
      PIE1 = 0b00000001;      // Enable Timer1 interrupt
      IOCBP = 0b00000000;
      IOCBN = 0b00000011;     // Negative edge detection on pin RB0 & RB1
      ADCON0 = 0b00010001;   // AN4 is ADC, ADC is enabled
      ADCON1 = 0b10010010;   // FOSC/8
      FlagForHalfSecond = 0;
}

void Interrupt(){

  if (TMR1IF_bit){      // Timer Interrupt happened
    TMR1IF_bit = 0;
    if (TimerRounds > 120 ) {    // 60 seconds to sleep
       TimerRounds = 0;
       FlagForSleep = 1;
    } else TimerRounds ++;
    FlagForHalfSecond = 1;
  }
    if (INTF_bit){  // Pin Interrupt happened
    INTF_bit = 0;
    if (IOCBF0_bit)
    {
      FlagForBtnPress = 1;
      IOCBF = 0b00000000;
    }
    delay_ms(5);
  }

}

I’d be very thankful if somebody could point me what is wrong with my initialization?

 

[betwixt]

I don't use MikroC so I can't check your code but I will comments that you should NEVER use delay functions inside an ISR and on PIC processors it is recommended that you configure interrupts then turn GIE on but not do it in the same instruction.

I didn't check your timing and prescaler calculation but it would be more appropriate to produce the 'delay()' in the ISR using register contents instead of a mikroC function that either uses interrupts itself or uses blocking code.

Brian.

 

[Aussie Susan]

Is that your entire code?
If so then you are missing the main loop.
I'm not familiar with the mikroc runtime, but it could well be that it will do a reset and then loop back to restart your code again.
if that is NOT the whole of your code then we need to see the rest to really understand what is going on.
I agree wth Brian - never put a delay in an ISR.
Susan

 

[Veketti]

Thank you for your input. I did move the intcon to main function and took the delay out from isr. Still no luck. I have added line which adds "test" to display to see whether it goes through the initialisation. And it does not. Commenting the T1CON passes the text to display. I can't seem to figure out what is wrong with that register.. Here is the complete code so far..

#include "float2ascii.h"
#include "LCD_control.h"

#define POWERSUPPLY lata.f5

unsigned int TimerRounds = 0;
char FlagForSleep = 0;
char FlagForBtnPress = 0;
bit FlagForHalfSecond;
double VoltageReading = 0.0;
char voltage[30];

void InitMain(void) {
      DACCON0.b7 = 0;          // DAC disabled
      SRCON0.b7 = 0;           // SR latch disabled
      CM1CON0.b7 = 0;          // Disable comparator 1
      CM2CON0.b7 = 0;          // Disable comparator 2

      TRISA  = 0b00011100;     // Set RA2,3,4 as inputs, others are output 0 = output 1 = input
      // RA3 is VREF+, RA4 is ADC, RA2 tied to ground VREF-, RA5 transistor pull
      TRISB  = 0b10010011;     // Set all RB as output except RB0, RB1, RB4, RB7. RB0 button RB1 provision for button
      ANSELA = 0b00010000;     // Set all as digital except RA4 which is ADC
      ANSELB = 0b00000000;     // Set all B ports as digital

      OPTION_REG = 0b10000000;
      T1CON = 0b00111101;  //Timer1 Registers Prescaler= 8 - TMR1 Preset = 3035 - Freq = 2.00 Hz - Period = 0.500008 seconds
      TMR1H = 11;             // preset for timer1 MSB register
      TMR1L = 219;             // preset for timer1 LSB register
      PIE1 = 0b00000001;      // Enable Timer1 interrupt
      IOCBP = 0b00000000;
      IOCBN = 0b00000011;     // Negative edge detection on pin RB0 & RB1
      ADCON0 = 0b00010001;   // AN4 is ADC, ADC is enabled
      ADCON1 = 0b10010010;   // FOSC/8
      FlagForHalfSecond = 0;
}

void Interrupt(){

  if (TMR1IF_bit){      // Timer Interrupt happened
    TMR1IF_bit = 0;
    if (TimerRounds > 120 ) {    // 60 seconds to sleep
       TimerRounds = 0;
       FlagForSleep = 1;
    } else TimerRounds ++;
    FlagForHalfSecond = 1;
  }
    if (INTF_bit){  // Pin Interrupt happened
    INTF_bit = 0;
    if (IOCBF0_bit)
    {
      FlagForBtnPress = 1;
      IOCBF = 0b00000000;
    }
  }

}

// Read the pressure
double ReadVoltage (void) {
      double analogvalue = 0.0;
      ADCON0.f1 = 1;                              // Enable conversion
      while (ADCON0.f1);                          // Wait for conversion to end
      analogvalue = (ADRESH << 8 ) + ADRESL;      // Store results
      analogvalue = (analogvalue/1024.00)*5; //calculate actual voltage across the sensor
      return analogvalue;
}

void Turnoffdisplay(){
       POWERSUPPLY = 1;     // Turn off display and stepup supply (Transistor)
       lata.f0 = 0;        // Turn off all display outputs
       lata.f1 = 0;        // before sleep
       latb.f2 = 0;
       latb.f3 = 0;
       latb.f5 = 0;
       latb.f6 = 0;
       delay_ms(10);
}


void main() {
     InitMain();
     POWERSUPPLY = 0;  // ground PNP transistor base to turn it on
     INTCON = 0b11011000;    // Interrupt on change bit enabled
     delay_ms(10);
     Lcd_Init();              // Initialize Lcd
     Lcd_Cmd(_LCD_CLEAR);
     Lcd_Cmd(_LCD_CURSOR_OFF);
     Lcd_Out(2,1,"Test ");   // This is test text that does it ever get this far.
     delay_ms(3000);

 while(1)
 {
   if (FlagForHalfSecond) {
      FlagForHalfSecond = 0;
      VoltageReading = ReadVoltage();
      Float2Ascii(VoltageReading, Voltage,4);
      Lcd_Out(1,1,voltage);
      LCD_Chr_Cp("V ");
   }
 
   if (FlagForSleep == 1)
   {
      Turnoffdisplay(); // Timer triggered sleep
      FlagForSleep = 0;
      asm sleep;
   }
 }

}

 

[KlausST]

Hi,

This still is not the complete code.
I agree with Susan: show complete code.

Still no luck

..is no error description. Tell us what you expect. Tell us what happens...and what does not happen as expected.

The ISR is better now and the main is better, too.

I don't think you need the "sleep flag". Just use the "ELSE" on halfSecondFlag.
Currently the microcontroller is active all the time, then after 120s it goes to sleep for a short time (until the next interrupt - timer interrupt or any other interrupt) then will be active for the rest of the 120s...
(Mind that there may be other interrupts active that are used by included libraries)

For low power battery operation ... it should be in "sleep" as long as possible - or as often as possible.
It wakes up on every interrupt. Checks flag for new display update, does this if necessary, goes to sleep.

Klaus

 

[Veketti]

What I meant with no luck that it didn't show that "Test" in display which in main procedure is calling by "
Lcd_Out(2,1,"Test "); ". However if I comment the T1CON in intialize -procedure it will show that "Test" in display. So I quess it never gets out of the initialize or is on forever loop in interrupt.

Good point, I should disable timer interrupts before going to sleep. Only interrupt that should wake from sleep is button RB0 press.

Sorry, I forgot to add the header files.

 

[KlausST]

Hi,

according your error description: I guess the MicroC libraries already use Timer1. (maybe for delay_ms(), maybe LCD_Control...).

********************

Good point, I should disable timer interrupts before going to sleep. Only interrupt that should wake from sleep is button RB0 press.

Usually not. But in your case: maybe.
Usually not, because it may stop proper library functions, it will stop other interrupt driven processes like UART and so on.
Thus the better solution (unless you really know which interrupts are needed to run) is, to decide in mail loop whther you go to sleep or not.

Example: UART:
The UART may use an interrupt to store the received byte into a software buffer. so it does not miss any UART traffic - even when you put your microcontroller to sleep.
It will process the according ISR and come back to main after the "sleep". for sure you already cleared your "sleep_flag" before and the microcontroller will become active .. draining out battery.

Klaus

 

[Aussie Susan]

The setup of Timer1 is correct (although I must admit I don't like the coding style of setting all the bits in the control registers with a magic number - and for some modules this can lead to mis-configuration). However you need to understand how these old-style timers (that only have the TMRnX registers) work.
You have calculated the initial value for the TIM1L and TMR1H registers correctly but you have forgotten that they are incremented for each tick of the input clock (125KHx in your case) so that the TMR1IF bit is set when the overflow from 0XFFFF (thinking of it as a 16-bit register for the moment) to 0x0000. Therefore the next 'cycle' starts with the TMR1X registers set to 0. In your case, the next cycle interrupt will occur when the TMR1X registers increment back up to 0xFFFF and overflow again. Admittedly that will be in 0.5243 seconds but that difference may be significant (and almost certainly will be for other applications).
If you want the timer to repeat the same 0.5 you need to reset the TMR1X registers within the ISR.
(You can ignore the next bit if you like as the difference it makes may not be much in your situation.)
Also remember that the timer keeps on ticking while the ISR is being triggered and running. Therefore you really need to do the following in the ISR (which is in psedo-code):
TMR1-Registers = 65536 - preset-value + TMR1-registers
What the above does is to account for the additional counts that the TMR1 registers have received from the hardware until your line of code is executed. To turn that into real code you need to read Section 21.5.1 in the data sheet on how to correctly read and write the THR1X registers.
Of course this is why the more modern timer circuits introduce the PRx registers - and your device has 3 such timers - so that the hardware does this all for you.
Although Timers 2, 4 and 6 in your MCU are only 8-bit, they do have the PRx registers and combining the pre- and post-scalars you can divide the input 'fosc/4' by a further 1024. While that does not directly give you something close to a 0.5 second interrupt, you can use the same scheme you have fr your 120 second flag in the ISR.
Susan

 

[Veketti]

I was about to say I'd go back to the easy to use, tested and trued timer0, but after Susan's post I'll try Timer2 instead. Hope that solves the issue.

One other thing confuses me and the datasheet didn't answer me. Do I need to declare INTE and IOCE both for RB0 to wake up from sleep? What about as I want at operation (not during sleep) use the same button to set flag in isr which I then check in main loop to do some stuff. Is INTE and IOCE needed or just INTE?

 

[KlausST]

Hi,

Yes, try timer0 or timer2.

TMR1-Registers = 65536 - preset-value + TMR1-registers

This is a good solution.
The most precise (without timer/counter or setup errors) solution is to use COMPARE registers while not changing TMR1 counter registers at all.
just add your desired timing to the compare registers on every ISR.

Example: (125kHZ timer clock, 20ms timeout=2500 ticks) in ISR do: COMPARE=COMPARE+2500.

******

According INTE/IOCE.
I´m not familiar with PIC...so if you don´t need for immediate reaction you may do without a pin_change_ISR. The pin_change_hardware sets a flag (the interrupt flag) and you may process this flag in main without running an ISR.

***
A little more power consuming.. is to make the timer_ISR run every 20ms (or in that magnitude) and poll the pin states. either in timer_isr or in main.

Klaus

 

[Veketti]

Wohoo, timer2 just worked like a charm! So happy. Thank you everybody, Your help has been invaluable.

Just a side note, would still like to hear from somebody who is familiar with that INTE/IOCE thing question in post #9..

 

[Aussie Susan]

You are dealing with two completely separate types o f interrupt.
INTE enables interrupts on the INT (RB0) pin and the INTEDG bit in the OPTION_REG register sets which edge is used to trigger the interrupt.
IOCE enables the Interrupt-On-Change that globally enables the individual IOC bits in the IOCBP and IOCBN registers. These trigger an interrupt when the corresponding port pin changes (0 to 1 or 1 to 0 depending on the IOCBP/IOCBN settings). IOC also lets you wake from sleep.
Therefore they BOTH use RB0 as a source (but IOCE also lets you use any of the other PORTB pins) but INTE uses one of the edges whereas IOCE allows you to use either or both edges.
By the way, all of this (and really all of the information that you have been told in this thread) is in the data sheet. I know that the Microchip data sheets can be a little hard to learn to read but they really are very comprehensive. Hopefully you can cross-reference what we have been saying here with the data sheet sections and that will help you understand how to get the information you need from them.
Susan