Search code examples
cstm32uart

Missing Last Byte On Circular Buffer Burst


Thanks to help from another user, I have implemented a simple circular buffer to read in some UART data, do some minor processing, then transmit on another UART. I receive a stream of 10-20 6-byte packets based on some user interaction with an outside device. I have a known SOF that is one of two values, four data bytes, then a EOF byte. The first XX packets always have the first SOF value, and the last packet always has the other known SOF value. The issue is that when I get that second SOF value, the IRQ doesn't hit again. As such, I miss the EOF byte in my state machine and the circular buffer pointer doesn't increment, so this last packet doesn't get transmitted. The correct final packet is loaded in the circular buffer (I don't store the EOF bytes), but because my head and tail indices are equal (because I don't proceed to the case STATE_WAIT_EOF state on the final byte, which would increment the head buffer), the head index doesn't get incremented, and the packet doesn't get processed/transmitted.

Here is the state machine in the IRQ handler:

void USART2_IRQHandler(void)
{
    uint8_t Curr_Byte = *RX_Touch_Data;

    switch(Pkt_State)
    {
        case STATE_WAIT_SOF:
        {
            Data_Indx = 0;

            if(( Curr_Byte == TD_PKT) || ( Curr_Byte == LO_PKT ))
            {
                Msg_Buff[Buff_Head_Indx].RX_Data[Data_Indx] = Curr_Byte;                
                ++Data_Indx;
                Pkt_State = STATE_RX_DATA;
            }
         }
         break;

        case STATE_RX_DATA:
        {
            Msg_Buff[Buff_Head_Indx].RX_Data[Data_Indx] = Curr_Byte;
            ++Data_Indx;

            if( Data_Indx == DATA_SIZE )
            {
                Pkt_State = STATE_WAIT_EOF;

                if (Msg_Buff[Buff_Head_Indx].RX_Data[0] == LO_PKT)
                {
                    LO_Pkt = TRUE;  //Set flag indicating got full data buff of last pkt
                }
            }
        }
        break;

        case STATE_WAIT_EOF:
        {
            if(LO_Pkt == TRUE)
            {
                DataFull = TRUE;    //Set breakpoint here, never hits
            }
        
            if( Curr_Byte == EOF_PKT )
            {
                uint8_t New_Buff_Head_Indx = ( Buff_Head_Indx + 1 ) % NUM_MESSAGES;     

                if( New_Buff_Head_Indx != Buff_Tail_Indx)
                {
                    Buff_Head_Indx = New_Buff_Head_Indx;
                }
                else
                {
                    //No space; msg will be lost
                }
            }

            Pkt_State = STATE_WAIT_SOF;
        }
        break;

        default:
        break;

    }

    HAL_UART_IRQHandler(&huart2);                       //Let HAL clean up flags

    HAL_UART_Receive_IT( &huart2, RX_Touch_Data, 1 );   //Get next byte
}

Of course, I also call HAL_UART_Receive_IT function once before entering my main while loop to kick off the process.

There isn't anything "special" about the last packet the device sends. I suspect if I could force the device to ONLY send the one type of packet, I'd find the same situtation: the final packet still wouldn't get sent (unfortunately, I can't do that). Also, note that if I trigger a new burst of packets via a new user interaction, the last packet of the FIRST burst gets "forced" out (which makes sense, as the head index gets incremented). So I could have ten user interactions with ten bursts of packets, and all will be fine, except the last one, which would be missing the final packet.

Does anyone see what I'm missing? I hope my explanation makes sense.

EDIT: Just for test, I blew out my circular buffer size to 10x what I expect from say, ten bursts. Just wnated to eliminate the possibility of any wraparound index issues. No change.

I've temporarily implemented a "patch" where I DON'T look for the EOF byte on the last packet. This works, but is a hack at best and I'd like to understand why it's not working without this.

EDIT2:

HAL_StatusTypeDef HAL_UART_Receive_IT(UART_HandleTypeDef *huart, 
uint8_t *pData, uint16_t Size)
{
   /* Check that a Rx process is not already ongoing */
   if (huart->RxState == HAL_UART_STATE_READY)
   {
       if ((pData == NULL) || (Size == 0U))
   {
       return HAL_ERROR;
   }

   /* Set Reception type to Standard reception */
   huart->ReceptionType = HAL_UART_RECEPTION_STANDARD;

   if (!(IS_LPUART_INSTANCE(huart->Instance)))
  {
     /* Check that USART RTOEN bit is set */
     if (READ_BIT(huart->Instance->CR2, USART_CR2_RTOEN) != 0U)
     {
        /* Enable the UART Receiver Timeout Interrupt */
        ATOMIC_SET_BIT(huart->Instance->CR1, USART_CR1_RTOIE);
     }
   }

   return (UART_Start_Receive_IT(huart, pData, Size));
 }
 else
 {
    return HAL_BUSY;
 }
}

I've set flags on the HAL_BUSY and HAL_ERROR conditions, they are never hit.

The only other potentially relevent code is where the tail index is examined for available packets:

if( Buff_Head_Indx != Buff_Tail_Indx )                                   
//There are available packets to send
{
    Result = Process_Touch_Data();
    Buff_Tail_Indx = ( Buff_Tail_Indx + 1) % NUM_MESSAGES;              
}

Solution

  • Problem

    Your data handling is all delayed by one byte. Here is the sequence of events:

    You call HAL_UART_Receive_IT( &huart2, RX_Touch_Data, 1 ); This enables the receive interrupt on the UART.

    A byte (let's say SOF) is received on the UART. USART2_IRQHandler() gets called. You read a byte from *RX_Touch_Data, which is not currently set to anything meaningful, and process that.

    You then call HAL_UART_IRQHandler() This reads the SOF byte from the UART and stores it in *RX_Touch_Data.

    When the next byte is received (let's say 0x53), your state machine processes the SOF, and then the HAL stores the 0x53 in *RX_Touch_Data. Ad-inifinitum. You're always one byte behind.

    After the final EOF comes in, it is buffered in *RX_Touch_Data, and since no more bytes appear, it is not processed by the state machine.

    Suggested solution

    Read the byte from UART2 yourself in your interrupt handler, something like:

    uint8_t Curr_Byte = UART2->DR & 0xff;
    

    and do not call HAL_UART_IRQHandler().

    I'd also recommend just enabling the receive interrupt yourself. Then you don't need to call HAL_UART_Receive_IT() any more. You just get an interrupt every time a byte is received. This might be enough:

    /* Enable the UART Data Register not empty Interrupt */
    __HAL_UART_ENABLE_IT(&huart2, UART_IT_RXNE);