Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CC2531 as USB CDC-ACM device stop TX data after ignoring it for a while #196

Closed
RongHQ opened this issue Apr 14, 2013 · 6 comments · Fixed by #199
Closed

CC2531 as USB CDC-ACM device stop TX data after ignoring it for a while #196

RongHQ opened this issue Apr 14, 2013 · 6 comments · Fixed by #199
Labels

Comments

@RongHQ
Copy link
Contributor

RongHQ commented Apr 14, 2013

In my code some data is sent via USB to host periodically. If I plug the CC2531 device without running Putty to receive data, then after a while, open Putty, no data can be received. I guess some buffer overflows, and there are no proper code to recover from it. I made a simple patch but if someone have a way to make the device be able to recover from the overflow that will be better.

My solution now is to add these code:

if(!(usb_cdc_acm_get_line_state() & 0x01)){
    leds_toggle(LEDS_RED);
    return;
 }

to function usb_serial_writeb function in usb-serial.c .

So no single byte can be written to usb_tx_data[] buffer if DTR in line state word is not set.

I think this can possibly prevent some buffer from overflow if no application is reading from the USB. This patch is tested and it works, and host computer is using Putty or cat on Ubuntu 12.04.

But if someone have a way to make the device be able to recover from the overflow that will be better.

@g-oikonomou
Copy link
Contributor

Rong, I have observed similar behaviour and I have changed the driver a bit to take better consideration of the cdc acm line state. I am hoping to include this fix with my next cc2530-related pull request. When this happens, I'll appreciate it if you can take some time to test a little and give feedback.

Quick question, what OS is that behaviour on? You said putty so is this win or linux? I would also like to know if it's native or a VM. If it's on win, what driver are you using?

Thanks for mentioning this, at least now I know it's not just me.

On 14 Apr 2013, at 15:20, RONG Hongqian notifications@github.com wrote:

In my code some data is sent via USB to host periodically. If I plug the CC2531 device without running Putty to receive data, then after a while, open Putty, no data can be received. I made a simple patch but if someone have a way to make the device be able to recover from the overflow that will be better.

I guess some buffer overflows, and there are no proper code to recover from it. My solution now is to add these code:

if(!(usb_cdc_acm_get_line_state() & 0x01)){
leds_toggle(LEDS_RED);
return;
}

to function usb_serial_writeb function in usb-serial.c .

So not single byte can be written to usb_tx_data[] buffer if DTR in line state word is not set.

I think this can possibly prevent some buffer from overflow if no application is reading from the USB. This patch is tested and it works, and host computer is using Putty or cat on Ubuntu 12.04.

But if someone have a way to make the device be able to recover from the overflow that will be better.


Reply to this email directly or view it on GitHub.

@RongHQ
Copy link
Contributor Author

RongHQ commented Apr 14, 2013

@g-oikonomou I have observed this behaviour on native Ubuntu 12.04, and also Ubuntu 12.04 in VM, when using Putty or cat /dev/ttyACM0 . With my very simple patch this behaviour disappears, tested using the Ubuntu in VM. I will test it later on the native Ubuntu.

@g-oikonomou
Copy link
Contributor

Can you try this patch and let me know whether it improves the situation? Cheers

$ git show HEAD
commit e4a3cb3db13dff253258f7f2188777777a16e851
Author: George Oikonomou <g.oikonomou@bristol.ac.uk>
Date:   Sat Mar 23 18:51:36 2013 +0000

    Receive CDC ACM events and act accordingly

diff --git a/platform/cc2530dk/dev/usb-serial.c b/platform/cc2530dk/dev/usb-serial.c
index 4f766ab..a0cfea1 100644
--- a/platform/cc2530dk/dev/usb-serial.c
+++ b/platform/cc2530dk/dev/usb-serial.c
@@ -170,6 +170,16 @@ do_work(void)
     enabled = 0;
   }

+  events = usb_cdc_acm_get_events();
+  if(events & USB_CDC_ACM_LINE_STATE) {
+    uint8_t line_state = usb_cdc_acm_get_line_state();
+    if(line_state == (USB_CDC_ACM_DTE | USB_CDC_ACM_RTS)) {
+      enabled = 1;
+    } else {
+      enabled = 0;
+    }
+  }
+
   if(!enabled) {
     return;
   }
@@ -254,6 +264,7 @@ PROCESS_THREAD(usb_serial_process, ev, data)
   usb_setup();
   usb_cdc_acm_setup();
   usb_set_global_event_process(&usb_serial_process);
+  usb_cdc_acm_set_event_process(&usb_serial_process);
   usb_set_ep_event_process(EPIN, &usb_serial_process);
   usb_set_ep_event_process(EPOUT, &usb_serial_process);


@RongHQ
Copy link
Contributor Author

RongHQ commented Apr 16, 2013

Your patch works. I tested cat and putty on Ubuntu 12.04 in VM, and on native Ubuntu 12.04. The behaviour disappears in the tests. Cheers!

My suggestion is to change the

if(line_state == (USB_CDC_ACM_DTE | USB_CDC_ACM_RTS)) 

to

if(line_state & USB_CDC_ACM_DTE )

In Universal Serial Bus Class Definitions for Communication Devices v1.1 (available http://www.usb.org/developers/devclass_docs/usbcdc11.pdf), Page 58( Page69 in pdf), D15..D2 in control signal bitmap are reserved. Also, D1 (USB_CDC_ACM_RTS) is ignored if device is full duplex.

@g-oikonomou
Copy link
Contributor

Your patch works. I tested cat and putty on Ubuntu 12.04 in VM, and on native Ubuntu 12.04. The behaviour disappears in the tests. Cheers!

Great, thanks for testing
My suggestion is to change the

if(line_state == (USB_CDC_ACM_DTE | USB_CDC_ACM_RTS))
to

if(line_state & USB_CDC_ACM_DTE )
That's a valid point, I didn't look at the spec when I wrote this, I was just going by what the host was sending me. Thanks for pointing this out.

g-oikonomou added a commit to g-oikonomou/contiki that referenced this issue Apr 16, 2013
- For the CC2538, simplify handling of USB_CDC_ACM_LINE_STATE
  events. Ignore the Carrier Control (RTS) bit when receiving
  a SET_CONTROL_LINE _STATE request, we are a full duplex device.
- Improve behaviour of the CC2531 USB stick when there is no
  process on the host to read IN data. Basically, we adopt the
  CC2538 approach and we only send data when a DTE is present

Closes contiki-os#196
@g-oikonomou
Copy link
Contributor

Fixed with #199

remyleone pushed a commit to remyleone/contiki that referenced this issue Mar 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants