UART RX Callback not triggering

[Theja]

Hello devzone,

I am using Nordic's NUS(Nordic uart service) example and i changed pin configuration to according to our custom board.

TX_done callback working fine but there is a problem with RX_done callback it is not triggering once in a while 

here is a code snippet regarding the problem  

static const struct device *uart = DEVICE_DT_GET(DT_NODELABEL(uart0));
static struct k_work_delayable uart_work;

K_SEM_DEFINE(nus_write_sem, 0, 1);

struct uart_data_t
{
	void *fifo_reserved;
	uint8_t data[UART_BUF_SIZE];
	uint16_t len;
};



static void uart_cb(const struct device *dev, struct uart_event *evt, void *user_data)
{
	ARG_UNUSED(dev);

	static size_t aborted_len;
	struct uart_data_t *buf;
	static uint8_t *aborted_buf;
	static bool disable_req;

	switch (evt->type)
	{
	case UART_TX_DONE:
		// LOG_DBG("UART_TX_DONE");
		// if ((evt->data.tx.len == 0) ||
		//     (!evt->data.tx.buf)) {
		// 	return;
		// }

		// if (aborted_buf) {
		// 	buf = CONTAINER_OF(aborted_buf, struct uart_data_t,
		// 			   data);
		// 	aborted_buf = NULL;
		// 	aborted_len = 0;
		// } else {
		// 	buf = CONTAINER_OF(evt->data.tx.buf,
		// 			   struct uart_data_t,
		// 			   data);
		// }

		// k_free(buf);

		// buf = k_fifo_get(&fifo_uart_tx_data, K_NO_WAIT);
		// if (!buf) {
		// 	return;
		// }

		// if (uart_tx(uart, buf->data, buf->len, SYS_FOREVER_MS)) {
		// 	LOG_WRN("Failed to send data over UART");
		// }

		break;

	case UART_RX_RDY:
		LOG_DBG("UART_RX_RDY");
		buf = CONTAINER_OF(evt->data.rx.buf, struct uart_data_t, data);
		buf->len += evt->data.rx.len;

		if (disable_req)
		{
			return;
		}


		// TODO: newly added
		// if (evt->data.rx.buf[buf->len - 1] == '\n')
		if ((evt->data.rx.buf[buf->len - 1] == '\n') || (evt->data.rx.buf[buf->len - 1] == '\r'))
		{
			disable_req = true;
			uart_rx_disable(uart);
			
			data_from_host = 1;

			memset(host_data, '\0', sizeof(host_data));

			memcpy(host_data, evt->data.rx.buf, (strlen(evt->data.rx.buf) - 2));
		}

		break;

	case UART_RX_DISABLED:
		LOG_DBG("UART_RX_DISABLED");
		disable_req = false;

		//TODO:Commenting newly 
		buf = k_malloc(sizeof(*buf));
		if (buf)
		{
			buf->len = 0;
		}
		else
		{
			LOG_WRN("Not able to allocate UART receive buffer");
			k_work_reschedule(&uart_work, UART_WAIT_FOR_BUF_DELAY);
			return;
		}

		uart_rx_enable(uart, buf->data, sizeof(buf->data),
					   UART_RX_TIMEOUT);

		break;

	case UART_RX_BUF_REQUEST:
		LOG_DBG("UART_RX_BUF_REQUEST");
		buf = k_malloc(sizeof(*buf));
		if (buf)
		{
			buf->len = 0;
			uart_rx_buf_rsp(uart, buf->data, sizeof(buf->data));
		}
		else
		{
			LOG_WRN("Not able to allocate UART receive buffer");
		}

		break;

	case UART_RX_BUF_RELEASED:
		LOG_DBG("UART_RX_BUF_RELEASED");
		buf = CONTAINER_OF(evt->data.rx_buf.buf, struct uart_data_t,
						   data);

		if (buf->len > 0)
		{
			k_fifo_put(&fifo_uart_rx_data, buf);
		}
		else
		{
			k_free(buf);
		}

		break;

	case UART_TX_ABORTED:
		LOG_DBG("UART_TX_ABORTED");
		if (!aborted_buf)
		{
			aborted_buf = (uint8_t *)evt->data.tx.buf;
		}

		aborted_len += evt->data.tx.len;
		buf = CONTAINER_OF(aborted_buf, struct uart_data_t,
						   data);

		uart_tx(uart, &buf->data[aborted_len],
				buf->len - aborted_len, SYS_FOREVER_MS);

		break;

	default:
		break;
	}
}

static void uart_work_handler(struct k_work *item)
{
	struct uart_data_t *buf;

	buf = k_malloc(sizeof(*buf));
	if (buf)
	{
		buf->len = 0;
	}
	else
	{
		LOG_WRN("Not able to allocate UART receive buffer");
		k_work_reschedule(&uart_work, UART_WAIT_FOR_BUF_DELAY);
		return;
	}

	uart_rx_enable(uart, buf->data, sizeof(buf->data), UART_RX_TIMEOUT);
}

static int uart_init(void)
{
	int err;
	struct uart_data_t *rx;

	if (!device_is_ready(uart))
	{
		LOG_ERR("UART device not ready");
		return -ENODEV;
	}

	rx = k_malloc(sizeof(*rx));
	if (rx)
	{
		rx->len = 0;
	}
	else
	{
		return -ENOMEM;
	}

	k_work_init_delayable(&uart_work, uart_work_handler);

	err = uart_callback_set(uart, uart_cb, NULL);
	if (err)
	{
		return err;
	}

	return uart_rx_enable(uart, rx->data, sizeof(rx->data),
						  UART_RX_TIMEOUT);
}

and am using NRF connect sdk (v2.3.0)

[Hieu]

Hi Theja,

The code you give in the opening post and in your later comment are different in a key part

Which is the correct one?

There is also no handling of the fifo_uart_rx_data.

As mentioned, we cannot reproduce the issue on our end with the Central UART sample. Meanwhile, your code contains multiple references to code not available to us, so we cannot effectively test it.

Let's attempt to move forward based on more information. Could you please also give some details about the communication between the nRF52833 and this "controller" device?

How often are packets sent from the controller?
How many bytes are there in a packet?
How, in details, is the callback not triggered?

Alternatively, please see if you can create a minimally modified copy of Central UART sample where the issue can be reproduced, along with the steps to reproduce it.

Best regards,

Hieu

[Theja]

Hi Hieu,

Sorry for that bumble, Uncommented one is the right one.

So I'm using NUS example, I didn't write 'fifo_uart_rx_data'  and whatever data I receive over UART I will copy from "evt->data.rx.buf[buf->len]" buffer and process it later.

This problem occurs in the central device, whatever data I receive through bluetooth, I will process and send it to main controller through UART and I will receive the ACK for the same over UART. While receiving ACK "UART_RX_RDY" callback will trigger. This is not triggering once in a while that is problem I believe you have understood.

Central device is receiving packet once in a minute which is of 25byte size.

After processing the packet received over Bluetooth the same will be sent over UART and the size of the data sent over UART will be 97 bytes and wait for ACK for 1 sec.

I am sure that ACK is coming within 1sec from main controller because, I'm monitoring it through serial terminal even after receiving ACK through UART that "UART_RX_RDY" callback is not triggering once in a while. 

[Hieu]

Hi Theja,

Then I think that the issue is likely in the copying of data to the buffer for post processing that you are using here.

	case UART_RX_RDY:
		LOG_DBG("UART_RX_RDY");
		buf = CONTAINER_OF(evt->data.rx.buf, struct uart_data_t, data);
		buf->len += evt->data.rx.len;

		if ((evt->data.rx.buf[buf->len - 1] == '\n') || (evt->data.rx.buf[buf->len - 1] == '\r'))
		{
			disable_req = true;
			uart_rx_disable(uart);
			
			data_from_host = 1;

			memset(host_data, '\0', sizeof(host_data));

			memcpy(host_data, evt->data.rx.buf, (strlen(evt->data.rx.buf) - 2));
		}

		break;

I think this code is generally not a correct handling of the Zephyr UART driver events. Please read the driver documentation here: https://developer.nordicsemi.com/nRF_Connect_SDK/doc/2.4.0/zephyr/hardware/peripherals/uart.html#c.uart_event_type.

These are some hypothetical bad scenarios/problems with your code:

• Buffer is empty, buffer size is 20, all null. Receiving two packages, "ABC\n" and "DEF\n"
  
  • Upon receiving "ABC\n" your code will pass "AB" to post processing (the length passed is buffer length -2 in your code)
  • Upon receiving "DEF\n" your code will pass "ABC\nDE" to post processing
• Receiving more data than the buffer can store. There will be three events consecutively.
  • UART_RX_RDY when the buffer is full
  • UART_RX_BUF_RELEASED right after
  • UART_RX_RDY when RX timeout
  • If timeout is short enough, your post processing might not have processed the first UART_RX_RDY event, and the data is lost. Furthermore, the first event will be without the '\n' and '\r' terminator. This message is effectively corrupted.
• strlen is not used safely. strlen rely on the array of bytes is null terminated. However, you cannot know whether the data is always null terminated or not. See that the buffer is dynamically allocated with k_malloc(). This introduces risks of getting an invalid large length.

I am a little surprised you didn't have more frequent issues other than just "missing some events."

Please focus into monitoring when the issue happens. I think you will see that LOG_DBG("UART_RX_RDY") will still happen, and likely the current or next data received would be corrupted. 

If you wish for a quick solution, I recommend leaving the UART_RX_** event in the Central UART sample as is. You can then process the data by extracting data from fifo_uart_rx_data, as done in the sample:

		/* Wait indefinitely for data to be sent over Bluetooth */
		struct uart_data_t *buf = k_fifo_get(&fifo_uart_rx_data,
						     K_FOREVER);

		err = bt_nus_client_send(&nus_client, buf->data, buf->len);
		if (err) {
			LOG_WRN("Failed to send data over BLE connection"
				"(err %d)", err);
		}

The Peripheral UART actually does it even better, as it handles the FIFO in a dedicated thread: 

void ble_write_thread(void)
{
	/* Don't go any further until BLE is initialized */
	k_sem_take(&ble_init_ok, K_FOREVER);

	for (;;) {
		/* Wait indefinitely for data to be sent over bluetooth */
		struct uart_data_t *buf = k_fifo_get(&fifo_uart_rx_data,
						     K_FOREVER);

		if (bt_nus_send(NULL, buf->data, buf->len)) {
			LOG_WRN("Failed to send data over BLE connection");
		}

		k_free(buf);
	}
}

K_THREAD_DEFINE(ble_write_thread_id, STACKSIZE, ble_write_thread, NULL, NULL,
		NULL, PRIORITY, 0, 0);