Not Equals Operator not working when interrupts are enabled on PIC32

Question

I am currently trying to parse a string received via UART from a GPS module on a PIC32MZ2048EFG100 micro-controller via a UART receive interrupt protocol. I am using MPLAB X IDE v4.10 and XC32 v2.05 for my IDE and compiler.

When I enabled the UART4 receive interrupt, the != operator stops functioning as it should. I have a block of code within my main function that should never be executed, but yet it is.

I have narrowed it down to enabling interrupts being the problem. If I comment out all working code in my ISR, I still face the issue of myCounter being incremented.

Here is the code I have in my main function.

int myCounter = 0;

void main ( void ){

    hal_sys_init();
    hal_uart4_init();
    gps_init();

    //Setup interrupt
    asm volatile("di"); //Disable all interrupts
    asm volatile("ehb"); //Disable all interrupts

    INTCON = 0; //Clear interrupt control register

    IEC0 = 0;
    IEC1 = 0;
    IEC2 = 0;
    IEC3 = 0;
    IEC4 = 0;
    IEC5 = 0;
    IEC6 = 0;

    INTCONbits.MVEC = 1; //Enable multi-vectored interrupts

    IFS5bits.U4RXIF = 0; //Clear interrupt flag
    IPC42bits.U4RXIP = 1; //Set priority level to 7
    IPC42bits.U4RXIS = 0; //Set sub-priority to 0
    IEC5bits.U4RXIE = 1; //Enable interrupt

    asm volatile("ei"); //Enable all interrupts

    while(1){
        int x = 0;
        if(x != 0){
            myCounter++;  //Should never be executed
        }
    }
}

When running this code on my PIC with interrupts enabled, myCounter gets incremented.

Here is the code for my interrupt service routine.

void __ISR(_UART4_RX_VECTOR, ipl7SRS) UART4_Interrupt(void) {
    while (U4STAbits.URXDA) {
        char c = U4RXREG;
        if (c == '\n') {
            currentLine[--lineIndex] = 0; //Overwrite /r as null terminator for string
            parseStringFlag = 1;
            lineIndex = 0;

            if (currentLine == buff_1) {
                currentLine = buff_2;
                previousLine = buff_1;
            } else {
                currentLine = buff_1;
                previousLine = buff_2;
            }

        } else if (lineIndex < MAX_LINE_LENGTH) {
            currentLine[lineIndex++] = c;
        } else {
            currentLine[--lineIndex] = c;
        }
    }
    IFS5bits.U4RXIF = 0; //Clear interrupt flag
    return;
}

Here is the basic ISR code that still makes myCounter increment.

void __ISR(_UART4_RX_VECTOR, ipl7SRS) UART4_Interrupt(void) {
    while (U4STAbits.URXDA) {
        char c = U4RXREG;
    }
    IFS5bits.U4RXIF = 0; //Clear interrupt flag
    return;
}

What could be causing the code that should never be executed to execute? If I run the interrupt code in main with interrupts disabled the code works and the code that should never be executed is not executed.

Answer 1

Here:

    if (c == '\n') {
        currentLine[--lineIndex] = 0; //Overwrite /r as null terminator for string

If the first character received were \n and lineIndex is initialised zero, lineIndex will be decremeted from zero. Assuming it is unsigned, then lineIndex < MAX_LINE_LENGTH will be false and:

    } else {
        currentLine[--lineIndex] = c;
    }

will run repeatedly until lineIndex is eventually decremented to MAX_LINE_LENGTH - 1 - stomping over a large swathe of memory - which is most likely what is happening in this case.

Suggest:

    if( lineIndex != 0 && c == '\n' && ) 
    {
        currentLine[--lineIndex] = 0; //Overwrite /r as null terminator for 

or:

    if( c == '\n' ) 
    {
        if( lineIndex != 0 ) 
        {
            lineindex-- ;
        }
        currentLine[lineIndex] = 0; //Overwrite /r as null terminator for 

depending on the semantics you require. It is not a given that the sending system uses CR+LF pairs for line-ends, and you should not assume that. The code should probably be further modified to check that the preceding character was indeed a CR before decrementing lineindex. An exercise for the reader.

And similarly for the final else:

    } 
    else if( lineIndex != 0 )
    {
        currentLine[--lineIndex] = c;
    }

or

    } 
    else
    {
        if( lineIndex != 0 ) 
        {
            lineindex-- ;
        }
        currentLine[lineIndex] = c;
    }

It is possible that the latter protection is not necessary, but the protection is useful perhaps for clarity and maintenance - it is defensive code - your call.

It may be a safer and more interrupt efficient design to have the ISR simply place any received character into a ring-buffer, and then deal with line-input outside of the interrupt context. You might increment a counter on every received \n and decrement it when \n were unbuffered so that the receiver will know how many lines are currently buffered for processing.