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

native: add UART driver based on /dev/tty #4443

Merged
merged 2 commits into from
Mar 31, 2016

Conversation

Yonezawa-T2
Copy link
Contributor

uart0 functionality is removed by #3164. This PR implements periph/uart, rather than deprecated uart0, using /dev/tty. A TTY device is specified on command line with -c (COM) option, since -t was used by the old implementation.

To use with netdev2_tap simultaneously, this PR adds asynchronus read system and modifies netdev2_tap to use it. It monitors multiple file descriptors by fcntl O_ASYNC option on non OS X system and by select system call on OS X system. It forks child process on OS X system.

I'm struggling to make my Mac with XBee connected by USB to be a border router of 6LoWPAN and made several fixes. Other fixes will be filled as separate pull requests. This PR also implements empty GPIO driver needed by the xbee driver.

Tested on OS X.

@OlegHahm OlegHahm added this to the Release 2016.03 milestone Dec 9, 2015
@OlegHahm OlegHahm added Platform: native Platform: This PR/issue effects the native platform Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: drivers Area: Device drivers labels Dec 9, 2015
@OlegHahm
Copy link
Member

OlegHahm commented Dec 9, 2015

Very nice PR - this feature is very much missing for a long time.

@kaspar030
Copy link
Contributor

Yep. @Yonezawa-T2 Thanks for getting into this!

@Yonezawa-T2
Copy link
Contributor Author

We have to update the wiki page when this PR is merged.

@Yonezawa-T2
Copy link
Contributor Author

I met a problem with netdev2_tap. It deadlocks when there are no spaces in pktbuf (for example, when packets are arriving too quickly).

For OS X, the child process monitoring file descriptors are suspended until explicitly resumed by the _recv function of netdev2_tap. When gnrc_netdev2_eth failed to allocate pktbuf, it does not call _recv function of netdev2_tap.

Possible solutions:

  • A) notifying error from gnrc_netdev2_eth to netdev2_tap to cleanup things
  • B) timeouting the suspension in async_read.c
  • C) checking memory availability in netdev2_tap

A)
Pros: cleanest way to do it
Cons: adds extra API for netdev2 driver

B)
Pros: no API changes
Pros: robust
Cons: OS X does not have sigtimedwait
Cons: suspends until timeout

C)
Pros: no API changes
Pros: resumes quickly
Cons: checking memory availability may be costly
Cons: relying on internal of gnrc_netdev2_eth (i.e. how much memory it requires)
Reference implementation: Yonezawa-T2@5a750a4

Which is better? Any alternatives?

@miri64 miri64 assigned kaspar030 and unassigned LudwigKnuepfer Dec 21, 2015
@miri64
Copy link
Member

miri64 commented Dec 21, 2015

Reassigned to @kaspar030 since he is Mr. netdev2_eth ;)

@kaspar030
Copy link
Contributor

kaspar030 commented Dec 21, 2015 via email

@kaspar030
Copy link
Contributor

C) doesn't seem like a good option as it messes with the layering (netdev2 itself was designed to be independent of gnrc, so we can't call any gnrc_* stuff there).

@kaspar030
Copy link
Contributor

B) seems tap specific.

@Yonezawa-T2
Copy link
Contributor Author

Implemented A.

@Yonezawa-T2
Copy link
Contributor Author

Rebased and resolved conflict with #3984.

@OlegHahm
Copy link
Member

OlegHahm commented Feb 2, 2016

Can you tell me how this is supposed to work? Where should uart_init()be called?

@Yonezawa-T2
Copy link
Contributor Author

@OlegHahm For example, when using XBee, uart_init is called from xbee_init:

  • auto_init
  • → auto_init_xbee (if xbee module is enabled)
  • → xbee_init
  • → uart_init

uart_init opens a TTY device file and registers a callback function, which is called from interrupt handler for SIGIO.

TTY devices are specified by command-line option -c, that are handled by cpu/native/startup.c, registered by tty_uart_setup. Multiple devices are allowed by repeating -c option. uart_init must be called after TTY devices are registered.

SIGIO is shared with netdev2_tap using new async_read.c, which uses select to detect which file descriptor is readable and which callback function should be invoked.

@Yonezawa-T2
Copy link
Contributor Author

rebased

@Yonezawa-T2
Copy link
Contributor Author

Rebased and updated GPIO function signatures.

@kYc0o kYc0o added the Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties label Mar 28, 2016
@OlegHahm
Copy link
Member

@kYc0o, @emmanuelsearch, @thomaseichinger, can you test?

@kYc0o
Copy link
Contributor

kYc0o commented Mar 29, 2016

Yes we can!

@kYc0o
Copy link
Contributor

kYc0o commented Mar 29, 2016

I'm having this on native with Apple LLVM version 7.3.0 (clang-703.0.29):

Building application "periph_uart" for "native" with MCU "native".

/Users/facosta/git/RIOT2/RIOT/tests/periph_uart/main.c:59:24: error: comparison of integers of different signs: 'int' and 'unsigned int' [-Werror,-Wsign-compare]
    if (dev < 0 || dev >= UART_NUMOF) {
                   ~~~ ^  ~~~~~~~~~~
/Users/facosta/git/RIOT2/RIOT/tests/periph_uart/main.c:182:23: error: comparison of integers of different signs: 'int' and 'unsigned int' [-Werror,-Wsign-compare]
    for (int i = 0; i < UART_NUMOF; i++) {

@Yonezawa-T2
Copy link
Contributor Author

Fixed and rebased.

@kYc0o
Copy link
Contributor

kYc0o commented Mar 30, 2016

Tested and working correctly, waiting for Murdock

@kYc0o kYc0o added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 30, 2016
@Yonezawa-T2
Copy link
Contributor Author

Created #5198 and #5199 for Murdock errors.

@Yonezawa-T2 Yonezawa-T2 added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Mar 30, 2016
@OlegHahm
Copy link
Member

Both are merged. Please rebase.

@Yonezawa-T2
Copy link
Contributor Author

Rebased.

@OlegHahm
Copy link
Member

Let's try to merge this before Sunday (another exception from the feature freeze).

uart0 functionality is removed by RIOT-OS#3164. This patch implements periph/uart,
rather than deprecated uart0, using /dev/tty.
To use with netdev2_tap simultaneously, this patch adds asynchronus read system
and modifies netdev2_tap to use it.

A TTY device is specified on command line with -c (COM) option, since -t was
used by the old implementation.

This patch also implements empty GPIO driver needed by the xbee driver.
On OS X, `netdev2_tap` suspends monitoring file descriptor until `_recv` is
called. If no spaces in left in pktbuf, `gnrc_netdev2_eth` does not call `_recv`
that results in deadlock.

With this commit, `gnrc_netdev2_eth` calls `_recv` with NULL buffer and non-zero
length parameter, that indicates the driver to drop frame and resume working.
@Yonezawa-T2 Yonezawa-T2 removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Mar 31, 2016
@Yonezawa-T2
Copy link
Contributor Author

Murdock passed (except NEEDS SQUASHING static-test). Squashed.

@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 31, 2016
@kYc0o
Copy link
Contributor

kYc0o commented Mar 31, 2016

Murdock happy -> GO!

@kYc0o kYc0o merged commit ecf7b46 into RIOT-OS:master Mar 31, 2016
@Yonezawa-T2
Copy link
Contributor Author

@OlegHahm
Copy link
Member

OlegHahm commented Apr 1, 2016

Cool - many thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties Platform: native Platform: This PR/issue effects the native platform 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