Hardware serial port RX locks up after overrun error.

[benwaffle]

Describe the bug
When you run the following code, and start spamming words (followed by the enter key) on your keyboard, eventually it stops reading data. You can also lock it up by just hitting enter without any letters. The USART1 interrupt stops firing, because the code seems to fail to set TXEIE. It also appears to only happen when there's an overrun error as verified by looking at USART_SR with gdb.

To Reproduce

#include <Arduino.h>

void setup() {
    Serial.begin(9600);
    Serial.println("hello");
}

void loop() {
    if (Serial.available() > 0) {
        printf("Serial available: %d\n", Serial.available());
        char buf[128] = {0};
        char *line = buf;
        size_t nbytes;
        if ((nbytes = Serial.readBytesUntil('\n', buf, sizeof buf)) == 0) {
            printf("# error: no data read from serial\n");
            return;
        }

        printf("# read %d [%s]\n", nbytes, line);
        printf("rest = %d\n", Serial.available());
    }
}

Steps to reproduce the behavior:

1. Run code
2. Open a serial monitor (e.g. pio device monitor)
3. Spam stuff into the serial console. (Or just hit enter)
4. See it lock up and refuse to take more input.

Expected behavior
Continue receiving input, possibly by dropping the next character.

Desktop (please complete the following information):

• OS: Mac OS 10.14.3
• Platformio: 3.6.6
• STM32 core version: 1.5.0
• Upload method: STLink

Board (please complete the following information):

• Name: Bluepill 128k

[benwaffle]

• looks similar to stm32: UART overrun race condition locks up system micropython/micropython#3375
• issue appears to be gone when i don't use TX on that uart device

[fpistm]

Hi @benwaffle
Seems similar to #467.

[benwaffle]

I tried the 2 patches from @ppescher but that did not fix it

[fpistm]

Ok. Thanks for the test.

[uzi18]

sizeof(buf)

[benwaffle]

@uzi18 The code compiles just fine. You only need the parenthesis for types, not variables.

[ppescher]

Hi @benwaffle
It may not work for you because you are using printf(). Try to only use Serial.print()/println().

[benwaffle]

I will try that, but printf() uses uart_debug_write() which uses the synchronous HAL_UART_Transmit(), without interrupts.

[ppescher]

@benwaffle the issue is with __HAL_LOCK(huart) which is not a real mutex or critical section. If a receive interrupt fires in between __HAL_LOCK/__HAL_UNLOCK the interrupt flag will not be re-enabled, like I describe in #467
My solution provides a critical section only for HAL_UART_Receive_IT/Transmit_IT, but not for HAL_UART_Transmit. You might try to wrap this function call with HAL_NVIC_DisableIRQ/EnableIRQ inside uart_debug_write(), like I did for uart_attach_rx_callback/tx_callback. Or you might just not use printf().

[fpistm]

I didn't see you used printf which is mainly used for debug purpose.
So mixing Serial and printf on the same uart is not a good solution.

[benwaffle]

@ppescher, thank you, disabling the IRQ in uart_debug_write() fixes the issue for printf(). I made a PR, #496, but I need some help with a few things for it.

[ppescher]

I think all you need is `HAL_NVIC_DisableIRQ(serial_debug.irq)` since the IRQ number is set during initialization.

[benwaffle]

@ppescher good catch, that makes it easier

[ShenRen]

I am happy to see the bug which have a improvement measure new.
@ppescher your PR only work for port of stm32 mcu, If i using stm32f4xxx , how should I chenge zhe code in function void HAL_UART_ErrorCallback(UART_HandleTypeDef *huart);
#496