-
Notifications
You must be signed in to change notification settings - Fork 2.1k
sys: factor stdio uart functions out of newlib's syscalls #3161
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| /* | ||
| * Copyright (C) 2015 Kaspar Schleiser <[email protected]> | ||
| * | ||
| * This file is subject to the terms and conditions of the GNU Lesser | ||
| * General Public License v2.1. See the file LICENSE in the top level | ||
| * directory for more details. | ||
| */ | ||
|
|
||
| /** | ||
| * @defgroup sys_uart_stdio UART stdio | ||
| * @ingroup sys | ||
| * | ||
| * @brief stdio init/read/write functions for UARTs | ||
| * | ||
| * @{ | ||
| * @file | ||
| * | ||
| * @author Kaspar Schleiser <[email protected]> | ||
| */ | ||
| #ifndef UART_STDIO_H | ||
| #define UART_STDIO_H | ||
|
|
||
| #ifdef __cplusplus | ||
| extern "C" { | ||
| #endif | ||
|
|
||
| /** | ||
| * @brief initialize the module | ||
| */ | ||
| void uart_stdio_init(void); | ||
|
|
||
| /** | ||
| * @brief read @p len bytes from stdio uart into @p buffer | ||
| * | ||
| * @param[out] buffer buffer to read into | ||
| * @param[in] len nr of bytes to read | ||
| * | ||
| * @return nr of bytes read | ||
| * @return <0 on error | ||
| */ | ||
| int uart_stdio_read(char* buffer, int len); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. params and return values are not properly documented - the same applies for the rest
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. params and return values are not properly documented
Is it sufficiently clear if I add @p before len and buffer in brief?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think Doxygen will still warn about undocumented parameters. Please try
to avoid introducing new doxygen warnings in the tree. In the long term we
want to be able to use the CI system to fail on any doxygen warnings, not
only in new files.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think Doxygen will still warn about undocumented parameters.
Again we're writing documentation for Doxygen, not our highly valued users.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think that doxygen will parse this correctly, leading to incomplete API documentation and warnings while building the documentation. Hence, the answer is no. Sorry, bro!
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
What do you think is the purpose of doxygen?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But by letting doxygen alert on missing parameter documentation we can make
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think is the purpose of doxygen?
Is doxygen working for us, or are we working for doxygen?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the C compiler working for us or ware we working for the C compiler? Will you introduce warnings into C code?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removing the source of warnings in C code usually doesn't add uglyness to the compiled program, while in this case in doxygen, it IMHO does. Whatever... |
||
|
|
||
| /** | ||
| * @brief write @p len bytes from @p buffer into uart | ||
| * | ||
| * @param[in] buffer buffer to read from | ||
| * @param[in] len nr of bytes to write | ||
| * | ||
| * @return nr of bytes written | ||
| * @return <0 on error | ||
| */ | ||
| int uart_stdio_write(const char* buffer, int len); | ||
|
|
||
| /** | ||
| * @brief internal callback for periph/uart drivers | ||
| * | ||
| * @param[in] arg (unused) | ||
| * @param[in] data character that has been received | ||
| */ | ||
| void uart_stdio_rx_cb(void *arg, char data); | ||
|
|
||
| #ifdef __cplusplus | ||
| } | ||
| #endif | ||
| /** @} */ | ||
| #endif /* UART_STDIO_H */ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,14 +34,9 @@ | |
| #include "board.h" | ||
| #include "thread.h" | ||
| #include "kernel.h" | ||
| #include "mutex.h" | ||
| #include "ringbuffer.h" | ||
| #include "irq.h" | ||
| #include "periph/uart.h" | ||
|
|
||
| #ifdef MODULE_UART0 | ||
| #include "board_uart0.h" | ||
| #endif | ||
| #include "uart_stdio.h" | ||
|
|
||
| /** | ||
| * @brief manage the heap | ||
|
|
@@ -50,42 +45,12 @@ extern char _sheap; /* start of the heap */ | |
| extern char _eheap; /* end of the heap */ | ||
| caddr_t heap_top = (caddr_t)&_sheap + 4; | ||
|
|
||
| #ifndef MODULE_UART0 | ||
| /** | ||
| * @brief use mutex for waiting on incoming UART chars | ||
| */ | ||
| static mutex_t uart_rx_mutex = MUTEX_INIT; | ||
| static char rx_buf_mem[STDIO_RX_BUFSIZE]; | ||
| static ringbuffer_t rx_buf; | ||
| #endif | ||
|
|
||
| /** | ||
| * @brief Receive a new character from the UART and put it into the receive buffer | ||
| */ | ||
| void rx_cb(void *arg, char data) | ||
| { | ||
| (void)arg; | ||
| #ifndef MODULE_UART0 | ||
| ringbuffer_add_one(&rx_buf, data); | ||
| mutex_unlock(&uart_rx_mutex); | ||
| #else | ||
| if (uart0_handler_pid) { | ||
| uart0_handle_incoming(data); | ||
| uart0_notify_thread(); | ||
| } | ||
| #endif | ||
| } | ||
|
|
||
| /** | ||
| * @brief Initialize NewLib, called by __libc_init_array() from the startup script | ||
| */ | ||
| void _init(void) | ||
| { | ||
| #ifndef MODULE_UART0 | ||
| mutex_lock(&uart_rx_mutex); | ||
| ringbuffer_init(&rx_buf, rx_buf_mem, STDIO_RX_BUFSIZE); | ||
| #endif | ||
| uart_init(STDIO, STDIO_BAUDRATE, rx_cb, 0, 0); | ||
| uart_stdio_init(); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -107,7 +72,7 @@ void _fini(void) | |
| void _exit(int n) | ||
| { | ||
| printf("#! exit %i: resetting\n", n); | ||
| NVIC_SystemReset(); | ||
| reboot(n); | ||
| while(1); | ||
| } | ||
|
|
||
|
|
@@ -204,19 +169,7 @@ int _read_r(struct _reent *r, int fd, void *buffer, unsigned int count) | |
| { | ||
| (void)r; | ||
| (void)fd; | ||
| #ifndef MODULE_UART0 | ||
| int res; | ||
| mutex_lock(&uart_rx_mutex); | ||
| unsigned state = disableIRQ(); | ||
| count = count < rx_buf.avail ? count : rx_buf.avail; | ||
| res = ringbuffer_get(&rx_buf, (char*)buffer, count); | ||
| restoreIRQ(state); | ||
| return res; | ||
| #else | ||
| char *res = (char*)buffer; | ||
| res[0] = (char)uart0_readc(); | ||
| return 1; | ||
| #endif | ||
| return uart_stdio_read(buffer, count); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "Warning unused parameter", add
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (at least until we add any kind of file descriptor support) |
||
| } | ||
|
|
||
| /** | ||
|
|
@@ -238,13 +191,7 @@ int _write_r(struct _reent *r, int fd, const void *data, unsigned int count) | |
| { | ||
| (void) r; | ||
| (void) fd; | ||
| unsigned int i = 0; | ||
|
|
||
| while (i < count) { | ||
| uart_write_blocking(STDIO, ((char*)data)[i++]); | ||
| } | ||
|
|
||
| return (int)i; | ||
| return uart_stdio_write(data, count); | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| include $(RIOTBASE)/Makefile.base |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| /* | ||
| * Copyright (C) 2013 INRIA | ||
| * 2015 Kaspar Schleiser <[email protected]> | ||
| * | ||
| * This file is subject to the terms and conditions of the GNU Lesser | ||
| * General Public License v2.1. See the file LICENSE in the top level | ||
| * directory for more details. | ||
| */ | ||
|
|
||
| /** | ||
| * @ingroup sys | ||
| * @{ | ||
| * | ||
| * @file | ||
| * @brief UART stdio implementation | ||
| * | ||
| * This file implements a UART callback and read/write functions. | ||
| * | ||
| * @author Oliver Hahm <[email protected]> | ||
| * @author Ludwig Ortmann <[email protected]> | ||
| * @author Kaspar Schleiser <[email protected]> | ||
| * | ||
| * @} | ||
| */ | ||
|
|
||
| #include <stdio.h> | ||
|
|
||
| #include "cpu_conf.h" | ||
| #include "ringbuffer.h" | ||
| #include "thread.h" | ||
| #include "mutex.h" | ||
| #include "irq.h" | ||
|
|
||
| #include "periph/uart.h" | ||
|
|
||
| #include "board.h" | ||
|
|
||
| #define ENABLE_DEBUG 0 | ||
| #include "debug.h" | ||
|
|
||
| #ifndef STDIO_RX_BUFSIZE | ||
| #define STDIO_RX_BUFSIZE (64) | ||
| #endif | ||
|
|
||
| /** | ||
| * @brief use mutex for waiting on incoming UART chars | ||
| */ | ||
| static mutex_t _rx_mutex = MUTEX_INIT; | ||
| static char _rx_buf_mem[STDIO_RX_BUFSIZE]; | ||
| static ringbuffer_t _rx_buf; | ||
|
|
||
| /** | ||
| * @brief Receive a new character from the UART and put it into the receive buffer | ||
| */ | ||
| void uart_stdio_rx_cb(void *arg, char data) | ||
| { | ||
| (void)arg; | ||
| ringbuffer_add_one(&_rx_buf, data); | ||
| mutex_unlock(&_rx_mutex); | ||
| } | ||
|
|
||
| void uart_stdio_init(void) | ||
| { | ||
| mutex_lock(&_rx_mutex); | ||
| ringbuffer_init(&_rx_buf, _rx_buf_mem, STDIO_RX_BUFSIZE); | ||
| uart_init(STDIO, STDIO_BAUDRATE, uart_stdio_rx_cb, 0, 0); | ||
| } | ||
|
|
||
| int uart_stdio_read(char* buffer, int count) | ||
| { | ||
| int res; | ||
| mutex_lock(&_rx_mutex); | ||
| unsigned state = disableIRQ(); | ||
| count = (count < _rx_buf.avail) ? count : _rx_buf.avail; | ||
| res = ringbuffer_get(&_rx_buf, (char*)buffer, count); | ||
| restoreIRQ(state); | ||
| return res; | ||
| } | ||
|
|
||
| int uart_stdio_write(const char* buffer, int len) | ||
| { | ||
| unsigned int i = len; | ||
|
|
||
| while (i--) { | ||
| uart_write_blocking(STDIO, *buffer++); | ||
| } | ||
|
|
||
| return len; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason you use a different style of license header here than in the C file? Would be nice if we could keep it consistent throughout RIOT...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On 06/16/15 02:48, Hauke Petersen wrote:
c&p mistake.