From 9247bd08c8268f6e79dbf846685f3d6128342916 Mon Sep 17 00:00:00 2001 From: devyte Date: Mon, 26 Mar 2018 12:53:22 -0300 Subject: [PATCH 1/7] Fix for #4565 (rx fifo length), protect access to rx_buffer --- cores/esp8266/uart.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/cores/esp8266/uart.c b/cores/esp8266/uart.c index c5acc9977d..15672ce2ad 100644 --- a/cores/esp8266/uart.c +++ b/cores/esp8266/uart.c @@ -100,7 +100,7 @@ inline size_t uart_rx_buffer_available(uart_t* uart) { } inline size_t uart_rx_fifo_available(uart_t* uart) { - return (USS(uart->uart_nr) >> USRXC) & 0x7F; + return (USS(uart->uart_nr) >> USRXC) & 0xFF; } const char overrun_str [] ICACHE_RODATA_ATTR STORE_ATTR = "uart input full!\r\n"; @@ -140,15 +140,17 @@ int uart_peek_char(uart_t* uart) if(uart == NULL || !uart->rx_enabled) { return -1; } + ETS_UART_INTR_DISABLE(); //access to rx_buffer can be interrupted by the isr (similar to a critical section), so disable interrupts here if (!uart_rx_available(uart)) { + ETS_UART_INTR_ENABLE(); return -1; } if (uart_rx_buffer_available(uart) == 0) { - ETS_UART_INTR_DISABLE(); uart_rx_copy_fifo_to_buffer(uart); - ETS_UART_INTR_ENABLE(); } - return uart->rx_buffer->buffer[uart->rx_buffer->rpos]; + int ret = uart->rx_buffer->buffer[uart->rx_buffer->rpos]; + ETS_UART_INTR_ENABLE(); + return ret; } int uart_read_char(uart_t* uart) @@ -158,7 +160,9 @@ int uart_read_char(uart_t* uart) } int data = uart_peek_char(uart); if(data != -1) { + ETS_UART_INTR_DISABLE(); uart->rx_buffer->rpos = (uart->rx_buffer->rpos + 1) % uart->rx_buffer->size; + ETS_UART_INTR_ENABLE(); } return data; } @@ -168,7 +172,11 @@ size_t uart_rx_available(uart_t* uart) if(uart == NULL || !uart->rx_enabled) { return 0; } - return uart_rx_buffer_available(uart) + uart_rx_fifo_available(uart); + ETS_UART_INTR_DISABLE(); + int uartrxbufferavail = uart_rx_buffer_available(uart); + ETS_UART_INTR_ENABLE(); + + return uartrxbufferavailable + uart_rx_fifo_available(uart); } @@ -186,7 +194,7 @@ void ICACHE_RAM_ATTR uart_isr(void * arg) USIC(uart->uart_nr) = USIS(uart->uart_nr); } -void uart_start_isr(uart_t* uart) +static void uart_start_isr(uart_t* uart) { if(uart == NULL || !uart->rx_enabled) { return; @@ -202,7 +210,7 @@ void uart_start_isr(uart_t* uart) ETS_UART_INTR_ENABLE(); } -void uart_stop_isr(uart_t* uart) +static void uart_stop_isr(uart_t* uart) { if(uart == NULL || !uart->rx_enabled) { return; @@ -383,6 +391,8 @@ void uart_uninit(uart_t* uart) return; } + uart_stop_isr(uart); + switch(uart->rx_pin) { case 3: pinMode(3, INPUT); From 982663df53ee9eece527d7999ebe7ac9168c8f9b Mon Sep 17 00:00:00 2001 From: devyte Date: Mon, 26 Mar 2018 14:09:11 -0300 Subject: [PATCH 2/7] Fix typo --- cores/esp8266/uart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp8266/uart.c b/cores/esp8266/uart.c index 15672ce2ad..5fb50f2e11 100644 --- a/cores/esp8266/uart.c +++ b/cores/esp8266/uart.c @@ -173,7 +173,7 @@ size_t uart_rx_available(uart_t* uart) return 0; } ETS_UART_INTR_DISABLE(); - int uartrxbufferavail = uart_rx_buffer_available(uart); + int uartrxbufferavailable = uart_rx_buffer_available(uart); ETS_UART_INTR_ENABLE(); return uartrxbufferavailable + uart_rx_fifo_available(uart); From 9f2af3fbce3cf3dd88f70e3b29fe4707301e79d4 Mon Sep 17 00:00:00 2001 From: devyte Date: Tue, 27 Mar 2018 03:21:18 -0300 Subject: [PATCH 3/7] reworked to separate safe from unsafe functions, factorized some code, constness --- cores/esp8266/uart.c | 519 +++++++++++++++++++++++++++---------------- 1 file changed, 330 insertions(+), 189 deletions(-) diff --git a/cores/esp8266/uart.c b/cores/esp8266/uart.c index 5fb50f2e11..ee857fd116 100644 --- a/cores/esp8266/uart.c +++ b/cores/esp8266/uart.c @@ -45,16 +45,20 @@ #include "esp8266_peri.h" #include "user_interface.h" +static const char overrun_str [] ICACHE_RODATA_ATTR STORE_ATTR = "uart input full!\r\n"; static int s_uart_debug_nr = UART0; -struct uart_rx_buffer_ { + +struct uart_rx_buffer_ +{ size_t size; size_t rpos; size_t wpos; uint8_t * buffer; }; -struct uart_ { +struct uart_ +{ int uart_nr; int baud_rate; bool rx_enabled; @@ -65,60 +69,89 @@ struct uart_ { struct uart_rx_buffer_ * rx_buffer; }; -size_t uart_resize_rx_buffer(uart_t* uart, size_t new_size) + +/* + In the context of the naming conventions in this file, "_unsafe" means two things: + 1. the input arguments are not checked. It is up to the caller to check argument sanity. + 2. The function body is not interrupt-safe, i.e.: the isr could fire anywhen during the + body execution, leading to corruption of the data shared between the body and the isr + (parts of the rx_buffer). + + The unsafe versions of the functions are private to this TU. There are "safe" versions that + wrap the unsafe ones with disabling/enabling of the uart interrupt for safe public use. +*/ + + + +inline size_t +uart_rx_fifo_available(uart_t* uart) { - if(uart == NULL || !uart->rx_enabled) { - return 0; - } - if(uart->rx_buffer->size == new_size) { - return uart->rx_buffer->size; - } - uint8_t * new_buf = (uint8_t*)malloc(new_size); - if(!new_buf) { - return uart->rx_buffer->size; - } - size_t new_wpos = 0; - ETS_UART_INTR_DISABLE(); - while(uart_rx_available(uart) && new_wpos < new_size) { - new_buf[new_wpos++] = uart_read_char(uart); - } - uint8_t * old_buf = uart->rx_buffer->buffer; - uart->rx_buffer->rpos = 0; - uart->rx_buffer->wpos = new_wpos; - uart->rx_buffer->size = new_size; - uart->rx_buffer->buffer = new_buf; - free(old_buf); - ETS_UART_INTR_ENABLE(); - return uart->rx_buffer->size; + return (USS(uart->uart_nr) >> USRXC) & 0xFF; } -inline size_t uart_rx_buffer_available(uart_t* uart) { - if(uart->rx_buffer->wpos < uart->rx_buffer->rpos) { + +/**********************************************************/ +/************ UNSAFE FUNCTIONS ****************************/ +/**********************************************************/ +inline size_t +uart_rx_buffer_available_unsafe(uart_t* uart) +{ + if(uart->rx_buffer->wpos < uart->rx_buffer->rpos) return (uart->rx_buffer->wpos + uart->rx_buffer->size) - uart->rx_buffer->rpos; - } + return uart->rx_buffer->wpos - uart->rx_buffer->rpos; } -inline size_t uart_rx_fifo_available(uart_t* uart) { - return (USS(uart->uart_nr) >> USRXC) & 0xFF; +inline size_t +uart_rx_available_unsafe(uart_t* uart) +{ + return uart_rx_buffer_available_unsafe(uart) + uart_rx_fifo_available(uart); } -const char overrun_str [] ICACHE_RODATA_ATTR STORE_ATTR = "uart input full!\r\n"; +inline int +uart_peek_char_unsafe(uart_t* uart) +{ + if (!uart_rx_available_unsafe(uart)) + return -1; + + //without the following if statement and body, there is a good chance of a fifo overrun + if (uart_rx_buffer_available_unsafe(uart) == 0) + uart_rx_copy_fifo_to_buffer_unsafe(uart); + + return uart->rx_buffer->buffer[uart->rx_buffer->rpos]; +} + +inline int +uart_read_char_unsafe(uart_t* uart) +{ + int data = uart_peek_char_unsafe(uart); + if(data != -1) + uart->rx_buffer->rpos = (uart->rx_buffer->rpos + 1) % uart->rx_buffer->size; + return data; +} + + +//#define UART_DISCARD_NEWEST // Copy all the rx fifo bytes that fit into the rx buffer -inline void uart_rx_copy_fifo_to_buffer(uart_t* uart) { - while(uart_rx_fifo_available(uart)){ +inline void +uart_rx_copy_fifo_to_buffer_unsafe(uart_t* uart) +{ + while(uart_rx_fifo_available(uart)) + { size_t nextPos = (uart->rx_buffer->wpos + 1) % uart->rx_buffer->size; - if(nextPos == uart->rx_buffer->rpos) { + if(nextPos == uart->rx_buffer->rpos) + { - if (!uart->overrun) { + if (!uart->overrun) + { uart->overrun = true; os_printf_plus(overrun_str); } // a choice has to be made here, // do we discard newest or oldest data? -#if 0 +#ifdef UART_DISCARD_NEWEST // discard newest data // Stop copying if rx buffer is full USF(uart->uart_nr); @@ -135,70 +168,100 @@ inline void uart_rx_copy_fifo_to_buffer(uart_t* uart) { } } -int uart_peek_char(uart_t* uart) + +/**********************************************************/ + + + +size_t +uart_rx_available(uart_t* uart) +{ + if(uart == NULL || !uart->rx_enabled) + return 0; + + ETS_UART_INTR_DISABLE(); + int uartrxbufferavailable = uart_rx_buffer_available_unsafe(uart); + ETS_UART_INTR_ENABLE(); + + return uartrxbufferavailable + uart_rx_fifo_available(uart); +} + +int +uart_peek_char(uart_t* uart) { - if(uart == NULL || !uart->rx_enabled) { + if(uart == NULL || !uart->rx_enabled) return -1; - } + ETS_UART_INTR_DISABLE(); //access to rx_buffer can be interrupted by the isr (similar to a critical section), so disable interrupts here - if (!uart_rx_available(uart)) { - ETS_UART_INTR_ENABLE(); - return -1; - } - if (uart_rx_buffer_available(uart) == 0) { - uart_rx_copy_fifo_to_buffer(uart); - } - int ret = uart->rx_buffer->buffer[uart->rx_buffer->rpos]; + int ret = uart_peek_char_unsafe(uart); ETS_UART_INTR_ENABLE(); return ret; } -int uart_read_char(uart_t* uart) +int +uart_read_char(uart_t* uart) { - if(uart == NULL) { + if(uart == NULL || !uart->rx_enabled) return -1; - } - int data = uart_peek_char(uart); - if(data != -1) { - ETS_UART_INTR_DISABLE(); - uart->rx_buffer->rpos = (uart->rx_buffer->rpos + 1) % uart->rx_buffer->size; - ETS_UART_INTR_ENABLE(); - } + + ETS_UART_INTR_DISABLE(); + int data = uart_read_char_unsafe(uart); + ETS_UART_INTR_ENABLE(); return data; } -size_t uart_rx_available(uart_t* uart) +size_t +uart_resize_rx_buffer(uart_t* uart, size_t new_size) { - if(uart == NULL || !uart->rx_enabled) { + if(uart == NULL || !uart->rx_enabled) return 0; - } + + if(uart->rx_buffer->size == new_size) + return uart->rx_buffer->size; + + uint8_t * new_buf = (uint8_t*)malloc(new_size); + if(!new_buf) + return uart->rx_buffer->size; + + size_t new_wpos = 0; ETS_UART_INTR_DISABLE(); - int uartrxbufferavailable = uart_rx_buffer_available(uart); + while(uart_rx_available_unsafe(uart) && new_wpos < new_size) + new_buf[new_wpos++] = uart_read_char_unsafe(uart); //if uart_rx_available_unsafe() returns non-0, uart_read_char_unsafe() can't return -1 + + uint8_t * old_buf = uart->rx_buffer->buffer; + uart->rx_buffer->rpos = 0; + uart->rx_buffer->wpos = new_wpos; + uart->rx_buffer->size = new_size; + uart->rx_buffer->buffer = new_buf; ETS_UART_INTR_ENABLE(); - - return uartrxbufferavailable + uart_rx_fifo_available(uart); + free(old_buf); + return uart->rx_buffer->size; } -void ICACHE_RAM_ATTR uart_isr(void * arg) + +void ICACHE_RAM_ATTR +uart_isr(void * arg) { uart_t* uart = (uart_t*)arg; - if(uart == NULL || !uart->rx_enabled) { + if(uart == NULL || !uart->rx_enabled) + { USIC(uart->uart_nr) = USIS(uart->uart_nr); ETS_UART_INTR_DISABLE(); return; } - if(USIS(uart->uart_nr) & ((1 << UIFF) | (1 << UITO))){ - uart_rx_copy_fifo_to_buffer(uart); - } + if(USIS(uart->uart_nr) & ((1 << UIFF) | (1 << UITO))) + uart_rx_copy_fifo_to_buffer_unsafe(uart); + USIC(uart->uart_nr) = USIS(uart->uart_nr); } -static void uart_start_isr(uart_t* uart) +static void +uart_start_isr(uart_t* uart) { - if(uart == NULL || !uart->rx_enabled) { + if(uart == NULL || !uart->rx_enabled) return; - } + // UCFFT value is when the RX fifo full interrupt triggers. A value of 1 // triggers the IRS very often. A value of 127 would not leave much time // for ISR to clear fifo before the next byte is dropped. So pick a value @@ -210,11 +273,12 @@ static void uart_start_isr(uart_t* uart) ETS_UART_INTR_ENABLE(); } -static void uart_stop_isr(uart_t* uart) +static void +uart_stop_isr(uart_t* uart) { - if(uart == NULL || !uart->rx_enabled) { + if(uart == NULL || !uart->rx_enabled) return; - } + ETS_UART_INTR_DISABLE(); USC1(uart->uart_nr) = 0; USIC(uart->uart_nr) = 0xffff; @@ -222,59 +286,90 @@ static void uart_stop_isr(uart_t* uart) ETS_UART_INTR_ATTACH(NULL, NULL); } -static void uart_do_write_char(uart_t* uart, char c) + + +/* + Reference for uart_tx_fifo_available() and uart_tx_fifo_full(): + -Espressif Techinical Reference doc, chapter 11.3.7 + -tools/sdk/uart_register.h + -cores/esp8266/esp8266_peri.h + */ +inline size_t +uart_tx_fifo_available(const int uart_nr) +{ + return (USS(uart_nr) >> USTXC) & 0xff; +} + +inline bool +uart_tx_fifo_full(const int uart_nr) { - while((USS(uart->uart_nr) >> USTXC) >= 0x7f); - USF(uart->uart_nr) = c; + return uart_tx_fifo_available(uart_nr) >= 0x7f; } -size_t uart_write_char(uart_t* uart, char c) + +static void +uart_do_write_char(const int uart_nr, char c) { - if(uart == NULL || !uart->tx_enabled) { + while(uart_tx_fifo_full(uart_nr)); + + USF(uart_nr) = c; +} + +size_t +uart_write_char(uart_t* uart, char c) +{ + if(uart == NULL || !uart->tx_enabled) return 0; - } - uart_do_write_char(uart, c); + + uart_do_write_char(uart->uart_nr, c); return 1; } -size_t uart_write(uart_t* uart, const char* buf, size_t size) +size_t +uart_write(uart_t* uart, const char* buf, size_t size) { - if(uart == NULL || !uart->tx_enabled) { + if(uart == NULL || !uart->tx_enabled) return 0; - } + size_t ret = size; - while (size--) { - uart_do_write_char(uart, *buf++); - } + const int uart_nr = uart->uart_nr; + while (size--) + uart_do_write_char(uart_nr, *buf++); + return ret; } -size_t uart_tx_free(uart_t* uart) + + +size_t +uart_tx_free(uart_t* uart) { - if(uart == NULL || !uart->tx_enabled) { + if(uart == NULL || !uart->tx_enabled) return 0; - } - return UART_TX_FIFO_SIZE - ((USS(uart->uart_nr) >> USTXC) & 0xff); + + return UART_TX_FIFO_SIZE - uart_tx_fifo_available(uart->uart_nr); } -void uart_wait_tx_empty(uart_t* uart) +void +uart_wait_tx_empty(uart_t* uart) { - if(uart == NULL || !uart->tx_enabled) { + if(uart == NULL || !uart->tx_enabled) return; - } - while(((USS(uart->uart_nr) >> USTXC) & 0xff) > 0) { + + while(uart_tx_fifo_available(uart->uart_nr) > 0) delay(0); - } + } -void uart_flush(uart_t* uart) +void +uart_flush(uart_t* uart) { - if(uart == NULL) { + if(uart == NULL) return; - } uint32_t tmp = 0x00000000; - if(uart->rx_enabled) { + if(uart->rx_enabled) + { tmp |= (1 << UCRXRST); ETS_UART_INTR_DISABLE(); uart->rx_buffer->rpos = 0; @@ -282,51 +377,55 @@ void uart_flush(uart_t* uart) ETS_UART_INTR_ENABLE(); } - if(uart->tx_enabled) { + if(uart->tx_enabled) tmp |= (1 << UCTXRST); - } USC0(uart->uart_nr) |= (tmp); USC0(uart->uart_nr) &= ~(tmp); } -void uart_set_baudrate(uart_t* uart, int baud_rate) +void +uart_set_baudrate(uart_t* uart, int baud_rate) { - if(uart == NULL) { + if(uart == NULL) return; - } + uart->baud_rate = baud_rate; USD(uart->uart_nr) = (ESP8266_CLOCK / uart->baud_rate); } -int uart_get_baudrate(uart_t* uart) +int +uart_get_baudrate(uart_t* uart) { - if(uart == NULL) { + if(uart == NULL) return 0; - } + return uart->baud_rate; } -uart_t* uart_init(int uart_nr, int baudrate, int config, int mode, int tx_pin, size_t rx_size) +uart_t* +uart_init(int uart_nr, int baudrate, int config, int mode, int tx_pin, size_t rx_size) { uart_t* uart = (uart_t*) malloc(sizeof(uart_t)); - if(uart == NULL) { + if(uart == NULL) return NULL; - } uart->uart_nr = uart_nr; uart->overrun = false; - switch(uart->uart_nr) { + switch(uart->uart_nr) + { case UART0: ETS_UART_INTR_DISABLE(); ETS_UART_INTR_ATTACH(NULL, NULL); uart->rx_enabled = (mode != UART_TX_ONLY); uart->tx_enabled = (mode != UART_RX_ONLY); uart->rx_pin = (uart->rx_enabled)?3:255; - if(uart->rx_enabled) { + if(uart->rx_enabled) + { struct uart_rx_buffer_ * rx_buffer = (struct uart_rx_buffer_ *)malloc(sizeof(struct uart_rx_buffer_)); - if(rx_buffer == NULL) { + if(rx_buffer == NULL) + { free(uart); return NULL; } @@ -334,7 +433,8 @@ uart_t* uart_init(int uart_nr, int baudrate, int config, int mode, int tx_pin, s rx_buffer->rpos = 0; rx_buffer->wpos = 0; rx_buffer->buffer = (uint8_t *)malloc(rx_buffer->size); - if(rx_buffer->buffer == NULL) { + if(rx_buffer->buffer == NULL) + { free(rx_buffer); free(uart); return NULL; @@ -342,29 +442,37 @@ uart_t* uart_init(int uart_nr, int baudrate, int config, int mode, int tx_pin, s uart->rx_buffer = rx_buffer; pinMode(uart->rx_pin, SPECIAL); } - if(uart->tx_enabled) { - if (tx_pin == 2) { + if(uart->tx_enabled) + { + if (tx_pin == 2) + { uart->tx_pin = 2; pinMode(uart->tx_pin, FUNCTION_4); - } else { + } + else + { uart->tx_pin = 1; pinMode(uart->tx_pin, FUNCTION_0); } - } else { + } + else + { uart->tx_pin = 255; } IOSWAP &= ~(1 << IOSWAPU0); break; + case UART1: // Note: uart_interrupt_handler does not support RX on UART 1. uart->rx_enabled = false; uart->tx_enabled = (mode != UART_RX_ONLY); uart->rx_pin = 255; uart->tx_pin = (uart->tx_enabled)?2:255; // GPIO7 as TX not possible! See GPIO pins used by UART - if(uart->tx_enabled) { + if(uart->tx_enabled) pinMode(uart->tx_pin, SPECIAL); - } + break; + case UART_NO: default: // big fail! @@ -378,22 +486,22 @@ uart_t* uart_init(int uart_nr, int baudrate, int config, int mode, int tx_pin, s USC1(uart->uart_nr) = 0; USIC(uart->uart_nr) = 0xffff; USIE(uart->uart_nr) = 0; - if(uart->uart_nr == UART0 && uart->rx_enabled) { + if(uart->uart_nr == UART0 && uart->rx_enabled) uart_start_isr(uart); - } return uart; } -void uart_uninit(uart_t* uart) +void +uart_uninit(uart_t* uart) { - if(uart == NULL) { + if(uart == NULL) return; - } uart_stop_isr(uart); - switch(uart->rx_pin) { + switch(uart->rx_pin) + { case 3: pinMode(3, INPUT); break; @@ -402,7 +510,8 @@ void uart_uninit(uart_t* uart) break; } - switch(uart->tx_pin) { + switch(uart->tx_pin) + { case 1: pinMode(1, INPUT); break; @@ -414,52 +523,62 @@ void uart_uninit(uart_t* uart) break; } - if(uart->rx_enabled){ + if(uart->rx_enabled) + { + uart_stop_isr(uart); free(uart->rx_buffer->buffer); free(uart->rx_buffer); - uart_stop_isr(uart); } free(uart); } -void uart_swap(uart_t* uart, int tx_pin) +void +uart_swap(uart_t* uart, int tx_pin) { - if(uart == NULL) { + if(uart == NULL) return; - } - switch(uart->uart_nr) { + + switch(uart->uart_nr) + { case UART0: - if(((uart->tx_pin == 1 || uart->tx_pin == 2) && uart->tx_enabled) || (uart->rx_pin == 3 && uart->rx_enabled)) { - if(uart->tx_enabled) { //TX + if(((uart->tx_pin == 1 || uart->tx_pin == 2) && uart->tx_enabled) || (uart->rx_pin == 3 && uart->rx_enabled)) + { + if(uart->tx_enabled) //TX + { pinMode(uart->tx_pin, INPUT); uart->tx_pin = 15; } - if(uart->rx_enabled) { //RX + if(uart->rx_enabled) //RX + { pinMode(uart->rx_pin, INPUT); uart->rx_pin = 13; } - if(uart->tx_enabled) { + if(uart->tx_enabled) pinMode(uart->tx_pin, FUNCTION_4); //TX - } - if(uart->rx_enabled) { + + if(uart->rx_enabled) pinMode(uart->rx_pin, FUNCTION_4); //RX - } + IOSWAP |= (1 << IOSWAPU0); - } else { - if(uart->tx_enabled) { //TX + } + else + { + if(uart->tx_enabled) //TX + { pinMode(uart->tx_pin, INPUT); uart->tx_pin = (tx_pin == 2)?2:1; } - if(uart->rx_enabled) { //RX + if(uart->rx_enabled) //RX + { pinMode(uart->rx_pin, INPUT); uart->rx_pin = 3; } - if(uart->tx_enabled) { + if(uart->tx_enabled) pinMode(uart->tx_pin, (tx_pin == 2)?FUNCTION_4:SPECIAL); //TX - } - if(uart->rx_enabled) { + + if(uart->rx_enabled) pinMode(3, SPECIAL); //RX - } + IOSWAP &= ~(1 << IOSWAPU0); } @@ -472,19 +591,25 @@ void uart_swap(uart_t* uart, int tx_pin) } } -void uart_set_tx(uart_t* uart, int tx_pin) +void +uart_set_tx(uart_t* uart, int tx_pin) { - if(uart == NULL) { + if(uart == NULL) return; - } - switch(uart->uart_nr) { + + switch(uart->uart_nr) + { case UART0: - if(uart->tx_enabled) { - if (uart->tx_pin == 1 && tx_pin == 2) { + if(uart->tx_enabled) + { + if (uart->tx_pin == 1 && tx_pin == 2) + { pinMode(uart->tx_pin, INPUT); uart->tx_pin = 2; pinMode(uart->tx_pin, FUNCTION_4); - } else if (uart->tx_pin == 2 && tx_pin != 2) { + } + else if (uart->tx_pin == 2 && tx_pin != 2) + { pinMode(uart->tx_pin, INPUT); uart->tx_pin = 1; pinMode(uart->tx_pin, SPECIAL); @@ -500,82 +625,97 @@ void uart_set_tx(uart_t* uart, int tx_pin) } } -void uart_set_pins(uart_t* uart, int tx, int rx) +void +uart_set_pins(uart_t* uart, int tx, int rx) { - if(uart == NULL) { + if(uart == NULL) return; - } - if(uart->uart_nr == UART0) { // Only UART0 allows pin changes - if(uart->tx_enabled && uart->tx_pin != tx) { - if( rx == 13 && tx == 15) { + if(uart->uart_nr == UART0) // Only UART0 allows pin changes + { + if(uart->tx_enabled && uart->tx_pin != tx) + { + if( rx == 13 && tx == 15) + { uart_swap(uart, 15); - } else if (rx == 3 && (tx == 1 || tx == 2)) { - if (uart->rx_pin != rx) { + } + else if (rx == 3 && (tx == 1 || tx == 2)) + { + if (uart->rx_pin != rx) uart_swap(uart, tx); - } else { + else uart_set_tx(uart, tx); - } } } - if(uart->rx_enabled && uart->rx_pin != rx && rx == 13 && tx == 15) { + if(uart->rx_enabled && uart->rx_pin != rx && rx == 13 && tx == 15) uart_swap(uart, 15); - } } } -bool uart_tx_enabled(uart_t* uart) +bool +uart_tx_enabled(uart_t* uart) { - if(uart == NULL) { + if(uart == NULL) return false; - } + return uart->tx_enabled; } -bool uart_rx_enabled(uart_t* uart) +bool +uart_rx_enabled(uart_t* uart) { - if(uart == NULL) { + if(uart == NULL) return false; - } + return uart->rx_enabled; } -bool uart_has_overrun (uart_t* uart) +bool +uart_has_overrun (uart_t* uart) { - if (uart == NULL || !uart->overrun) { + if (uart == NULL || !uart->overrun) return false; - } + // clear flag uart->overrun = false; return true; } -static void uart_ignore_char(char c) +static void +uart_ignore_char(char c) { (void) c; } -static void uart0_write_char(char c) +inline void +uart_write_char_delay(const int uart_nr, char c) { - while(((USS(0) >> USTXC) & 0xff) >= 0x7F) { + while(uart_tx_fifo_full(uart_nr)) delay(0); - } - USF(0) = c; + + USF(uart_nr) = c; + } -static void uart1_write_char(char c) +static void +uart0_write_char(char c) { - while(((USS(1) >> USTXC) & 0xff) >= 0x7F) { - delay(0); - } - USF(1) = c; + uart_write_char_delay(0, c); +} + +static void +uart1_write_char(char c) +{ + uart_write_char_delay(1, c); } -void uart_set_debug(int uart_nr) +void +uart_set_debug(int uart_nr) { s_uart_debug_nr = uart_nr; - switch(s_uart_debug_nr) { + switch(s_uart_debug_nr) + { case UART0: system_set_os_print(1); ets_install_putc1((void *) &uart0_write_char); @@ -592,7 +732,8 @@ void uart_set_debug(int uart_nr) } } -int uart_get_debug() +int +uart_get_debug() { return s_uart_debug_nr; } From ec358a91bd34a0a0be9d404634bc446aff7f12a1 Mon Sep 17 00:00:00 2001 From: devyte Date: Tue, 27 Mar 2018 03:25:39 -0300 Subject: [PATCH 4/7] additional rework for uart_rx_fifo_available() --- cores/esp8266/uart.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cores/esp8266/uart.c b/cores/esp8266/uart.c index ee857fd116..1a94edf0a7 100644 --- a/cores/esp8266/uart.c +++ b/cores/esp8266/uart.c @@ -84,9 +84,9 @@ struct uart_ inline size_t -uart_rx_fifo_available(uart_t* uart) +uart_rx_fifo_available(const int uart_nr) { - return (USS(uart->uart_nr) >> USRXC) & 0xFF; + return (USS(uart_nr) >> USRXC) & 0xFF; } @@ -105,7 +105,7 @@ uart_rx_buffer_available_unsafe(uart_t* uart) inline size_t uart_rx_available_unsafe(uart_t* uart) { - return uart_rx_buffer_available_unsafe(uart) + uart_rx_fifo_available(uart); + return uart_rx_buffer_available_unsafe(uart) + uart_rx_fifo_available(uart->uart_nr); } inline int @@ -137,7 +137,7 @@ uart_read_char_unsafe(uart_t* uart) inline void uart_rx_copy_fifo_to_buffer_unsafe(uart_t* uart) { - while(uart_rx_fifo_available(uart)) + while(uart_rx_fifo_available(uart->uart_nr)) { size_t nextPos = (uart->rx_buffer->wpos + 1) % uart->rx_buffer->size; if(nextPos == uart->rx_buffer->rpos) @@ -183,7 +183,7 @@ uart_rx_available(uart_t* uart) int uartrxbufferavailable = uart_rx_buffer_available_unsafe(uart); ETS_UART_INTR_ENABLE(); - return uartrxbufferavailable + uart_rx_fifo_available(uart); + return uartrxbufferavailable + uart_rx_fifo_available(uart->uart_nr); } int From 0b6ea38acb7fb342fd84817c6201ef14f6cc8375 Mon Sep 17 00:00:00 2001 From: devyte Date: Tue, 27 Mar 2018 13:01:53 -0300 Subject: [PATCH 5/7] swapped unsafe function definition order --- cores/esp8266/uart.c | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/cores/esp8266/uart.c b/cores/esp8266/uart.c index 1a94edf0a7..6015ae06a0 100644 --- a/cores/esp8266/uart.c +++ b/cores/esp8266/uart.c @@ -108,28 +108,6 @@ uart_rx_available_unsafe(uart_t* uart) return uart_rx_buffer_available_unsafe(uart) + uart_rx_fifo_available(uart->uart_nr); } -inline int -uart_peek_char_unsafe(uart_t* uart) -{ - if (!uart_rx_available_unsafe(uart)) - return -1; - - //without the following if statement and body, there is a good chance of a fifo overrun - if (uart_rx_buffer_available_unsafe(uart) == 0) - uart_rx_copy_fifo_to_buffer_unsafe(uart); - - return uart->rx_buffer->buffer[uart->rx_buffer->rpos]; -} - -inline int -uart_read_char_unsafe(uart_t* uart) -{ - int data = uart_peek_char_unsafe(uart); - if(data != -1) - uart->rx_buffer->rpos = (uart->rx_buffer->rpos + 1) % uart->rx_buffer->size; - return data; -} - //#define UART_DISCARD_NEWEST @@ -168,6 +146,28 @@ uart_rx_copy_fifo_to_buffer_unsafe(uart_t* uart) } } +inline int +uart_peek_char_unsafe(uart_t* uart) +{ + if (!uart_rx_available_unsafe(uart)) + return -1; + + //without the following if statement and body, there is a good chance of a fifo overrun + if (uart_rx_buffer_available_unsafe(uart) == 0) + uart_rx_copy_fifo_to_buffer_unsafe(uart); + + return uart->rx_buffer->buffer[uart->rx_buffer->rpos]; +} + +inline int +uart_read_char_unsafe(uart_t* uart) +{ + int data = uart_peek_char_unsafe(uart); + if(data != -1) + uart->rx_buffer->rpos = (uart->rx_buffer->rpos + 1) % uart->rx_buffer->size; + return data; +} + /**********************************************************/ From 225e570f48b50be42a5ff5f3901f5e0e94560a99 Mon Sep 17 00:00:00 2001 From: devyte Date: Tue, 27 Mar 2018 14:22:50 -0300 Subject: [PATCH 6/7] Remove static for overrun string --- cores/esp8266/uart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp8266/uart.c b/cores/esp8266/uart.c index 6015ae06a0..7c73c69f3c 100644 --- a/cores/esp8266/uart.c +++ b/cores/esp8266/uart.c @@ -45,7 +45,7 @@ #include "esp8266_peri.h" #include "user_interface.h" -static const char overrun_str [] ICACHE_RODATA_ATTR STORE_ATTR = "uart input full!\r\n"; +const char overrun_str [] ICACHE_RODATA_ATTR STORE_ATTR = "uart input full!\r\n"; static int s_uart_debug_nr = UART0; From c610910bec979af5c08170948c2ef680d193055e Mon Sep 17 00:00:00 2001 From: devyte Date: Tue, 27 Mar 2018 21:50:57 -0300 Subject: [PATCH 7/7] Some shorthand for perf and readability --- cores/esp8266/uart.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/cores/esp8266/uart.c b/cores/esp8266/uart.c index 7c73c69f3c..47814f7a9d 100644 --- a/cores/esp8266/uart.c +++ b/cores/esp8266/uart.c @@ -94,18 +94,18 @@ uart_rx_fifo_available(const int uart_nr) /************ UNSAFE FUNCTIONS ****************************/ /**********************************************************/ inline size_t -uart_rx_buffer_available_unsafe(uart_t* uart) +uart_rx_buffer_available_unsafe(const struct uart_rx_buffer_ * rx_buffer) { - if(uart->rx_buffer->wpos < uart->rx_buffer->rpos) - return (uart->rx_buffer->wpos + uart->rx_buffer->size) - uart->rx_buffer->rpos; + if(rx_buffer->wpos < rx_buffer->rpos) + return (rx_buffer->wpos + rx_buffer->size) - rx_buffer->rpos; - return uart->rx_buffer->wpos - uart->rx_buffer->rpos; + return rx_buffer->wpos - rx_buffer->rpos; } inline size_t uart_rx_available_unsafe(uart_t* uart) { - return uart_rx_buffer_available_unsafe(uart) + uart_rx_fifo_available(uart->uart_nr); + return uart_rx_buffer_available_unsafe(uart->rx_buffer) + uart_rx_fifo_available(uart->uart_nr); } @@ -115,10 +115,12 @@ uart_rx_available_unsafe(uart_t* uart) inline void uart_rx_copy_fifo_to_buffer_unsafe(uart_t* uart) { + struct uart_rx_buffer_ *rx_buffer = uart->rx_buffer; + while(uart_rx_fifo_available(uart->uart_nr)) { - size_t nextPos = (uart->rx_buffer->wpos + 1) % uart->rx_buffer->size; - if(nextPos == uart->rx_buffer->rpos) + size_t nextPos = (rx_buffer->wpos + 1) % rx_buffer->size; + if(nextPos == rx_buffer->rpos) { if (!uart->overrun) @@ -136,13 +138,13 @@ uart_rx_copy_fifo_to_buffer_unsafe(uart_t* uart) break; #else // discard oldest data - if (++uart->rx_buffer->rpos == uart->rx_buffer->size) - uart->rx_buffer->rpos = 0; + if (++rx_buffer->rpos == rx_buffer->size) + rx_buffer->rpos = 0; #endif } uint8_t data = USF(uart->uart_nr); - uart->rx_buffer->buffer[uart->rx_buffer->wpos] = data; - uart->rx_buffer->wpos = nextPos; + rx_buffer->buffer[rx_buffer->wpos] = data; + rx_buffer->wpos = nextPos; } } @@ -153,7 +155,7 @@ uart_peek_char_unsafe(uart_t* uart) return -1; //without the following if statement and body, there is a good chance of a fifo overrun - if (uart_rx_buffer_available_unsafe(uart) == 0) + if (uart_rx_buffer_available_unsafe(uart->rx_buffer) == 0) uart_rx_copy_fifo_to_buffer_unsafe(uart); return uart->rx_buffer->buffer[uart->rx_buffer->rpos]; @@ -180,7 +182,7 @@ uart_rx_available(uart_t* uart) return 0; ETS_UART_INTR_DISABLE(); - int uartrxbufferavailable = uart_rx_buffer_available_unsafe(uart); + int uartrxbufferavailable = uart_rx_buffer_available_unsafe(uart->rx_buffer); ETS_UART_INTR_ENABLE(); return uartrxbufferavailable + uart_rx_fifo_available(uart->uart_nr);