Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Makefile.dep
Original file line number Diff line number Diff line change
Expand Up @@ -325,3 +325,7 @@ ifneq (,$(filter cpp11-compat,$(USEMODULE)))
USEMODULE += timex
FEATURES_REQUIRED += cpp
endif

ifneq (,$(filter newlib,$(USEMODULE)))
USEMODULE += uart_stdio
endif
66 changes: 66 additions & 0 deletions sys/include/uart_stdio.h
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.
*/
Copy link
Copy Markdown
Contributor

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...

Copy link
Copy Markdown
Contributor Author

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:

Any reason you use a different style of license header here than in the C file?

c&p mistake.


/**
* @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);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Again we're writing documentation for Doxygen, not our highly valued users.

What do you think is the purpose of doxygen?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

But by letting doxygen alert on missing parameter documentation we can make
sure that the api documents are always up to date with regards to API
changes.
On Jul 7, 2015 4:36 PM, "Kaspar Schleiser" [email protected] wrote:

In sys/include/uart_stdio.h
#3161 (comment):

+#ifndef UART_STDIO_H
+#define UART_STDIO_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**

  • * @brief initialize the module
  • /
    +void uart_stdio_init(void);
    +
    +/
    *
  • * @brief read len bytes from stdio uart into buffer
  • /
    +int uart_stdio_read(char
    buffer, int len);

I think Doxygen will still warn about undocumented parameters.
Again we're writing documentation for Doxygen, not our highly valued users.


Reply to this email directly or view it on GitHub
https://github.com/RIOT-OS/RIOT/pull/3161/files#r34046394.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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 */
63 changes: 5 additions & 58 deletions sys/newlib/syscalls.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
}

/**
Expand All @@ -107,7 +72,7 @@ void _fini(void)
void _exit(int n)
{
printf("#! exit %i: resetting\n", n);
NVIC_SystemReset();
reboot(n);
while(1);
}

Expand Down Expand Up @@ -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);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"Warning unused parameter", add

    (void)r;
    (void)fd;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(at least until we add any kind of file descriptor support)

}

/**
Expand All @@ -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);
}

/**
Expand Down
1 change: 1 addition & 0 deletions sys/uart_stdio/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include $(RIOTBASE)/Makefile.base
89 changes: 89 additions & 0 deletions sys/uart_stdio/uart_stdio.c
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;
}