missing a character after a fixed interval while receiving through UART in stm32(bluepill)

Question

I am trying to send a string "hello" continiously through UART1 from Atmega 328P to the BLUEPILL board. I am doing the reception by by interrupt. The problem is after few times it starts to give "ello" "hello" alternatively on my serial monitor, I am not able to figure out whats going wrong. BAUD RATE-9600 parity bit- NONE STop bit- 1

main.c:

The usart_IRQ is called here under "usart code begin here":

#include "main.h"
#include "uart.h"
UART_HandleTypeDef huart1;
char *readBuffer;

void SystemClock_Config(void);
static void MX_GPIO_Init(void);
static void MX_USART1_UART_Init(void);

int main(void)
{
 HAL_Init();
 SystemClock_Config();
 MX_GPIO_Init();
  MX_USART1_UART_Init();

 /* USER CODE BEGIN 2 */
  __HAL_UART_ENABLE_IT(&huart1, UART_IT_RXNE);
    HAL_NVIC_SetPriority(USART1_IRQn, 0, 0);
    HAL_NVIC_EnableIRQ(USART1_IRQn);
  /* USER CODE END 2 */

The string received is stored in the "readBuffer" using the readstring and the printed to the serial monitor using the writestring forwhich i created a separate UART library.

while (1)
  {
    /* USER CODE END WHILE */

      readBuffer=serialReadString1();
      writeString1(readBuffer);
      HAL_Delay(500);

    /* USER CODE BEGIN 3 */
  }

USART1_IRQHandler

I have enabled the Global interrupt and called the received function there, and used the return because I dont want to use the HAL_irq:

void USART1_IRQHandler(void)
{
  /* USER CODE BEGIN USART1_IRQn 0 */
 receivestring1();
 return;
  /* USER CODE END USART1_IRQn 0 */
  HAL_UART_IRQHandler(&huart1);
  /* USER CODE BEGIN USART1_IRQn 1 */

  /* USER CODE END USART1_IRQn 1 */
}

uart.c

for write string:

#include"main.h"
#include "uart.h"
#define RX_BUFFER_SIZE1 144

char rxBuffer1[RX_BUFFER_SIZE1];
uint16_t rx1ReadPos;


void sendChar1( uint8_t ch)
{
  while(!(USART1->SR & USART_SR_TXE));
  USART1->DR = ch;
}


void writeString1(const char c[])
{
    uint16_t i = 0;
        for (i = 0; i<= strlen(c); i++)
        {
            sendChar1(c[i]);
        }
        memset(rxBuffer1,0,RX_BUFFER_SIZE1);
        rx1ReadPos = 0;
}

for read string:

 unsigned char receivedChar1(void)
    {
       while(!(USART1->SR & USART_SR_RXNE));//check whether any tx is going on
       return USART1->DR;
    }
    
    void receivestring1()
    {
        while(!(USART1->SR & USART_SR_RXNE));//check whether any tx is going on
        rxBuffer1[rx1ReadPos] = USART1->DR;
    
                if (rxBuffer1[rx1ReadPos] != '\n')
                {
                    rx1ReadPos++;
                    if (rx1ReadPos >= RX_BUFFER_SIZE1)
                    {
                        rx1ReadPos = 0;
                    }
                }
    }
    
    char* serialReadString1()
    {
        return rxBuffer1;
    }

Answer 1

I can spot several potential issues in your code:

1. You should not skip the call of HAL_UART_IRQHandler() in USART1_IRQHandler(). Each interrupt usually needs to be acknowledged (cleared) in either the interrupt controller (NVIC) and/or the respective peripheral unit (the UART in your case). If this is not done, you might end up with the interrupt handler being called again right away, although there has not been a real new event.

2. The while(!(USART1->SR & USART_SR_RXNE)) at the start of receivedChar1() and receivestring1() should not be necessary, since the interrupt should only be generated if the RX is not empty. Your interrupt configuration looks correct, so this might be only necessary due to issue #1 (see above).

3. The variables rxBuffer1and rx1ReadPos are manipulated both in receivestring1() and writeString1(). Since receivestring1() is called from USART1_IRQHandler() while writeString1() is called from main() this manipulation can happen virtually in parallel. This can become problematic for two reasons:

   • The compiler might not be aware, the content of rxBuffer1 might change at any (e.g. also while processing the for loop in writeString1()). This can be avoided by adding the volatile keyword to rxBuffer1, rx1ReadPos and also the argument of writeString1(). This way the compiler will re-evaluate strlen(c) on every pass of the for loop.
   • The code might still try to change rx1ReadPos at virtually the same time, e.g. when a new character is received by receivestring1() just in the moment before rx1ReadPos is reset to zero in writeString1(), which would lead to losing that character. This is called a race-condition and can only be avoided by either blocking interrupts for a limited time or by constructing the code in a way that makes sure, that parallel execution can not lead to data-loss. One possible solution could be the use of two separate buffers - one used for receiving and one used for writing.

4. writeString1() is writing the characters via sendChar1() to the same UART1 that is receiving the data. This is not a principally okay but sending the data is thus using the same slow 9600 baud-rate. This makes it really probable, that writeString1() gets interrupted by USART1_IRQHandler(), i.e. that additional data is received. It might be better to use a different UART configured to a higher baud-rate for debug purposes.

I suppose that your "ello" problem is primarily caused by issue #3.

Addendum

To check whether the stm32 correctly receives the characters from the Atmega you could simplify your code by sending out each received character directly from the interrupt handler (avoiding the complexity of the asynchronous buffer handling). This could look like this:

int main(void)
{
  [...]
  while(1)
  {
    // everything is done in IRQ handler
  }
}

void USART1_IRQHandler(void)
{
  /* USER CODE BEGIN USART1_IRQn 0 */
  unsigned char newChar;
  newChar = USART1->DR; // read received char
  USART1->DR = newChar; // send it out again right away

  /* USER CODE END USART1_IRQn 0 */
  HAL_UART_IRQHandler(&huart1);
  /* USER CODE BEGIN USART1_IRQn 1 */

  /* USER CODE END USART1_IRQn 1 */
}

Addendum #2

By request here is a sketch how the buffer handling could be improved:

char rxBuffer1[RX_BUFFER_SIZE1];
char rxBuffer2[RX_BUFFER_SIZE1];
char* volatile rxBufferPtr = rxBuffer1;
volatile bool SwitchBufferRequest = false;

void writeString1(const char buffer[])
{
    for (int i = 0; i<= strlen(buffer); i++)
    {
        sendChar1(buffer[i]);
    }
    memset(buffer,0,RX_BUFFER_SIZE1);
}

void receivestring1()
{
    static uint16_t rxBufferPos = 0;

    if (SwitchBufferRequest)
    {
        if (rxBufferPtr == rxBuffer1)
        {
            rxBufferPtr = rxBuffer2;
        }
        else
        {
            rxBufferPtr = rxBuffer1;
        }
        rxBufferPos = 0;
        SwitchBufferRequest = false;
    }
    
    rxBufferPtr[rxBufferPos] = USART1->DR;
    
    if (rxBufferPos < (RX_BUFFER_SIZE1 - 2)) // buffer full?
    {
        rxBufferPos++;
    }
}
    
char* serialReadString1()
{
    char* rxBufferWithData = rxBufferPtr;

    SwitchBufferRequest = true;
    while (SwitchBufferRequest == true) {;} // wait for buffer switch
    return rxBufferWithData;
}

void USART1_IRQHandler(void)
{
  /* USER CODE BEGIN USART1_IRQn 0 */
  receivestring1();
  /* USER CODE END USART1_IRQn 0 */
  HAL_UART_IRQHandler(&huart1);
  /* USER CODE BEGIN USART1_IRQn 1 */

  /* USER CODE END USART1_IRQn 1 */
}

int main(void)
{
  [...]
  while (1)
  {
    /* USER CODE END WHILE */

      readBuffer=serialReadString1();
      writeString1(readBuffer);
      HAL_Delay(500);

    /* USER CODE BEGIN 3 */
  }
}

Note: This sketch tries to modify your code as little as possible. The key ideas are:

1. Use two separate buffers (rxBuffer1 and rxBuffer2) -- one for receiving data and one for sending received data out again.
2. Buffers are switched each time new data is needed in main()
3. Buffer switching is requested in serialReadString1() via the flag SwitchBufferRequest. Buffer switching itself is done within the ISR to avoid any race conditions.
4. Note the order of operations in serialReadString1(): First the pointer to the latest received data is copied in a separate variable before SwitchBufferRequest is set to true because rxBufferPtr might change at any time afterwards.
5. writeString1() is only modified slightly by removing the reset of rxBufferPos (now done in ISR) and clearing the RX buffer which is not used by the RX ISR.