UART RX Callback not triggering

Question

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] == '
')
 if ((evt->data.rx.buf[buf->len - 1] == '
') || (evt->data.rx.buf[buf->len - 1] == ''))
 {
 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 · Accepted Answer

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] == '
') || (evt->data.rx.buf[buf->len - 1] == ''))
 {
 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
" and "DEF
" 
 
 Upon receiving "ABC
" your code will pass "AB" to post processing (the length passed is buffer length -2 in your code) 
 Upon receiving "DEF
" your code will pass "ABC
DE" 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 '
' and '' 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);

UART RX Callback not triggering

Top Replies