Skip to content

Fix serial stdio - add new get_stdio_serial() #2466

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

Fix serial stdio - add new get_stdio_serial() #2466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7f80e45
stdio serial object - get_stdio_serial() that returns Serial&
0xc0170 Aug 17, 2016
526e731
stdio copy in HAL removal
0xc0170 Aug 17, 2016
efea142
board - fix vfprintf - use putc() instead of serial stdio
0xc0170 Aug 16, 2016
8db5b87
retarget - stdio serial new placement for Serial
0xc0170 Aug 17, 2016
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
7 changes: 7 additions & 0 deletions hal/api/Serial.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,13 @@ class Serial : public SerialBase, public Stream {
PlatformMutex _mutex;
};

/** Get the stdio Serial object, which is lazy instantiated. There's only
* one stdio object that should be used within an application.
*
* @returns stdio object
*/
Serial& get_stdio_serial();
Copy link
Contributor

@geky geky Aug 17, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Small question, should this be a reference? This could confuse the ownership of the Serial object, and I'm not sure what the benefit is over Serial*.

With Serial& it's easy to accidentally do this:

void maximize_baud() {
    Serial s = get_stdio_serial();
    s.baud(96000000);
    // s's destructor is called, possibly deinitializing serial
}

Copy link
Member

Choose a reason for hiding this comment

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

I believe it should be a reference, a pointer might be NULL, a reference is never null.
Your question leverage another one, why the Serial class is copy constructible ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With Serial& it's easy to accidentally do this:

That's a user error, we can protect it as @pan- highlighted it's about copy ctor. I'll have a look at it


} // namespace mbed

#endif
Expand Down
10 changes: 1 addition & 9 deletions hal/common/mbed_board.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@
#include "critical.h"
#include "serial_api.h"

#if DEVICE_SERIAL
extern int stdio_uart_inited;
extern serial_t stdio_uart;
#endif

WEAK void mbed_die(void) {
#if !defined (NRF51_H) && !defined(TARGET_EFM32)
core_util_critical_section_enter();
Expand Down Expand Up @@ -79,11 +74,8 @@ void mbed_error_vfprintf(const char * format, va_list arg) {
char buffer[128];
int size = vsprintf(buffer, format, arg);
if (size > 0) {
if (!stdio_uart_inited) {
serial_init(&stdio_uart, STDIO_UART_TX, STDIO_UART_RX);
}
for (int i = 0; i < size; i++) {
serial_putc(&stdio_uart, buffer[i]);
putc(buffer[i], stdout);
}
}
core_util_critical_section_exit();
Expand Down
49 changes: 30 additions & 19 deletions hal/common/retarget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "SingletonPtr.h"
#include "PlatformMutex.h"
#include "mbed_error.h"
#include "Serial.h"
#include <stdlib.h>
#if DEVICE_STDIO_MESSAGES
#include <stdio.h>
Expand Down Expand Up @@ -94,24 +95,36 @@ FileHandle::~FileHandle() {
}

#if DEVICE_SERIAL
extern int stdio_uart_inited;
extern serial_t stdio_uart;
#if MBED_CONF_CORE_STDIO_CONVERT_NEWLINES
static char stdio_in_prev;
static char stdio_out_prev;
#endif
#endif

static void init_serial() {
#if DEVICE_SERIAL
if (stdio_uart_inited) return;
serial_init(&stdio_uart, STDIO_UART_TX, STDIO_UART_RX);
namespace mbed {

static SingletonPtr<PlatformMutex> mutex_stdio_serial;

Serial& get_stdio_serial()
{
static bool stdio_inited = false;
static unsigned stdio_uart[(sizeof(Serial) + sizeof(unsigned) - 1) / sizeof(unsigned)];

if (!stdio_inited) {
mutex_stdio_serial->lock();
new (stdio_uart) Serial(STDIO_UART_TX, STDIO_UART_RX);
mutex_stdio_serial->unlock();
#if MBED_CONF_CORE_STDIO_BAUD_RATE
serial_baud(&stdio_uart, MBED_CONF_CORE_STDIO_BAUD_RATE);
#endif
reinterpret_cast<Serial*>(stdio_uart)->baud(MBED_CONF_CORE_STDIO_BAUD_RATE);
#endif
stdio_inited = true;
}
return reinterpret_cast<Serial&>(stdio_uart);
}

} // namespace mbed

#endif

static inline int openmode_to_posix(int openmode) {
int posix = openmode;
#ifdef __ARMCC_VERSION
Expand Down Expand Up @@ -158,13 +171,13 @@ extern "C" FILEHANDLE PREFIX(_open)(const char* name, int openmode) {
/* Use the posix convention that stdin,out,err are filehandles 0,1,2.
*/
if (std::strcmp(name, __stdin_name) == 0) {
init_serial();
get_stdio_serial();
return 0;
} else if (std::strcmp(name, __stdout_name) == 0) {
init_serial();
get_stdio_serial();
return 1;
} else if (std::strcmp(name, __stderr_name) == 0) {
init_serial();
get_stdio_serial();
return 2;
}
#endif
Expand Down Expand Up @@ -240,18 +253,17 @@ extern "C" int PREFIX(_write)(FILEHANDLE fh, const unsigned char *buffer, unsign
int n; // n is the number of bytes written
if (fh < 3) {
#if DEVICE_SERIAL
if (!stdio_uart_inited) init_serial();
#if MBED_CONF_CORE_STDIO_CONVERT_NEWLINES
for (unsigned int i = 0; i < length; i++) {
if (buffer[i] == '\n' && stdio_out_prev != '\r') {
serial_putc(&stdio_uart, '\r');
get_stdio_serial().putc('\r');
}
serial_putc(&stdio_uart, buffer[i]);
get_stdio_serial().putc(buffer[i]);
stdio_out_prev = buffer[i];
}
#else
for (unsigned int i = 0; i < length; i++) {
serial_putc(&stdio_uart, buffer[i]);
get_stdio_serial().putc(buffer[i]);
}
#endif
#endif
Expand All @@ -278,10 +290,9 @@ extern "C" int PREFIX(_read)(FILEHANDLE fh, unsigned char *buffer, unsigned int
if (fh < 3) {
// only read a character at a time from stdin
#if DEVICE_SERIAL
if (!stdio_uart_inited) init_serial();
#if MBED_CONF_CORE_STDIO_CONVERT_NEWLINES
while (true) {
char c = serial_getc(&stdio_uart);
char c = get_stdio_serial().getc();
if ((c == '\r' && stdio_in_prev != '\n') ||
(c == '\n' && stdio_in_prev != '\r')) {
stdio_in_prev = c;
Expand All @@ -299,7 +310,7 @@ extern "C" int PREFIX(_read)(FILEHANDLE fh, unsigned char *buffer, unsigned int
}
}
#else
*buffer = serial_getc(&stdio_uart);
*buffer = get_stdio_serial().getc();
#endif
#endif
n = 1;
Expand Down
10 changes: 0 additions & 10 deletions hal/targets/hal/TARGET_ARM_SSG/TARGET_BEETLE/serial_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@ static const PinMap PinMap_UART_RX[] = {

static uart_irq_handler irq_handler;

int stdio_uart_inited = 0;
serial_t stdio_uart;

struct serial_global_data_s {
uint32_t serial_irq_id;
gpio_t sw_rts, sw_cts;
Expand Down Expand Up @@ -113,13 +110,6 @@ void serial_init(serial_t *obj, PinName tx, PinName rx) {
uart_data[obj->index].sw_rts.pin = NC;
uart_data[obj->index].sw_cts.pin = NC;
serial_set_flow_control(obj, FlowControlNone, NC, NC);

is_stdio_uart = (uart == STDIO_UART) ? (1) : (0);

if (is_stdio_uart) {
stdio_uart_inited = 1;
memcpy(&stdio_uart, obj, sizeof(serial_t));
}
}

void serial_free(serial_t *obj) {
Expand Down
10 changes: 0 additions & 10 deletions hal/targets/hal/TARGET_ARM_SSG/TARGET_IOTSS/serial_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,6 @@ static const PinMap PinMap_UART_RX[] = {

static uart_irq_handler irq_handler;

int stdio_uart_inited = 0;
serial_t stdio_uart;

struct serial_global_data_s {
uint32_t serial_irq_id;
gpio_t sw_rts, sw_cts;
Expand Down Expand Up @@ -156,13 +153,6 @@ void serial_init(serial_t *obj, PinName tx, PinName rx) {
uart_data[obj->index].sw_rts.pin = NC;
uart_data[obj->index].sw_cts.pin = NC;
serial_set_flow_control(obj, FlowControlNone, NC, NC);

is_stdio_uart = (uart == STDIO_UART) ? (1) : (0);

if (is_stdio_uart) {
stdio_uart_inited = 1;
memcpy(&stdio_uart, obj, sizeof(serial_t));
}
}

void serial_free(serial_t *obj) {
Expand Down
10 changes: 0 additions & 10 deletions hal/targets/hal/TARGET_ARM_SSG/TARGET_MPS2/serial_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,6 @@ static const PinMap PinMap_UART_RX[] = {

static uart_irq_handler irq_handler;

int stdio_uart_inited = 0;
serial_t stdio_uart;

struct serial_global_data_s {
uint32_t serial_irq_id;
gpio_t sw_rts, sw_cts;
Expand Down Expand Up @@ -158,13 +155,6 @@ void serial_init(serial_t *obj, PinName tx, PinName rx) {
uart_data[obj->index].sw_rts.pin = NC;
uart_data[obj->index].sw_cts.pin = NC;
serial_set_flow_control(obj, FlowControlNone, NC, NC);

is_stdio_uart = (uart == STDIO_UART) ? (1) : (0);

if (is_stdio_uart) {
stdio_uart_inited = 1;
memcpy(&stdio_uart, obj, sizeof(serial_t));
}
}

void serial_free(serial_t *obj) {
Expand Down
7 changes: 0 additions & 7 deletions hal/targets/hal/TARGET_Atmel/TARGET_SAM_CortexM0P/serial_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@ void uart5_irq(void);
static uint32_t serial_irq_ids[USART_NUM] = {0};
static uart_irq_handler irq_handler;

int stdio_uart_inited = 0;
serial_t stdio_uart;

extern uint8_t g_sys_init;

static inline void usart_syncing(serial_t *obj)
Expand Down Expand Up @@ -329,10 +326,6 @@ void serial_init(serial_t *obj, PinName tx, PinName rx)
}
}

if (uart == STDIO_UART) {
stdio_uart_inited = 1;
memcpy(&stdio_uart, obj, sizeof(serial_t));
}
/* Wait until synchronization is complete */
usart_syncing(obj);

Expand Down
11 changes: 1 addition & 10 deletions hal/targets/hal/TARGET_Atmel/TARGET_SAM_CortexM4/serial_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,6 @@ static void uart5_irq(void);
static void uart6_irq(void);
static void uart7_irq(void);


int stdio_uart_inited = 0;
serial_t stdio_uart;

extern uint8_t g_sys_init;

static int get_usart_clock_id(UARTName peripheral)
Expand Down Expand Up @@ -192,11 +188,6 @@ void serial_init(serial_t *obj, PinName tx, PinName rx)
if(rx != NC) {
usart_enable_rx((Usart*)uart);
}

if(uart == STDIO_UART) {
stdio_uart_inited = 1;
memcpy(&stdio_uart, obj, sizeof(serial_t));
}
}

void serial_free(serial_t *obj)
Expand Down Expand Up @@ -783,4 +774,4 @@ void serial_rx_abort_asynch(serial_t *obj)
pSERIAL_S(obj)->actrec = false;
}

#endif
#endif
8 changes: 0 additions & 8 deletions hal/targets/hal/TARGET_Freescale/TARGET_KLXX/TARGET_KL05Z/serial_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,6 @@
static uint32_t serial_irq_ids[UART_NUM] = {0};
static uart_irq_handler irq_handler;

int stdio_uart_inited = 0;
serial_t stdio_uart;

void serial_init(serial_t *obj, PinName tx, PinName rx) {
// determine the UART to use
UARTName uart_tx = (UARTName)pinmap_peripheral(tx, PinMap_UART_TX);
Expand Down Expand Up @@ -104,11 +101,6 @@ void serial_init(serial_t *obj, PinName tx, PinName rx) {
}

obj->uart->C2 |= (UARTLP_C2_RE_MASK | UARTLP_C2_TE_MASK);

if (uart == STDIO_UART) {
stdio_uart_inited = 1;
memcpy(&stdio_uart, obj, sizeof(serial_t));
}
}

void serial_free(serial_t *obj) {
Expand Down
8 changes: 0 additions & 8 deletions hal/targets/hal/TARGET_Freescale/TARGET_KLXX/TARGET_KL25Z/serial_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@
static uint32_t serial_irq_ids[UART_NUM] = {0};
static uart_irq_handler irq_handler;

int stdio_uart_inited = 0;
serial_t stdio_uart;

void serial_init(serial_t *obj, PinName tx, PinName rx) {
// determine the UART to use
UARTName uart_tx = (UARTName)pinmap_peripheral(tx, PinMap_UART_TX);
Expand Down Expand Up @@ -115,11 +112,6 @@ void serial_init(serial_t *obj, PinName tx, PinName rx) {
obj->uart->C2 |= UARTLP_C2_RE_MASK;
pin_mode(rx, PullUp);
}

if (uart == STDIO_UART) {
stdio_uart_inited = 1;
memcpy(&stdio_uart, obj, sizeof(serial_t));
}
}

void serial_free(serial_t *obj) {
Expand Down
8 changes: 0 additions & 8 deletions hal/targets/hal/TARGET_Freescale/TARGET_KLXX/TARGET_KL26Z/serial_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@
static uint32_t serial_irq_ids[UART_NUM] = {0};
static uart_irq_handler irq_handler;

int stdio_uart_inited = 0;
serial_t stdio_uart;

void serial_init(serial_t *obj, PinName tx, PinName rx) {
// determine the UART to use
UARTName uart_tx = (UARTName)pinmap_peripheral(tx, PinMap_UART_TX);
Expand Down Expand Up @@ -115,11 +112,6 @@ void serial_init(serial_t *obj, PinName tx, PinName rx) {
obj->uart->C2 |= UARTLP_C2_RE_MASK;
pin_mode(rx, PullUp);
}

if (uart == STDIO_UART) {
stdio_uart_inited = 1;
memcpy(&stdio_uart, obj, sizeof(serial_t));
}
}

void serial_free(serial_t *obj) {
Expand Down
8 changes: 0 additions & 8 deletions hal/targets/hal/TARGET_Freescale/TARGET_KLXX/TARGET_KL43Z/serial_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@
static uint32_t serial_irq_ids[UART_NUM] = {0};
static uart_irq_handler irq_handler;

int stdio_uart_inited = 0;
serial_t stdio_uart;

static inline uint32_t serial_get_src_clock(serial_t *obj) {
uint32_t mux, srcclk;

Expand Down Expand Up @@ -108,11 +105,6 @@ void serial_init(serial_t *obj, PinName tx, PinName rx) {
if (rx != NC) pin_mode(rx, PullUp);

obj->uart->CTRL |= (LPUART_CTRL_RE_MASK | LPUART_CTRL_TE_MASK);

if (uart == STDIO_UART) {
stdio_uart_inited = 1;
memcpy(&stdio_uart, obj, sizeof(serial_t));
}
}

void serial_free(serial_t *obj) {
Expand Down
8 changes: 0 additions & 8 deletions hal/targets/hal/TARGET_Freescale/TARGET_KLXX/TARGET_KL46Z/serial_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@
static uint32_t serial_irq_ids[UART_NUM] = {0};
static uart_irq_handler irq_handler;

int stdio_uart_inited = 0;
serial_t stdio_uart;

void serial_init(serial_t *obj, PinName tx, PinName rx) {
// determine the UART to use
UARTName uart_tx = (UARTName)pinmap_peripheral(tx, PinMap_UART_TX);
Expand Down Expand Up @@ -115,11 +112,6 @@ void serial_init(serial_t *obj, PinName tx, PinName rx) {
obj->uart->C2 |= UARTLP_C2_RE_MASK;
pin_mode(rx, PullUp);
}

if (uart == STDIO_UART) {
stdio_uart_inited = 1;
memcpy(&stdio_uart, obj, sizeof(serial_t));
}
}

void serial_free(serial_t *obj) {
Expand Down
Loading