Linux CDC ACM Host App passing misaligned bytes to app_usbd_cdc_acm_read()

[Mark J]

Hi,

I am using a Linux Host to communicate with my nRF52840 as a USB CDC ACM device. As a test, I ran minicom and was able to transfer bytes to my Nordic app successfully. I used the Nordic egs to get this far. When I switch from minicom to my Linux USB Host app, I am seeing very strange byte alignment issues. If (for example), I write the following 4 bytes { 0x01, 0x02, 0x03, 0x04 } I receive 4 bytes within my Nordic app app_usbd_cdc_acm_read() call, but I receive 0x02, 0x03, 0x04, 0x00. I expected that this was a simple bug where I selected an incorrect offset, but no such "luck". My offsets are correct.

On Linux, CDC ACM driver shows my device as /ttyACM0. I have setup a udev rule as per this and simlilar articles: https://hackaday.com/2009/09/18/how-to-write-udev-rules/#comments

Specifically, I have added the following entry to a /etc/udev/rules.d/99-com.rules file entry to create /dev/ttyDwUSBBLE with specific access to my device with Vid=0x030e and Pid=0x0000

KERNEL=="ttyACM[0-9]*", SUBSYSTEM=="tty", ATTRS{idVendor}=="030e", ATTRS{idProduct}=="0000", SYMLINK="ttyDwUSBBLE"

Opening and writing to /dev/ttyACM0 and /dev/ttyDwUSBBLE has the same behaviour - the first byte written does not come through and and extra byte is added to the end of my written data.

I am quite confident that my Nordic code is fine, but that the Linux CDC driver has issues or that I am configuring the Linux CDC port incorrectly. This is my config:

// sPort is /dev/ttyACM0 
bool CUSBHost::init(char* sPort)
{   
    // If already open, close first.
    if(m_fileDescripter > 0)
    {
        close(m_fileDescripter);
        m_fileDescripter = 0;
    }
    
    m_fileDescripter = open (sPort, O_RDWR | O_NOCTTY );
    if (m_fileDescripter < 0)
    {
        printf("Error opening port %s\r\n", sPort);
        return false;
    }

    // Set connection speed, always 8n1 (no parity). Try default of 230k first.
    if(!setIfaceAttrib (B4000000))  
    {
        printf("Error setting USB Host speed.\r\n");
        return false;
    }
    
    // All set.
    m_initialized = true;
    
    return true;
}

This calls setIfaceAttrib() shown below.

bool CUSBHost::setIfaceAttrib (int speed)
{
    struct termios tty;
    memset (&tty, 0, sizeof tty);
    if (tcgetattr (m_fileDescripter, &tty) != 0)
    {
        cout << "Error from tcgetattr()." << endl;
        return false;
    }

    cfsetospeed (&tty, (speed_t) speed);
    cfsetispeed (&tty, (speed_t) speed);
    
    // The 3 disable lines below are suggested for Linux hosts such as the Pi.
    tty.c_iflag &= ~(IXON | IXOFF | IXANY); // Turn off s/w flow ctrl
    tty.c_iflag &= ~(INLCR | IGNCR | ICRNL );
    tty.c_oflag &= ~OPOST; // Prevent special interpretation of output bytes (e.g. newline chars)
    tty.c_oflag &= ~ONLCR; // Prevent conversion of newline to carriage return/line feed
    tty.c_oflag &= ~OCRNL;
    
    tty.c_lflag &= ~(ECHO | ECHOE | ECHONL | ICANON | ISIG | IEXTEN);

    tty.c_cflag     &=  ~PARENB;            // No parity.
    tty.c_cflag     &=  ~CSTOPB;            // Not 2 stop bits, but only 1
    tty.c_cflag     &=  ~CSIZE;             // Clear size
    tty.c_cflag     |=  CS8;                // Set to 8 bits.

    tty.c_cc[VMIN]   =  0;                  // Read doesn't block
    tty.c_cc[VTIME]  =  1;                  // 0.1 seconds read timeout
    tty.c_cflag     |=  CREAD | CLOCAL;     // Turn on READ & ignore ctrl lines
    
    // Make raw 
    cfmakeraw(&tty);
   
    //tcflush(m_fileDescripter, TCIFLUSH);
    if (tcsetattr (m_fileDescripter, TCSANOW, &tty) != 0)
    {
        cout << "Error from tcsetattr()." << endl;
        return false;
    }
    return true;
}

If Nordic had any Linux CDC Host comms examples that would clarify how the port should be setup to talk to Nordic USB devices, that would be very helpful. I expect that my issue relates to how my port is setup in my Linux Host.

Online info suggests that Linux does things with ACM devices that could explain the strange behaviour. I think that my issue may be resolved by changing my USB CDC Host port config.

Thanks in advance,

Mark J

[Jørgen Holmefjord]

Hi,

Unfortunately, I'm not aware of any Linux CDC Host communication examples. If you are not able to reproduce the issue with minicom you may get better suggestions if you ask your question in a Linux forum. Especially if you think that the issue lays on the Linux side with your port setup, and not on the Nordic side.

Best regards,
Jørgen

[Mark J]

Hi Jorgen,

Thanks for replying ...

In the end, the issue was actually my misunderstanding of the Nordic USB CDC ACM Class. Specifically - the app_usbd_cdc_acm_read() call. The implementation of reads within this class is .... special (choosing my words carefully).

The app_usbd_cdc_acm_read() function DOES NOT always read. It is commonly more of a read REQUEST that the driver reads incoming data into the buffer provided when the specified number of data bytes are available and then notifies the API consumer via the APP_USBD_CDC_ACM_USER_EVT_RX_DONE event. 

When handling the APP_USBD_CDC_ACM_USER_EVT_RX_DONE event, API consumers can call app_usbd_cdc_acm_read() in case there are any additional bytes to read. If there are, NRF_SUCCESS is returned and bytes are read into the provided buffer. If not, a value of NRF_ERROR_IO_PENDING is returned.

It took alot of empirical testing to resolve how the API worked. Better API doc text and more comments within the example code would have made my task dramatically easier.

Note that I believe that I found a bug .... When calling app_usbd_cdc_acm_read() when handling the APP_USBD_CDC_ACM_USER_EVT_PORT_OPEN event (as per the eg), I received a return value of NRF_SUCCESS - when no incomming bytes were present. The return should have been NRF_ERROR_IO_PENDING. When calling app_usbd_cdc_acm_read() within the APP_USBD_CDC_ACM_USER_EVT_RX_DONE event, returns from the function seemed to be correct.

Below is my event handler code with heavy comments. 

void cdc_acm_user_ev_handler(app_usbd_class_inst_t const * p_inst,
                                    app_usbd_cdc_acm_user_event_t event)
{
  ret_code_t ret;

  app_usbd_cdc_acm_t const * p_cdc_acm = app_usbd_cdc_acm_class_get(p_inst);

  switch (event)
  {
    case APP_USBD_CDC_ACM_USER_EVT_PORT_OPEN:

      SEGGER_RTT_printf(0, "CDC Port Open\n");

      // This call does not actually read any incoming bytes, it simply REQUESTS that
      // the m_rx_buffer[] will be filled by 1 byte whenever 1 byte comes in. 
      // When this byte is received, an APP_USBD_CDC_ACM_USER_EVT_RX_DONE will be generated.
      // Note that NRF_ERROR_IO_PENDING should be returned if no bytes have been recieved
      // but (for some reason) when this call is made within this event type, NRF_SUCCESS always
      // seems to be returned, so ignore the return value.
      app_usbd_cdc_acm_read(&m_app_cdc_acm, &(m_rx_buffer[m_rx_idx]), 1);
      
      break;

    case APP_USBD_CDC_ACM_USER_EVT_PORT_CLOSE:
      SEGGER_RTT_printf(0, "CDC Port Close\n");
      break;

    case APP_USBD_CDC_ACM_USER_EVT_TX_DONE:
      SEGGER_RTT_printf(0, "CDC Tx Done\n");
      break;
     
    case APP_USBD_CDC_ACM_USER_EVT_RX_DONE:
    {
      SEGGER_RTT_printf(0, "Read event.\n");

      // Get amount of incoming data. Should match the length of data within the last request (i.e. read()).
      size_t size = app_usbd_cdc_acm_rx_size(p_cdc_acm);

      // Increment m_rx_idx by the number of bytes in for proper rx indexing.
      m_rx_idx+=size;

      // As per the usbd_ble_uart eg (the eg that uses a Softdevice), read data in 1 char at
      // a time and add it to our buffer at m_rx_idx.
      while(true)
      {
        // Read additional bytes 1 at a time. If they are available, NRF_SUCCESS will be returned.
        // If a byte is not available, this app_usbd_cdc_acm_read() call will REQUEST that it is
        // read into m_rx_buffer[m_rx_idx] when it comes in. When this happens, this event will be triggered again.
        // Note that even if we have a full pckt received before this read call, it is required so that we can get
        // this APP_USBD_CDC_ACM_USER_EVT_RX_DONE event again when the next pckt is sent.
        ret = app_usbd_cdc_acm_read(&m_app_cdc_acm, &(m_rx_buffer[m_rx_idx]), 1);

        // DEBUG SEGGER_RTT_printf(0, "Read with ret %d and size %d and byte 0x%02X into idx %d\n", ret, size, m_rx_buffer[m_rx_idx], m_rx_idx);
   
        // Check ret.
        if(ret == NRF_SUCCESS) { m_rx_idx++; } // Keep going as there may be more bytes.
        else if(ret == NRF_ERROR_IO_PENDING) 
        { 
          // DEBUG for(int x=0; x<m_rx_idx; x++) { SEGGER_RTT_printf(0, "idx %d: 0x%02X\n", x, m_rx_buffer[x]); }

          // Check the pckt len at m_rx_buffer[1] to see if we have a complete pckt.
          if(m_rx_idx>=2)
          {
            uint8_t pcktLenInPckt =  m_rx_buffer[1];
            if(m_rx_idx >= pcktLenInPckt)
            {
              // Schedule an event on the main thread to process the pckt. Will run usbd_cdc_data_handler() below from main.
              app_sched_event_put(NULL,  0, main_usbd_cdc_sched_handler);
            }
          }
          
          // All done reading data that drove the event.
          return; 
        }
        else
        {
          SEGGER_RTT_printf(0, "app_usbd_cdc_acm_read() error: %d. USB read fail.\n", ret);

          return;
        }
        
      }

      break; // case APP_USBD_CDC_ACM_USER_EVT_RX_DONE:
    }

    default:
      break;
  }
}

I will suggest this comment as an answer in the hope that somebody can verify it.

I would strongly encourage Nordic to update both API docs and any examples that use app_usbd_cdc_acm_read().

Thanks,

Mark J

[Jørgen Holmefjord]

I think the library documentation explains how the function works pretty similarly to how you describe it.

For the issue with wrong return code when called from the APP_USBD_CDC_ACM_USER_EVT_PORT_OPEN, I was not able to reproduce this with the USBD CDC ACM example in SDK 16. I modified the code to print the return code, and also check the available data in the internal buffer:

case APP_USBD_CDC_ACM_USER_EVT_PORT_OPEN:
{
    bsp_board_led_on(LED_CDC_ACM_OPEN);
    NRF_LOG_INFO("PORT_OPEN Bytes waiting: %d", app_usbd_cdc_acm_bytes_stored(p_cdc_acm));

    /*Setup first transfer*/
    ret_code_t ret = app_usbd_cdc_acm_read(&m_app_cdc_acm,
                                           m_rx_buffer,
                                           READ_SIZE);

    NRF_LOG_INFO("ret: %d", ret);
    UNUSED_VARIABLE(ret);
    break;
}

This is the output:

<info> app: PORT_OPEN Bytes waiting: 0
<info> app: ret: 146

Did you test with the latest SDK?

[Mark J]

Hi Jorgen,

Thanks for the reply. To answer your question - no I have not tested with the latest SDK as I have to move on to getting code delivered. I can work around the known issue in my rev SDK (incorrect return value of NRF_SUCCESS from app_usbd_cdc_acm_read() when handling the APP_USBD_CDC_ACM_USER_EVT_PORT_OPEN).

Regarding the docs, I would suggest that my long winded explanation of the issue is better than the /* Setup first transfer*/ comment in the example. Usually Nordic docs and examples are much better than industry standard, so I was a little surprised at the docs and examples for this API. 

Thanks,

Mark J

[Mark J]

I have updated my cdc_acm_user_ev_handler() implementation to read into a byte and then add that byte to my incoming byte array. Cleaner and easier to understand.

void cdc_acm_user_ev_handler(app_usbd_class_inst_t const * p_inst,
                                    app_usbd_cdc_acm_user_event_t event)
{
  ret_code_t ret;

  app_usbd_cdc_acm_t const * p_cdc_acm = app_usbd_cdc_acm_class_get(p_inst);

  switch (event)
  {
    case APP_USBD_CDC_ACM_USER_EVT_PORT_OPEN:

      SEGGER_RTT_printf(0, "CDC Port Open\n");

      // This call does not actually read any incoming bytes, it simply REQUESTS that
      // the m_rx_byte will be overwritten by 1 incoming byte whenever that byte comes in. 
      // When this byte is received, an APP_USBD_CDC_ACM_USER_EVT_RX_DONE will be generated.
      // Note that NRF_ERROR_IO_PENDING should be returned if no bytes have been recieved
      // but (for some reason) when this call is made within this event type, NRF_SUCCESS always
      // seems to be returned, so ignore the return value.
      app_usbd_cdc_acm_read(&m_app_cdc_acm, &m_rx_byte, 1);
      
      break;

    case APP_USBD_CDC_ACM_USER_EVT_PORT_CLOSE:
      SEGGER_RTT_printf(0, "CDC Port Close\n");
      break;

    case APP_USBD_CDC_ACM_USER_EVT_TX_DONE:
      // DEBUG SEGGER_RTT_printf(0, "CDC Tx Done\n");
      break;
     
    case APP_USBD_CDC_ACM_USER_EVT_RX_DONE:
    {
      // Copy the byte read (that triggered this event) into m_rx_buffer[].
      m_rx_buffer[m_rx_idx++] = m_rx_byte;

      // There may be additional bytes available to read. Continue to process all available bytes. 
      while(true)
      {
        // Read additional bytes 1 at a time. If they are available, NRF_SUCCESS will be returned.
        // If a byte is not available, this app_usbd_cdc_acm_read() call will REQUEST that it is
        // read into m_rx_byte when it comes in. When this happens, this event will be triggered again.
        // Note that even if we have a full pckt received before this read call, it is required so that we can get
        // this APP_USBD_CDC_ACM_USER_EVT_RX_DONE event again when the next pckt is received.
        ret = app_usbd_cdc_acm_read(&m_app_cdc_acm, &m_rx_byte, 1);

        // DEBUG SEGGER_RTT_printf(0, "Read with ret %d and size %d and byte 0x%02X into idx %d\n", ret, size, m_rx_buffer[m_rx_idx], m_rx_idx);
   
        // Check ret.
        if(ret == NRF_SUCCESS) 
        {
          // Update m_rx_buffer[] and keep going as there may be more bytes.
          m_rx_buffer[m_rx_idx++] = m_rx_byte;
        }
        else if(ret == NRF_ERROR_IO_PENDING) 
        { 
          // DEBUG for(int x=0; x<m_rx_idx; x++) { SEGGER_RTT_printf(0, "idx %d: 0x%02X\n", x, m_rx_buffer[x]); }

          // Check the pckt len at m_rx_buffer[1] to see if we have a complete pckt.
          if(m_rx_idx>=2)
          {
            uint8_t pcktLenInPckt =  m_rx_buffer[1];
            if(m_rx_idx >= pcktLenInPckt)
            {
              // Schedule an event on the main thread to process the pckt. Will run usbd_cdc_data_handler() below on main.
              app_sched_event_put(NULL,  0, main_usbd_cdc_sched_handler);
            }
          }
          
          // All done reading data that drove the event.
          return; 
        }
        else
        {
          SEGGER_RTT_printf(0, "app_usbd_cdc_acm_read() error: %d. USB read fail.\n", ret);

          return;
        }
        
      }

      break; // case APP_USBD_CDC_ACM_USER_EVT_RX_DONE:
    }

    default:
      break;
  }
}