UARTE communication using CONFIG_UART_INTERRUPT_DRIVEN dropping data after 8 bytes

[Jared]

I've configured UART2 on the nRF9160 to connect to a nRF52840. The nRF52840 generates a payload and sends it over to the nRF91. The payload size during testing was 65 bytes. The problem with all of this? Using the CONFIG_UART_INTERRUPT_DRIVEN code, the UART peripheral is able to receive the first 8 bytes correctly. Then there's an additional 4 bytes of bogus data followed by the start of the next payload. (Same first 8 bytes) I'm running UART @ 250k BAUD.

Everything matches up with the transmitting device signal wise. 

I'm looking into the ASYNC API to see if that makes a difference. It's a bit hairy though and overkill for this application. (Previous implementation was on a nRF52 buffering each byte as it came in via interrupt)

I'm initializing like this:

static int uart_init(void)
{

  uart = device_get_binding(DT_LABEL(DT_NODELABEL(uart2)));
  if (!uart)
  {
    LOG_ERR("UART binding failed");
    return -ENXIO;
  }

  /* Set the data */
  uart_data.len = 0;

  /* Set up callback for serial event */
  uart_irq_callback_set(uart, uart_interrupt_handler);

  /* Enable the irq */
  uart_irq_rx_enable(uart);

  /* No errors */
  return 0;
}

This is what the callback looks like.

static void uart_interrupt_handler(struct device *dev)
{
  uart_irq_update(dev);

  /* Check if there's serial data. */
  while (uart_irq_rx_ready(dev))
  {

    int data_length;

    // Read the data
    data_length = uart_fifo_read(dev, &uart_data.buffer[uart_data.len],
                                 UART_BUF_SIZE - uart_data.len);

    printk("%x ", uart_data.buffer[uart_data.len]);

    /* First byte is how much data */
    if (uart_data.len == 0)
    {
      chunk_size = uart_data.buffer[0];
      printk("start chunk size = %d\n", chunk_size);
    }

    /* Increment data amount */
    uart_data.len += data_length;

    /* Decode the data if chunk_size == uart_data. */
    if (uart_data.len >= chunk_size + 1)
    {
      /* Do stuff */
      
      uart_data.len = 0;

    }
  }

}

Here's the overlay

/*
 * Copyright (c) 2020 Circuit Dojo LLC
 *
 * SPDX-License-Identifier: Apache-2.0
 */

&uart2 {
	compatible = "nordic,nrf-uarte";
	current-speed = <250000>;
	status = "okay";
	tx-pin = <24>;
	rx-pin = <23>;
	rts-pin = <29>;
	cts-pin = <30>;
};

It seems like there may be a potential error condition or that the interrupt is not getting called fast enough to process the data?

Any suggestions?

Thanks!

[Jared]

Appears to be fixed with ASYNC. I used the code in the peripheral_uart example. Github Link

I'm using flow control so here's my final prj.conf

# Enable the UART driver
CONFIG_UART_ASYNC_API=y
CONFIG_UART_LINE_CTRL=y
CONFIG_NRFX_UARTE2=y
CONFIG_SERIAL=y

[Jared]

While this is good for many cases, it doesn't quite work for my use case. I'd like to read byte by byte as I have variable payloads. Any thoughts here would be great!

[JONATHAN LL]

Hi Jared,

Check out Ring buffers, https://docs.zephyrproject.org/latest/reference/kernel/other/ring_buffers.html#implementation

Regards,
Jonathan

[Jared]

Hey Jonathan,

Thanks for the tip. I'm not sure how it relates though.

This seems to be an issue related to the UART implementation unless Im missing something huge here. If I simply print out the bytes from the ISR, it prints the first 8 and then stops. Seems like there's an internal 8 byte buffer. Anything after that 8 bytes gets dropped as far as I can tell.

[JONATHAN LL]

Hi, I might have misunderstood you original problem.

I see that you are using a uart_cb function not implemented as a nrf9160 example yet so I would recommend to look at this example.
Could you se if the same behavior persists using the example implementation?
I have not tested to see if there is a 8 byte internal buffer or tested to se if it the buffer is configurable at this moment.

Regards,
Jonathan

[Jared]

Thanks Jonathan! 

I will try exactly that and let you know. I think the biggest difference in mine was that I was trying to read byte by byte. I found that code elsewhere in NCS. Must have been for a different purpose!

[Jared]

Hey Jonathan,

I was able to try your suggestion here. Here's my code looks like:

static uint8_t buf[256];

static void uart_cb(struct device *dev)
{

  uart_irq_update(dev);

  /* Read as much data as possible */
  if (uart_irq_rx_ready(dev))
  {
    /* Read operation and get bytes read*/
    int len = uart_fifo_read(dev, buf, 256);

    LOG_INF("%d bytes", len);
  }

  if (uart_irq_tx_ready(dev))
  {
    /* Release last buf */

    /* Get next */
  }
}

int comms_init(comms_evt_callback_t cb)
{
  if (cb == NULL)
  {
    return -EINVAL;
  }

  /* Copy pointer to callback */
  callback = cb;

  /* Get the UART device */
  uart = device_get_binding(DT_LABEL(DT_NODELABEL(uart2)));
  if (!uart)
  {
    return -ENXIO;
  }

  /* Enable ISR */
  uart_irq_callback_set(uart, uart_cb);

  /* Enable RX events */
  uart_irq_rx_enable(uart);

  /* Enable the thread */
  k_sem_give(&comms_init_ok);

  return 0;
}

It's extremely similar compared to the example you sent. Unfortunately it's still receiving only single bytes. Here's the debug printout showing I only got 12 bytes out of the 30+ bytes sent on the other end.

1 bytes
1 bytes
1 bytes
1 bytes
1 bytes
1 bytes
1 bytes
1 bytes
1 bytes
1 bytes
1 bytes
1 bytes

[Jared]

The only solution that I've found for this is a as follows:

1.  Use the ASYNC API as I was before (Github Link)

2. On the other device, pad any remaining bytes with 0's so they always align with the size of the RX buffer on the nRF9160 

3. When trying to figure out message boundaries, the first byte indicates how many data bytes are in a stream. So technically the following byte after the last in the "frame" would be the size. I now disregard any size of 0. 

It's a little clunky but it works for now. 

Edit:

Also another hint from Eli H. that if you're logging within the interrupt context you will, for sure, drop data if CONFIG_LOG_MODE_IMMEDIATE is set to 'y'. Either turn of immediate logging or remove logging from the ISR and you should be good to go!