• AVR Freaks

Hot!watchdog RESET

Author
AMPS
Super Member
  • Total Posts : 429
  • Reward points : 0
  • Status: offline
2019/04/09 20:53:59 (permalink)
0

watchdog RESET

dear all,
I have pasted my code partial. i am using pic16F886 with hitech compiler version 9.86. I have tested code for RS485 ckt alone and its working well without single pole mismatch. now i including this hardware in hardware and testing communication which evolved Relays and display led.when i used below code it started hanging some of the cases. MCU wont allow to do any kind of operations,if i power off and on then its start giving valid polls So i used watchdog timer and i found its recovering when it hangs.
my question why this hang in code. i suspect on intilization and using config bit selection.when i used watchdog also in 50 polls 2 poll misfire and then give proper response
 
And also facing issue while declaring variable
unsigned char rxbuf[25];
unsigned char ser_data[25];
unsigned char crc_data[25];
 
if i declare unsigned char rxbuf[50]; its show buffer oversized
post edited by AMPS - 2019/04/09 21:06:19

Amps
*.*.*.*.*.*.*.*.*.*.*.*.*
#1

12 Replies Related Threads

    1and0
    Access is Denied
    • Total Posts : 9628
    • Reward points : 0
    • Joined: 2007/05/06 12:03:20
    • Location: Harry's Gray Matter
    • Status: online
    Re: watchdog RESET 2019/04/09 21:13:50 (permalink)
    +1 (1)
    Just took a brief look at your code. Here

    void Serial_Interrupt() {
    if((1==RCIE)&&(1==RCIF)) {
    RCIF=0;

    RCIF is a read-only bit and I don't see where RCREG is read.
    #2
    qhb
    Superb Member
    • Total Posts : 9998
    • Reward points : 0
    • Joined: 2016/06/05 14:55:32
    • Location: One step ahead...
    • Status: offline
    Re: watchdog RESET 2019/04/09 21:19:26 (permalink)
    +1 (1)
    A few comments
    void Serial_Interrupt() {
    if((1==RCIE)&&(1==RCIF)) {

    If you are never going to clear RCIE, there is no need to test it (which just slows your interrupt service down unnecessarily.code]
    RCIF=0;[/code]
    It is impossible to clear RCIF in this way. That is a read only bit. It happens automatically when you read RCREG, so just delete this line.
     
    for(index=0; index<8; index++) {
    rxbuf[index] = Serial_Receive_byte();

    Don't do this inside an interrupt service!
    Receive one byte, and get out as quickly as you can.
    You will have to rearrange your logic to handle one byte per service.
     
    It looks like you started to do this in the following code. As you have it now, the "index" variable will never be ">= 8".
     
     n.b. Why are you calling a function called "Serial_Receive_byte()" (which you did not provide code for), when all you need to do at that point in time is to read the RCREG register?
     
    post edited by qhb - 2019/04/09 21:21:31

    Nearly there...
    #3
    qhb
    Superb Member
    • Total Posts : 9998
    • Reward points : 0
    • Joined: 2016/06/05 14:55:32
    • Location: One step ahead...
    • Status: offline
    Re: watchdog RESET 2019/04/09 22:11:06 (permalink)
    0
    Also this is pointless:
    if(FERR==1) {
      SPEN=0;
      SPEN=1;
    }
    FERR is just a flag giving you information about the most recently received character.
    There is no need to clear it, it will update again as soon as you receive another character.
     
     

    Nearly there...
    #4
    AMPS
    Super Member
    • Total Posts : 429
    • Reward points : 0
    • Status: offline
    Re: watchdog RESET 2019/04/09 23:23:51 (permalink)
    0
     without using it in product its working fine just calling Serial_interrupt function in timer interrupt i have checked more than 10K poll . i wont get single poll missed.
    when i used in product timer interrupt checking background calculation and processing it at same time serial interrupt also called.
    I have made changes in timer interrupt and initialization in my code  to make it work on product. communication happening but hangs after 10  to 50 polls . due to watchdog timer i could see it resting to initial condition
     

     
     
     

    void Serial_1_Send_byte(int trbuf1) {
    TXREG=trbuf1;
    while(0==TRMT);
    while(0==TXIF);
    TXIF=0;
     
     
     

    }
     
     
     
    char Serial_Receive_byte() {
     
     
     
    while(RCIF==0); // Wait till the data is received
    RCIF=0; // Clear receiver flag
    return(RCREG); // Return the received data to calling function
     
     
     
    }
     
     
     
     
     
     
     
    void Serial_Interrupt() {
    if((1==RCIE)&&(1==RCIF)) {
    RCIF=0;
    for(index=0; index<8; index++) {
    rxbuf[index] = Serial_Receive_byte();
     
     
     
    }
    if(index>=8) {
    rec_flag = 1;
    DE=1;
     
     
     
    }
    if(index>=20) {
    index=0;
    }
     
     
     
    if(FERR==1) {
    SPEN=0;
    SPEN=1;
    }
     
     
     
    if(OERR==1) {
    CREN=0;
    CREN=1;
    }
    DE=1;
    serial_access();
     
     
     
    if(DE==1) {

    }
     
     
     

    }
     
     
     
     
     
     
     
    }
     
     
     

    post edited by AMPS - 2019/04/09 23:33:46

    Amps
    *.*.*.*.*.*.*.*.*.*.*.*.*
    #5
    qhb
    Superb Member
    • Total Posts : 9998
    • Reward points : 0
    • Joined: 2016/06/05 14:55:32
    • Location: One step ahead...
    • Status: offline
    Re: watchdog RESET 2019/04/09 23:34:23 (permalink)
    +1 (1)
    It's rubbish code.
    If you don't want to take advice, you're on your own.
     

    Nearly there...
    #6
    1and0
    Access is Denied
    • Total Posts : 9628
    • Reward points : 0
    • Joined: 2007/05/06 12:03:20
    • Location: Harry's Gray Matter
    • Status: online
    Re: watchdog RESET 2019/04/10 00:18:42 (permalink)
    +2 (2)
    qhb
    It's rubbish code.
    If you don't want to take advice, you're on your own.

    Sometimes I'm wondering if we're wasting our time. sad: sad
    #7
    AMPS
    Super Member
    • Total Posts : 429
    • Reward points : 0
    • Status: offline
    Re: watchdog RESET 2019/04/10 00:18:54 (permalink)
    0
    This is working code i have attached with RS485 communication. With below initialization , I never see single poll mismatch with code. code continuously send request and deliver the response.
     
    The code posted above only made changes in initialization since it uses  peripheral port for control relays.
     
    I am sorry if mistake i have done. let me know if any changes to be done. i wil glad to take it and make it work.
     
    I have added asm("clrwdt"); in timer and main loop while doing with actual product

    void interrupt isr(void) {
     

    asm("clrwdt"); 
    Serial_Interrupt();
    if (TMR1IF==1) {
    TMR1IF = 0;
    TMR1H = 0xD8; // 2mSec Timer Interrupt (5msec ACTUAL as per H/W)
    TMR1L = 0xFC; // //0x61; //5D
     

    }
    if (PRG_RUN == 1) {
    BLINKCOUNT1++;
    if (BLINKCOUNT1 > 1200) {
    BLINKCOUNT1 = 0;
    if (bits.BLINKFLAG1 == 0)
    bits.BLINKFLAG1 = 1;
    else //if(bits.BLINKFLAG==1)//window flashing
    bits.BLINKFLAG1 = 0;
     

    }
     

    BLINKCOUNT++;
    if (BLINKCOUNT > 50) {
    BLINKCOUNT = 0;
    if (bits.BLINKFLAG == 0)
    bits.BLINKFLAG = 1;
    else //if(bits.BLINKFLAG==1)//window flashing
    bits.BLINKFLAG = 0;
     

    if (bits.FLASHFLAG == 0) {
    FLASHCOUNT++;
    if (FLASHCOUNT > 1) {
    bits.FLASHFLAG = 1;
    LED_R = 0;
    LED_G = 0;
    }
    } else { //if(bits.FLASHFLAG==1)//run mode leds blinking.
    FLASHCOUNT++;
    if (FLASHCOUNT > 8) {
    FLASHCOUNT = 0;
    bits.FLASHFLAG = 0;
    LED_R = 1;
    LED_G = 1;
    }
    }
    }
     

    }
    }
     

    void main() {
    InitController(); // Initialisation of Controller
    InitUART();
     

    TMR1IE = 1;
    PEIE = 1;
    GIE = 1; // Global interrupt enabled.
    DE=0;
     
     
     

    while(1) {
    asm("clrwdt");
    }
     

    }
     


    post edited by AMPS - 2019/04/13 02:56:34

    Amps
    *.*.*.*.*.*.*.*.*.*.*.*.*
    #8
    pcbbc
    Super Member
    • Total Posts : 1253
    • Reward points : 0
    • Joined: 2014/03/27 07:04:41
    • Location: 0
    • Status: offline
    Re: watchdog RESET 2019/04/10 01:34:23 (permalink)
    +1 (1)
    Once again, in case you missed it the first time...
    Serial_Interrupt calls...
    Serial_Receive_byte...
    Which does while(RCIF==0);

    What if there is no byte to receive? Your code just hangs and the watchdog eventually resets.
    What if there is an error on the serial comms? You aren’t checking for that either.

    Never ever do any kind of delay, or loop waiting for things to happen in an ISR. You are just asking for trouble with hangs and non-responsive mainline code.
    For example this in your ISR __delay_ms(50); is a total NO, NO!

    You may well consider it “working code”, but have you tried sending it only 4 of the 8 bytes it is expecting? I bet you haven’t, because it will cause exactly the symptoms you describe. Maybe your sender always sends 8 bytes, but they are not guaranteed to arrive, or arrive without errors. You must account for the 0.1% cases as well as the 99.9% if you want your code to work 100% of the time (and not just 99.9%).

    Feel free to fix the obvious errors and come back if your code still does not work. We’ll be delighted to look further.
    #9
    AMPS
    Super Member
    • Total Posts : 429
    • Reward points : 0
    • Status: offline
    Re: watchdog RESET 2019/04/10 02:21:04 (permalink)
    0
    thanks . but i would like to know where i need to make correction. yes i know i should used delay program so if you check my actual code i have eliminated __delay_ms(50);  and i have used asm("nop"); function.
     
    can you elaborate this 2 conditions
     
    What if there is no byte to receive? Your code just hangs and the watchdog eventually resets.
    What if there is an error on the serial comms? You aren’t checking for that either.
     
    char Serial_Receive_byte() {

    if(OERR==1) {
    CREN=0;
    CREN=1;
    }
    while(RCIF==0); // Wait till the data is received
    //RCIF=0; // Clear receiver flag
    return(RCREG); // Return the received data to calling function
    }
     
    post edited by AMPS - 2019/04/10 02:49:01

    Amps
    *.*.*.*.*.*.*.*.*.*.*.*.*
    #10
    pcbbc
    Super Member
    • Total Posts : 1253
    • Reward points : 0
    • Joined: 2014/03/27 07:04:41
    • Location: 0
    • Status: offline
    Re: watchdog RESET 2019/04/10 03:01:19 (permalink)
    +1 (1)
    A start would be...
    void Serial_Interrupt()
    {
        if((1==RCIE)&&(1==RCIF)) {
            if (index < 8) {
                // No need to clear RCIF here, it is cleared by reading RCREG - clearing manually does NOTHING.
                rxbuf[index++] = RCREG;
            }
        }
        // __delay_ms(50); //Remove this line - delays in ISR are universally BAD
        if(index>=8)
        {
            rec_flag = 1;
            DE=1;
        }
        if(index>=20) {
            index=0;
        }
        if(FERR==1) {
            SPEN=0;
            SPEN=1;
        }
        if(OERR==1) {
            CREN=0;
            CREN=1;
        }
        //Response_data(); //This code is also rubbish for the same reason - it loops sending data.
        // And it cannot go here now, as it will get called every time you receive 1 byte (not after 8).
        if(DE==1)
        {
            // It needs to go here where you originally had it.
            // But it is still not correct because it LOOPS.  Build the data you need to send in the response
            // message buffer (ser_data), but do NOT actually send it. Send it out one byte at a time per
            // interrupt when the TXIF flag is set.  The code to do this will look similar, but not identical,
            // to how the receive buffer is coded above.
            Response_data();       
        }
    }

    #11
    AMPS
    Super Member
    • Total Posts : 429
    • Reward points : 0
    • Status: offline
    Re: watchdog RESET 2019/04/10 04:05:17 (permalink)
    0
    I have made changes as below

    oid Serial_Interrupt() {
    if((1==RCIE)&&(1==RCIF)) {
    for(index=0; index<8; index++) {
    rxbuf[index] = Serial_Receive_byte();

    }
    if(index>=8) {
    rec_flag = 1;
    DE=1;
    }
    if(index>=20) {
    index=0;
    }
    if(FERR==1) {
    SPEN=0;
    SPEN=1;
    }
    if(DE==1) {
    Response_data();
    }
    }
    char Serial_Receive_byte() {
    if(OERR==1) {
    CREN=0;
    CREN=1;
    }
    while(RCIF==0); // Wait till the data is received
    RCIF=0;
    return(RCREG); // Return the received data to calling function
    }


    Amps
    *.*.*.*.*.*.*.*.*.*.*.*.*
    #12
    pcbbc
    Super Member
    • Total Posts : 1253
    • Reward points : 0
    • Joined: 2014/03/27 07:04:41
    • Location: 0
    • Status: offline
    Re: watchdog RESET 2019/04/10 05:11:02 (permalink)
    +2 (2)
    You still have a loop receiving data:
    for(index=0; index<8; index++) {
    rxbuf[index] = Serial_Receive_byte();
    }

     
    Did you not see the code I replaced this with?
    Do you not understand why you should never LOOP in an ISR?
    Why did you decide to ignore the code I posted?
    How did you fix the looping issue sending data in Response_data?  It doesn't look like you have.
     
    This is all the code that is necessary to receive a byte into your buffer:
    rxbuf[index++] = RCREG;

    As I, and others have told you, you do not need to call Serial_Receive_byte or set RCIF = 0.
    RCIF will be cleared automatically when you read the last byte from RCREG.
     
    If you make changes, post your entire code again.  It's hard to keep track otherwise.
    #13
    Jump to:
    © 2019 APG vNext Commercial Version 4.5