PicForum

Hi, I'm working on a program where I control the phase firing of a heater using a microcontroller. I am using the interrupt on change on pin RB2 of the PIC16f887 to check for the zero crossing moment of the AC line. At the Positive cycle, the input to the PIC is low, at the negative cycle, the input to the PIC is high. The problem is the {void Pause(double milliseconds)} function is not delaying the pulse I send for the phase firing ciruit. Am I passing the value wrongly? Please help

Code: Select all

#pragma config CONFIG1 = 0x2CD2
#pragma config CONFIG2 = 0x0700
#include <stdio.h>
#include <stdlib.h>

#include <xc.h>
#define _XTAL_FREQ 20000000

void initPhase(void);
void Pause(double);
static double C = 0;
double X = 0;
double milliseconds;


void initPhase(void)
{
    PORTB  = 0;
    ANSELH = 0x00;
    TRISBbits.TRISB0 = 1;
    TRISAbits.TRISA4 = 0;
    WPUB = 0b00000001;
    IOCB = 0x04;
    OPTION_REGbits.nRBPU = 1;            // clear RBPU bit (OPTION_REG.7)
    OPTION_REGbits.INTEDG = 1;
   
    INTCONbits.GIE = 1; //global interrupt enable
    INTCONbits.PEIE = 0; //disable peripheral interrupts (PEIE)
    INTCONbits.RBIE = 1;
    INTCONbits.INTE = 1;//For external interrupt (RB0/INT)PORTB  = 0;
   
}

void light(X)
{
    Pause(X);
    PORTAbits.RA4 = 1;
    __delay_ms(0.5);
    PORTAbits.RA4 = 0;
}

void Pause(double milliseconds)
{
    while(milliseconds > 0)
    {
      __delay_ms(0.1);
      milliseconds = milliseconds - 0.1;
    }
}

void __interrupt () isr()
{
    INTCONbits.GIE = 0;
    if (INTCONbits.RBIF == 1)
    {
        if (PORTBbits.RB2 == 1)
        {
            __delay_ms(1);
            light(C);
        }
        else
            light(C);
        INTCONbits.RBIF = 0;
        INTCONbits.GIE  = 1;
    }
   
    if (INTCONbits.INTF == 1)
    {
        if(PORTBbits.RB1 == 1)
        {
            PORTBbits.RB1 = 0;
        }
        else if (PORTBbits.RB1 == 0)
        {
            PORTBbits.RB1 = 1; 
        }
        INTCONbits.INTF = 0;
        INTCONbits.GIE  = 1;
    }         
}

void main(void)
{
   
    TRISBbits.TRISB1 = 0;
    PORTBbits.RB1 = 0;
    __delay_ms(1000);
    initPhase();
    while(1)
    {
        C=0.8;
    }
}

Thank you in advance

Never EVER twiddle the GIE bit inside an interrupt service!

The hardware does that for you. If you set it yourself before the ISR exits, you WILL have nasty problems.

It is also very bad practice to have a time delay inside an interrupt service.
It's better to setup a timer to interrupt again at the end of the required time.

It's also bad prctice to do ANY floating point operations inside an ISR.
Your code would be far more efficient if you used integer variables, and work in microseconds rather than fractions of milliseconds.

Is there any chance of moving to a neweer PIC16F device with LATx registers?
If you start using any other outputs on PORTA, you could get some nasty read-modify-write effects on that port.

I am restricted to use this PIC. However I can change it to RB3, will it help?

Does exiting the ISR automatically set the GIE?

SamuelWong wrote:I am restricted to use this PIC.

That's a shame. Newer PIC16F chips are cheaper, and much nicer to work with.

However I can change it to RB3, will it help?

You are only using one pin on PORTA, so there's no problem at the moment.

Does exiting the ISR automatically set the GIE?

Yes. And entering the ISR automatically clears it.
Setting it yourself will allow another interrupt to occur before it has exited THIS service, corrupting all the context saving done by the compiler code.

I was using an int for testing and it was working fine and delayed it by the ms and it was working. Once I changed it to double for better accuracy it stops working

Do you realise that "double" is floating point?
Floating point takes much more resources than integer calculations on an 8 bit processor, and should be avoided at all costs.
That's why I suggested working in microseconds rather than milliseconds.

changed all the variables to int, working as expected;

Code: Select all

#pragma config CONFIG1 = 0x2CD2
#pragma config CONFIG2 = 0x0700
#include <stdio.h>
#include <stdlib.h>

#include <xc.h>
#define _XTAL_FREQ 20000000

void initPhase(void);
void Pause(int);
int C = 0;
int X = 0;
//int milliseconds;


void initPhase(void)
{
    PORTB  = 0;
    ANSELH = 0x00;
    TRISBbits.TRISB0 = 1;
    TRISAbits.TRISA4 = 0;
    WPUB = 0b00000001;
    IOCB = 0x04;
    OPTION_REGbits.nRBPU = 1;            // clear RBPU bit (OPTION_REG.7)
    OPTION_REGbits.INTEDG = 1;
   
    INTCONbits.GIE = 1; //global interrupt enable
    INTCONbits.PEIE = 0; //disable peripheral interrupts (PEIE)
    INTCONbits.RBIE = 1;
    INTCONbits.INTE = 1;//For external interrupt (RB0/INT)PORTB  = 0;
   
}

void light(X)
{
    Pause(X);
    PORTAbits.RA4 = 1;
    __delay_ms(0.5);
    PORTAbits.RA4 = 0;
}

void Pause(int milliseconds)
{
    while(milliseconds > 0)
    {
      __delay_ms(1);
      milliseconds = milliseconds -1;
    }
}

void __interrupt () isr()
{
    if (INTCONbits.RBIF == 1)
    {
        if (PORTBbits.RB2 == 1)
        {
            __delay_ms(1);
            light(C);
        }
        else
            light(C);
        INTCONbits.RBIF = 0;
    }
   
    if (INTCONbits.INTF == 1)
    {
        if(PORTBbits.RB1 == 1)
        {
            PORTBbits.RB1 = 0;
        }
        else if (PORTBbits.RB1 == 0)
        {
            PORTBbits.RB1 = 1; 
        }
        INTCONbits.INTF = 0;
    }         
}

void main(void)
{
   
    TRISBbits.TRISB1 = 0;
    PORTBbits.RB1 = 0;
    __delay_ms(1000);
    initPhase();
    while(1)
    {
        C=2;
    }
}

why can't I use double

Ok, I changed it to microseconds in integers, Its working now

You call the __delay_ms function both directly and indirectly (in the light() function) - this is a VERY BAD IDEA.
ISRs should be very quick and the the least amount of work possible. In your case you would be far better off to have the ISR set a flag that you test in your main loop.
Susan

I agree that delays inside ISRs are a very bad idea (as I already mentioned in my first response).
Note that __delay_ms is NOT a function call. It's a macro with inline code, and I'm pretty sure that XC8 makes sure usage in an ISR won't tromp over non-ISR scratch.