Problems with delay for phase firing [SOLVED]

(instructions, reset, WDT, specifications...) PIC12F6xx, PIC16Fxxx, PIC16F6x, PIC16F7x

Problems with delay for phase firing [SOLVED]

Postby SamuelWong » Fri Jul 24, 2020 5:34 am

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
SamuelWong
 
Posts: 11
Joined: Wed Jul 22, 2020 6:20 am

Re: Problems with delay for phase firing

Postby ric » Fri Jul 24, 2020 5:45 am

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.
Latest test project, an LED matrix display made from one reel of addressable LEDs. here
User avatar
ric
Verified identity
 
Posts: 659
Joined: Sat May 24, 2014 2:35 pm
Location: Melbourne, Australia
PIC experience: Professional 5+ years with MCHP products

Re: Problems with delay for phase firing

Postby SamuelWong » Fri Jul 24, 2020 5:51 am

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
 
Posts: 11
Joined: Wed Jul 22, 2020 6:20 am

Re: Problems with delay for phase firing

Postby ric » Fri Jul 24, 2020 5:54 am

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.
Latest test project, an LED matrix display made from one reel of addressable LEDs. here
User avatar
ric
Verified identity
 
Posts: 659
Joined: Sat May 24, 2014 2:35 pm
Location: Melbourne, Australia
PIC experience: Professional 5+ years with MCHP products

Re: Problems with delay for phase firing

Postby SamuelWong » Fri Jul 24, 2020 5:56 am

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
SamuelWong
 
Posts: 11
Joined: Wed Jul 22, 2020 6:20 am

Re: Problems with delay for phase firing

Postby ric » Fri Jul 24, 2020 6:03 am

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.
Latest test project, an LED matrix display made from one reel of addressable LEDs. here
User avatar
ric
Verified identity
 
Posts: 659
Joined: Sat May 24, 2014 2:35 pm
Location: Melbourne, Australia
PIC experience: Professional 5+ years with MCHP products

Re: Problems with delay for phase firing

Postby SamuelWong » Fri Jul 24, 2020 6:07 am

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
SamuelWong
 
Posts: 11
Joined: Wed Jul 22, 2020 6:20 am

Re: Problems with delay for phase firing

Postby SamuelWong » Fri Jul 24, 2020 6:22 am

Ok, I changed it to microseconds in integers, Its working now
SamuelWong
 
Posts: 11
Joined: Wed Jul 22, 2020 6:20 am

Re: Problems with delay for phase firing [SOLVED]

Postby AussieSusan » Mon Jul 27, 2020 9:57 am

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
AussieSusan
Verified identity
 
Posts: 173
Joined: Mon Jun 16, 2014 4:45 am
PIC experience: Experienced Hobbyist

Re: Problems with delay for phase firing [SOLVED]

Postby ric » Mon Jul 27, 2020 11:49 am

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.
Latest test project, an LED matrix display made from one reel of addressable LEDs. here
User avatar
ric
Verified identity
 
Posts: 659
Joined: Sat May 24, 2014 2:35 pm
Location: Melbourne, Australia
PIC experience: Professional 5+ years with MCHP products

Next

Return to 14-Bit Core

Who is online

Users browsing this forum: No registered users and 6 guests

cron