NCS UARTE: rx timeout on buffer overflow

Hi,

At work we are developing custom HW with an nRF9160, using NCS 1.8.0 for now, but I have confirmed the following behaviour on a nRF9160 DK with NCS 1.9.1 as well.

The uart driver seems to have 2 buffers, so it can seemlessly switch from one dma buffer to the next, while the first, full one is "switched out" for a new one. 
I have my uart configured as ASYNC (with hw counting) and a small millisecond delay to get the callback, after a datapacket has been received, so that I do not have to wait until the buffer is full to get my data.

Now the problem:
Say you are in an isr for a comperatively long time and all the while, the uart gets spammed with data, so that the buffers get both fully filled and then some more. Since this happend while in an isr, you can't process the data and switch buffers. After this happens, the uart callback function is only ever called when a buffer is full, and never again, after the timeout set in uart_rx_enable().

To test this, I have attached a small sample program, that uses uart0 as a printk output and uart1 as the receiving uart.The prj.conf is a bit convoluted, as I just copied the one I use for developement at work, but it works on a nRF9160dk as well.

Just set a breakpoint inside uart_callback() and while the program is sitting on the bp, spam about 2*MAIN_UART_BUF_LEN+1 or more to the uart, to ensure that an overflow of both uart buffers occur. remove the breakpoint and run again. Now the uart_callback() only gets called after the buffer is full, and not after the timeout, that is set in uart_rx_enable()

Workaround (possible inspiration for a fix?):

[ EDIT: this has been determined unreliable, so maybe don't look too closely at the following snippet ]

I have modified the function rx_timeout() inside the driver \zephyr\drivers\serial\uart_nrfx_uarte.c to the following:

static void rx_timeout(struct k_timer *timer)
{
	struct uarte_nrfx_data *data = k_timer_user_data_get(timer);
	const struct device *dev = data->dev;
	const struct uarte_nrfx_config *cfg = get_dev_config(dev);
	uint32_t read;

	if (data->async->is_in_irq) {
		return;
	}

	/* Disable ENDRX ISR, in case ENDRX event is generated, it will be
	 * handled after rx_timeout routine is complete.
	 */
	nrf_uarte_int_disable(get_uarte_instance(dev),
			      NRF_UARTE_INT_ENDRX_MASK);

	if (hw_rx_counting_enabled(data)) {
		read = nrfx_timer_capture(&cfg->timer, 0);
	} else {
		read = data->async->rx_cnt.cnt;
	}

	/* Check if data was received since last function call */
	if (read != data->async->rx_total_byte_cnt) {
		data->async->rx_total_byte_cnt = read;
		data->async->rx_timeout_left = data->async->rx_timeout;
	}

	/* Check if there is data that was not sent to user yet
	 * Note though that 'len' is a count of data bytes received, but not
	 * necessarily the amount available in the current buffer
	 */
	int32_t len = data->async->rx_total_byte_cnt
		    - data->async->rx_total_user_byte_cnt;

	if (!hw_rx_counting_enabled(data) &&
	    (len < 0)) {
		/* Prevent too low value of rx_cnt.cnt which may occur due to
		 * latencies in handling of the RXRDY interrupt.
		 * At this point, the number of received bytes is at least
		 * equal to what was reported to the user.
		 */
		data->async->rx_cnt.cnt = data->async->rx_total_user_byte_cnt;
		len = 0;
	}

	/* Check for current buffer being full.
	 * if the UART receives characters before the the ENDRX is handled
	 * and the 'next' buffer is set up, then the SHORT between ENDRX and
	 * STARTRX will mean that data will be going into to the 'next' buffer
	 * until the ENDRX event gets a chance to be handled.
	 */
	bool clipped = false;

	if (len + data->async->rx_offset > data->async->rx_buf_len) {
		clipped = true;
    
		/* In case buffer length and rx offset are equal, we encountered a buffer overflow. */
		if (data->async->rx_buf_len != data->async->rx_offset) {
			len = data->async->rx_buf_len - data->async->rx_offset;
		}
	}

	if (len > 0) {
		if (clipped ||
			(data->async->rx_timeout_left
				< data->async->rx_timeout_slab)) {
			/* rx_timeout ms elapsed since last receiving */
			notify_uart_rx_rdy(dev, len % data->async->rx_buf_len);
			data->async->rx_offset += len % data->async->rx_buf_len;
			data->async->rx_total_user_byte_cnt += len;
		} else {
			data->async->rx_timeout_left -=
				data->async->rx_timeout_slab;
		}

		/* Do not stop rx timeout timer to wait for buffer switching.
		* In case of an overflow, the timer might not get restarted.
		*/
	}

	nrf_uarte_int_enable(get_uarte_instance(dev),
			     NRF_UARTE_INT_ENDRX_MASK);

}

I have noticed that when an overflow occurs, some counter variables inside this function stray apart from one another.

5751.hello_world_UART_rx_test_OF.zip

Related