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

USBUS: CDC ACM to UART functionality with example. #12268

Closed
wants to merge 10 commits into from

Conversation

bergzand
Copy link
Member

Contribution description

This PR adds integration for UART peripherals for the USBUS CDC ACM (serial over usb) functionality. It allows for transfering data over serial, configuring baud rate and parity bits of the UART peripheral from the host computer.

Testing procedure

Run examples/usbus_uart_adapter. When attached to a host computer, it should provide multiple (depending on the board) ttyACM devices of which the first one is connected to the RIOT shell and the others are proxied to the UART peripherals on the board.

On a samr21-xpro, using make list-ttys this shows up as:

/sys/bus/usb/devices/3-9.2.3: RIOT-os.org USB device serial: '', tty(s): ttyACM3, ttyACM2, ttyACM4

In this case, ttyACM3 is the first serial function and is connected to the RIOT shell.

Issues/PRs references

Depends on #11085

@bergzand bergzand added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: USB Area: Universal Serial Bus labels Sep 17, 2019
@bergzand bergzand requested a review from dylad September 17, 2019 21:07
@bergzand
Copy link
Member Author

bergzand commented Oct 1, 2019

Rebased now that #11085 is merged, all dependencies are merged

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 1, 2019
@leandrolanzieri
Copy link
Contributor

I'm getting:
arm-none-eabi-gcc: error: /home/leandro/Work/RIOT/examples/usbus_uart_adapter/bin/samr21-xpro/usbus_cdc_acm_uart.a: No such file or directory

Should usbus_cdc_acm_uart be a PSEUDOMODULE?

@bergzand
Copy link
Member Author

bergzand commented Oct 2, 2019

@leandrolanzieri That's probably the case.

I have some time this evening to get this PR into proper shape again.

@leandrolanzieri leandrolanzieri removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 2, 2019
@stale
Copy link

stale bot commented Apr 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Apr 4, 2020
@stale stale bot removed State: stale State: The issue / PR has no activity for >185 days labels Apr 24, 2020
@bergzand
Copy link
Member Author

Rebased and reworked into a better shape

@bergzand
Copy link
Member Author

Should usbus_cdc_acm_uart be a PSEUDOMODULE?

Also fixed this

@bergzand
Copy link
Member Author

I've replaced the stdio_cdc_acm module previously used here with stdio_null. This way all UART devices on the board can be used for USB serial operation and 2 additional endpoints can be used for UART CDC ACM instead of having to use those for stdio.

@bergzand bergzand added this to Backlog / Proposals in RIOT Sprint Day via automation Apr 24, 2020
@leandrolanzieri leandrolanzieri added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 24, 2020
Comment on lines +92 to +94
if (data == '\n') {
usbus_cdc_acm_flush(&acmuart->cdcacm);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about transmit byte(s) as soon as they arrive in order to be more like a USB-to-UART adapter ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those send chunks of bytes too

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Koen is there any timeout here, or would this essentially be line based?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something timeout-based would be better here indeed. I'll try to come up with something.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would imagine you could receive bytes on the UART faster than you can submit USB packets.

So for the best performance, you would have double-buffering and

  • fill the RX buffer in the UART callback
  • in a thread, check if data is in the RX buffer, if so swap buffers and transfer the buffer over USB

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it seems line based. Is it very inefficient to flush every byte? It probably is, so some timeout would be needed.

Somethink like:

  1. flush on every LF ('\r' instead of '\n', as terminals either send LF or CRLF).
  2. flush every N bytes (16, 32, ...)
  3. flush every M ms (with M being about the time it takes to transfer N bytes?)

maybe 3. is sufficient?

Chunking might cause problems, if the sender sends with a deliberate pause to not overload a shitty UART implementation, but the adapter sends full-speed N byte bursts...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I prefer the flush after timeout and possible after every newline.

@kaspar030 kaspar030 moved this from Backlog / Proposals to In progress in RIOT Sprint Day Jun 9, 2020
@kaspar030 kaspar030 self-assigned this Jun 9, 2020
@kaspar030
Copy link
Contributor

I've flashed that uart thingy on a particle xenon. my linux cannot enumerate it.

[372332.182197] usb 1-1.1: new full-speed USB device number 53 using xhci_hcd
[372347.579939] usb 1-1.1: device descriptor read/64, error -110

lsusb hangs

@bergzand
Copy link
Member Author

bergzand commented Jun 9, 2020

I've flashed that uart thingy on a particle xenon. my linux cannot enumerate it.

[372332.182197] usb 1-1.1: new full-speed USB device number 53 using xhci_hcd
[372347.579939] usb 1-1.1: device descriptor read/64, error -110

lsusb hangs

I can reproduce this on the nRF52840dk, I'm trying to figure out if this is caused by this PR or if it is broken in general.

@bergzand
Copy link
Member Author

bergzand commented Jun 9, 2020

I can reproduce this on the nRF52840dk, I'm trying to figure out if this is caused by this PR or if it is broken in general.

The CDC ECM test (tests/usbus_cdc_ecm) works fine, I guess this is caused by this PR. 😢

@kaspar030
Copy link
Contributor

I got a nucleo-f767zi. stlink uart and mcu usb connected. that should loopback. when I type into the terminal that's connected to the MCU, chars arrive at stlink terminal. but not the other way around.

@kaspar030
Copy link
Contributor

I got a nucleo-f767zi. stlink uart and mcu usb connected. that should loopback. when I type into the terminal that's connected to the MCU, chars arrive at stlink terminal. but not the other way around.

I think this is related to the flushing. I changed my terminal to convert cr to crlf, now on enter, data arrives on the other side.

@bergzand
Copy link
Member Author

bergzand commented Jun 9, 2020

I've flashed that uart thingy on a particle xenon. my linux cannot enumerate it.

[372332.182197] usb 1-1.1: new full-speed USB device number 53 using xhci_hcd
[372347.579939] usb 1-1.1: device descriptor read/64, error -110

lsusb hangs

Fixed with the latest commit

@bergzand bergzand removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jun 9, 2020
@benpicco
Copy link
Contributor

benpicco commented Jul 3, 2020

I flashed this on samr21-xpro, I connected mico-USB cables to both Debug-USB (/dev/ttyACM0) as well as Target-USB (/dev/ttyACM1, /dev/ttyACM2).

The Debug Adapter UART (/dev/ttyACM0) is connected to the samr21 UART exported as /dev/ttyACM1.
So when I write to either of those with pyterm, I should receive it on the other one.

However, this only works in one direction:

  • writing Hello from Linux to /dev/ttyACM0 is received on /dev/ttyACM1
  • writing Hello from RIOT to /dev/ttyACM1 is not received on /dev/ttyACM0

@jdavid
Copy link
Contributor

jdavid commented Nov 4, 2020

I'm trying to add the arduino feature to the feather-m0 board but Serial.print doesn't work. From what I've seen I think that the feather-m0 uses sys/usb/usbus/cdc/acm for printing; this also matches what I've seen in Arduino: #define Serial SerialUSB.

Reading the comments here it looks like I need this PR to get Serial.print working with the feather-m0, is that correct?

@jdavid jdavid mentioned this pull request Nov 4, 2020
6 tasks
@benpicco benpicco removed this from In progress in RIOT Sprint Day Feb 1, 2021
@stale
Copy link

stale bot commented Jun 2, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Jun 2, 2021
@stale stale bot closed this Jul 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: USB Area: Universal Serial Bus State: stale State: The issue / PR has no activity for >185 days Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants