From 2e9a1c0f6ed931d20966a4bf5520798dccf81dd1 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sun, 6 Jan 2019 23:30:34 -0800 Subject: [PATCH 1/5] First Compile --- cores/esp8266/uart.c | 432 ++++++++++++++++++++++++------------------- cores/esp8266/uart.h | 64 ++++--- 2 files changed, 278 insertions(+), 218 deletions(-) diff --git a/cores/esp8266/uart.c b/cores/esp8266/uart.c index e16b309df0..71deb60851 100644 --- a/cores/esp8266/uart.c +++ b/cores/esp8266/uart.c @@ -21,25 +21,25 @@ */ -/** - * UART GPIOs - * - * UART0 TX: 1 or 2 - * UART0 RX: 3 - * - * UART0 SWAP TX: 15 - * UART0 SWAP RX: 13 - * - * - * UART1 TX: 7 (NC) or 2 - * UART1 RX: 8 (NC) - * - * UART1 SWAP TX: 11 (NC) - * UART1 SWAP RX: 6 (NC) - * - * NC = Not Connected to Module Pads --> No Access - * - */ + /** + * UART GPIOs + * + * UART0 TX: 1 or 2 + * UART0 RX: 3 + * + * UART0 SWAP TX: 15 + * UART0 SWAP RX: 13 + * + * + * UART1 TX: 7 (NC) or 2 + * UART1 RX: 8 (NC) + * + * UART1 SWAP TX: 11 (NC) + * UART1 SWAP RX: 6 (NC) + * + * NC = Not Connected to Module Pads --> No Access + * + */ #include "Arduino.h" #include #include "uart.h" @@ -47,7 +47,7 @@ #include "user_interface.h" #include "uart_register.h" -//const char overrun_str [] PROGMEM STORE_ATTR = "uart input full!\r\n"; + //const char overrun_str [] PROGMEM STORE_ATTR = "uart input full!\r\n"; static int s_uart_debug_nr = UART0; @@ -59,7 +59,7 @@ struct uart_rx_buffer_ uint8_t * buffer; }; -struct uart_ +struct uart_ { int uart_nr; int baud_rate; @@ -72,6 +72,13 @@ struct uart_ struct uart_rx_buffer_ * rx_buffer; }; +struct uartIntContext_t +{ + uartInterruptHandler callback; + void* arg; +}; + +static volatile struct uartIntContext_t s_uartInterruptContext[2]; /* In the context of the naming conventions in this file, "_unsafe" means two things: @@ -88,7 +95,7 @@ struct uart_ // called by ISR inline size_t ICACHE_RAM_ATTR -uart_rx_fifo_available(const int uart_nr) +uart_rx_fifo_available(const int uart_nr) { return (USS(uart_nr) >> USRXC) & 0xFF; } @@ -97,12 +104,12 @@ uart_rx_fifo_available(const int uart_nr) /**********************************************************/ /************ UNSAFE FUNCTIONS ****************************/ /**********************************************************/ -inline size_t -uart_rx_buffer_available_unsafe(const struct uart_rx_buffer_ * rx_buffer) +inline size_t +uart_rx_buffer_available_unsafe(const struct uart_rx_buffer_ * rx_buffer) { - if(rx_buffer->wpos < rx_buffer->rpos) - return (rx_buffer->wpos + rx_buffer->size) - rx_buffer->rpos; - + if (rx_buffer->wpos < rx_buffer->rpos) + return (rx_buffer->wpos + rx_buffer->size) - rx_buffer->rpos; + return rx_buffer->wpos - rx_buffer->rpos; } @@ -117,14 +124,14 @@ uart_rx_available_unsafe(uart_t* uart) // Copy all the rx fifo bytes that fit into the rx buffer // called by ISR inline void ICACHE_RAM_ATTR -uart_rx_copy_fifo_to_buffer_unsafe(uart_t* uart) +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)) + while (uart_rx_fifo_available(uart->uart_nr)) { size_t nextPos = (rx_buffer->wpos + 1) % rx_buffer->size; - if(nextPos == rx_buffer->rpos) + if (nextPos == rx_buffer->rpos) { if (!uart->rx_overrun) { @@ -151,17 +158,17 @@ uart_rx_copy_fifo_to_buffer_unsafe(uart_t* uart) } } -inline int +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->rx_buffer) == 0) // hw fifo can't be peeked, data need to be copied to sw uart_rx_copy_fifo_to_buffer_unsafe(uart); - + return uart->rx_buffer->buffer[uart->rx_buffer->rpos]; } @@ -180,12 +187,12 @@ uart_read_char_unsafe(uart_t* uart) return -1; } -size_t +size_t uart_rx_available(uart_t* uart) { - if(uart == NULL || !uart->rx_enabled) + if (uart == NULL || !uart->rx_enabled) return 0; - + ETS_UART_INTR_DISABLE(); int uartrxbufferavailable = uart_rx_buffer_available_unsafe(uart->rx_buffer); ETS_UART_INTR_ENABLE(); @@ -193,30 +200,30 @@ uart_rx_available(uart_t* uart) return uartrxbufferavailable + uart_rx_fifo_available(uart->uart_nr); } -int +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 int ret = uart_peek_char_unsafe(uart); ETS_UART_INTR_ENABLE(); return ret; } -int +int uart_read_char(uart_t* uart) { uint8_t ret; - return uart_read(uart, (char*)&ret, 1)? ret: -1; + return uart_read(uart, (char*)&ret, 1) ? ret : -1; } // loopback-test BW jumps by 190% size_t uart_read(uart_t* uart, char* userbuffer, size_t usersize) { - if(uart == NULL || !uart->rx_enabled) + if (uart == NULL || !uart->rx_enabled) return 0; size_t ret = 0; @@ -230,15 +237,15 @@ uart_read(uart_t* uart, char* userbuffer, size_t usersize) while (ret < usersize && uart_rx_fifo_available(uart->uart_nr)) userbuffer[ret++] = USF(uart->uart_nr); - // no more sw/hw data available + // no more sw/hw data available break; } // pour sw buffer to user's buffer // get largest linear length from sw buffer - size_t chunk = uart->rx_buffer->rpos < uart->rx_buffer->wpos? - uart->rx_buffer->wpos - uart->rx_buffer->rpos: - uart->rx_buffer->size - uart->rx_buffer->rpos; + size_t chunk = uart->rx_buffer->rpos < uart->rx_buffer->wpos ? + uart->rx_buffer->wpos - uart->rx_buffer->rpos : + uart->rx_buffer->size - uart->rx_buffer->rpos; if (ret + chunk > usersize) chunk = usersize - ret; memcpy(userbuffer + ret, uart->rx_buffer->buffer + uart->rx_buffer->rpos, chunk); @@ -250,26 +257,26 @@ uart_read(uart_t* uart, char* userbuffer, size_t usersize) return ret; } -size_t +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) + if (uart->rx_buffer->size == new_size) return uart->rx_buffer->size; uint8_t * new_buf = (uint8_t*)malloc(new_size); - if(!new_buf) + if (!new_buf) return uart->rx_buffer->size; - + size_t new_wpos = 0; ETS_UART_INTR_DISABLE(); - while(uart_rx_available_unsafe(uart) && new_wpos < new_size) + 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 if (new_wpos == new_size) new_wpos = 0; - + uint8_t * old_buf = uart->rx_buffer->buffer; uart->rx_buffer->rpos = 0; uart->rx_buffer->wpos = new_wpos; @@ -283,42 +290,105 @@ uart_resize_rx_buffer(uart_t* uart, size_t new_size) size_t uart_get_rx_buffer_size(uart_t* uart) { - return uart && uart->rx_enabled? uart->rx_buffer->size: 0; + return uart && uart->rx_enabled ? uart->rx_buffer->size : 0; } -void ICACHE_RAM_ATTR -uart_isr(void * arg) +void ICACHE_RAM_ATTR +uart_isr(void* arg) { - uart_t* uart = (uart_t*)arg; - uint32_t usis = USIS(uart->uart_nr); - - if(uart == NULL || !uart->rx_enabled) + // check to confirm we are being called + if (arg == s_uartInterruptContext) { - USIC(uart->uart_nr) = usis; - ETS_UART_INTR_DISABLE(); - return; + // Interrupt handler is shared between UART0 & UART1 + // so we need to check both + for (uint8_t uartNum = 0; uartNum < 2; uartNum++) + { + if (USIS(uartNum)) + { + if (s_uartInterruptContext[uartNum].callback != NULL) + { + s_uartInterruptContext[uartNum].callback(s_uartInterruptContext[uartNum].arg); + } + else + { + // Clear all interrupts flags (just in case) + USIE(uartNum) = 0; + USIC(uartNum) = 0xffff; + } + } + } } +} + +void ICACHE_RAM_ATTR +uart_isrDefault(void * arg) +{ + uart_t* uart = (uart_t*)arg; + uint32_t usis = USIS(uart->uart_nr); - if(usis & (1 << UIFF)) + if (usis & (1 << UIFF)) uart_rx_copy_fifo_to_buffer_unsafe(uart); - if((usis & (1 << UIOF)) && !uart->rx_overrun) + if ((usis & (1 << UIOF)) && !uart->rx_overrun) { uart->rx_overrun = true; //os_printf_plus(overrun_str); } - + if (usis & ((1 << UIFR) | (1 << UIPE) | (1 << UITO))) uart->rx_error = true; USIC(uart->uart_nr) = usis; } -static void +void +uart_subscribeInterrupt(int uart_nr, uartInterruptHandler callback, void* param) +{ + s_uartInterruptContext[uart_nr].callback = callback; + s_uartInterruptContext[uart_nr].arg = param; + + int other_nr = (uart_nr + 1) % 2; + if (s_uartInterruptContext[other_nr].callback == NULL) + { + ETS_UART_INTR_ATTACH(uart_isr, s_uartInterruptContext); + } +} + +void +uart_unsubscribeInterrupt(int uart_nr, uartInterruptHandler callback) +{ + ETS_UART_INTR_DISABLE(); + + if (s_uartInterruptContext[uart_nr].callback == callback) + { + // turn off uart + USC1(uart_nr) = 0; + USIC(uart_nr) = 0xffff; + USIE(uart_nr) = 0; + + s_uartInterruptContext[uart_nr].callback = NULL; + s_uartInterruptContext[uart_nr].arg = NULL; + + int other_nr = (uart_nr + 1) % 2; + if (s_uartInterruptContext[other_nr].callback == NULL) + { + // detach our ISR + ETS_UART_INTR_ATTACH(NULL, NULL); + + // return so we don't enable interrupts since there is no ISR anymore + return; + } + } + + ETS_UART_INTR_ENABLE(); +} + + +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 @@ -329,9 +399,9 @@ uart_start_isr(uart_t* uart) // - 4..120 give > 2300Kibits/s // - 1, 2, 3 are below // was 100, use 16 to stay away from overrun - #define INTRIGG 16 +#define INTRIGG 16 - //was:USC1(uart->uart_nr) = (INTRIGG << UCFFT) | (0x02 << UCTOT) | (1 <uart_nr) = (INTRIGG << UCFFT) | (0x02 << UCTOT) | (1 <uart_nr) = (INTRIGG << UCFFT); USIC(uart->uart_nr) = 0xffff; //was: USIE(uart->uart_nr) = (1 << UIFF) | (1 << UIFR) | (1 << UITO); @@ -341,25 +411,10 @@ uart_start_isr(uart_t* uart) // UIPE: parity error // UITO: rx fifo timeout USIE(uart->uart_nr) = (1 << UIFF) | (1 << UIOF) | (1 << UIFR) | (1 << UIPE) | (1 << UITO); - ETS_UART_INTR_ATTACH(uart_isr, (void *)uart); - ETS_UART_INTR_ENABLE(); -} - -static void -uart_stop_isr(uart_t* uart) -{ - if(uart == NULL || !uart->rx_enabled) - return; + uart_subscribeInterrupt(uart->uart_nr, uart_isrDefault, (void *)uart); - ETS_UART_INTR_DISABLE(); - USC1(uart->uart_nr) = 0; - USIC(uart->uart_nr) = 0xffff; - USIE(uart->uart_nr) = 0; - ETS_UART_INTR_ATTACH(NULL, NULL); } - - /* Reference for uart_tx_fifo_available() and uart_tx_fifo_full(): -Espressif Techinical Reference doc, chapter 11.3.7 @@ -379,28 +434,28 @@ uart_tx_fifo_full(const int uart_nr) } -static void +static void uart_do_write_char(const int uart_nr, char c) { - while(uart_tx_fifo_full(uart_nr)); + while (uart_tx_fifo_full(uart_nr)); USF(uart_nr) = c; } -size_t +size_t uart_write_char(uart_t* uart, char c) { - if(uart == NULL || !uart->tx_enabled) + if (uart == NULL || !uart->tx_enabled) return 0; uart_do_write_char(uart->uart_nr, c); return 1; } -size_t +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; @@ -413,34 +468,34 @@ uart_write(uart_t* uart, const char* buf, size_t size) -size_t +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 - uart_tx_fifo_available(uart->uart_nr); } -void +void uart_wait_tx_empty(uart_t* uart) { - if(uart == NULL || !uart->tx_enabled) + if (uart == NULL || !uart->tx_enabled) return; - while(uart_tx_fifo_available(uart->uart_nr) > 0) + while (uart_tx_fifo_available(uart->uart_nr) > 0) delay(0); } -void +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(); @@ -449,86 +504,86 @@ 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 +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 +int uart_get_baudrate(uart_t* uart) { - if(uart == NULL) + if (uart == NULL) return 0; return uart->baud_rate; } -uart_t* +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) + uart_t* uart = (uart_t*)malloc(sizeof(uart_t)); + if (uart == NULL) return NULL; uart->uart_nr = uart_nr; uart->rx_overrun = false; uart->rx_error = false; - switch(uart->uart_nr) + ETS_UART_INTR_DISABLE(); + + 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) + uart->rx_pin = (uart->rx_enabled) ? 3 : 255; + 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; + free(uart); + return NULL; } rx_buffer->size = rx_size;//var this 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; + free(rx_buffer); + free(uart); + return NULL; } uart->rx_buffer = rx_buffer; pinMode(uart->rx_pin, SPECIAL); } - if(uart->tx_enabled) + if (uart->tx_enabled) { - if (tx_pin == 2) + 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; } @@ -540,10 +595,9 @@ uart_init(int uart_nr, int baudrate, int config, int mode, int tx_pin, size_t rx 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) + uart->tx_pin = (uart->tx_enabled) ? 2 : 255; // GPIO7 as TX not possible! See GPIO pins used by UART + if (uart->tx_enabled) pinMode(uart->tx_pin, SPECIAL); - break; case UART_NO: @@ -559,21 +613,21 @@ uart_init(int uart_nr, int baudrate, int config, int mode, int tx_pin, size_t rx 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); + ETS_UART_INTR_ENABLE(); + return uart; } -void +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); @@ -583,7 +637,7 @@ uart_uninit(uart_t* uart) break; } - switch(uart->tx_pin) + switch (uart->tx_pin) { case 1: pinMode(1, INPUT); @@ -596,60 +650,60 @@ uart_uninit(uart_t* uart) break; } - if(uart->rx_enabled) + if (uart->rx_enabled) { - uart_stop_isr(uart); + uart_unsubscribeInterrupt(uart->uart_nr, uart_isrDefault); free(uart->rx_buffer->buffer); free(uart->rx_buffer); } free(uart); } -void +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_pin == 1 || uart->tx_pin == 2) && uart->tx_enabled) || (uart->rx_pin == 3 && uart->rx_enabled)) { - if(uart->tx_enabled) //TX + 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 + } + else { - if(uart->tx_enabled) //TX + if (uart->tx_enabled) //TX { pinMode(uart->tx_pin, INPUT); - uart->tx_pin = (tx_pin == 2)?2:1; + 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) - pinMode(uart->tx_pin, (tx_pin == 2)?FUNCTION_4:SPECIAL); //TX + 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); @@ -664,24 +718,24 @@ uart_swap(uart_t* uart, int tx_pin) } } -void +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_enabled) { - if (uart->tx_pin == 1 && tx_pin == 2) + 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; @@ -698,21 +752,21 @@ uart_set_tx(uart_t* uart, int tx_pin) } } -void +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->uart_nr == UART0) // Only UART0 allows pin changes { - if(uart->tx_enabled && uart->tx_pin != tx) + if (uart->tx_enabled && uart->tx_pin != tx) { - if( rx == 13 && tx == 15) + if (rx == 13 && tx == 15) { uart_swap(uart, 15); - } - else if (rx == 3 && (tx == 1 || tx == 2)) + } + else if (rx == 3 && (tx == 1 || tx == 2)) { if (uart->rx_pin != rx) uart_swap(uart, tx); @@ -720,32 +774,32 @@ uart_set_pins(uart_t* uart, int tx, int rx) 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 +bool uart_tx_enabled(uart_t* uart) { - if(uart == NULL) + if (uart == NULL) return false; return uart->tx_enabled; } -bool +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->rx_overrun) return false; @@ -756,7 +810,7 @@ uart_has_overrun (uart_t* uart) } bool -uart_has_rx_error (uart_t* uart) +uart_has_rx_error(uart_t* uart) { if (uart == NULL || !uart->rx_error) return false; @@ -766,57 +820,57 @@ uart_has_rx_error (uart_t* uart) return true; } -static void +static void uart_ignore_char(char c) { - (void) c; + (void)c; } inline void uart_write_char_delay(const int uart_nr, char c) { - while(uart_tx_fifo_full(uart_nr)) + while (uart_tx_fifo_full(uart_nr)) delay(0); USF(uart_nr) = c; } -static void +static void uart0_write_char(char c) { uart_write_char_delay(0, c); } -static void +static void uart1_write_char(char c) { uart_write_char_delay(1, c); } -void +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); + ets_install_putc1((void *)&uart0_write_char); break; case UART1: system_set_os_print(1); - ets_install_putc1((void *) &uart1_write_char); + ets_install_putc1((void *)&uart1_write_char); break; case UART_NO: default: system_set_os_print(0); - ets_install_putc1((void *) &uart_ignore_char); + ets_install_putc1((void *)&uart_ignore_char); break; } } -int +int uart_get_debug() { return s_uart_debug_nr; @@ -851,7 +905,7 @@ uart_detect_baudrate(int uart_nr) doTrigger = true; // Initialize for a next round int32_t baudrate = UART_CLK_FREQ / divisor; - static const int default_rates[] = {300, 600, 1200, 2400, 4800, 9600, 19200, 38400, 57600, 74880, 115200, 230400, 256000, 460800, 921600, 1843200, 3686400}; + static const int default_rates[] = { 300, 600, 1200, 2400, 4800, 9600, 19200, 38400, 57600, 74880, 115200, 230400, 256000, 460800, 921600, 1843200, 3686400 }; size_t i; for (i = 1; i < sizeof(default_rates) / sizeof(default_rates[0]) - 1; i++) // find the nearest real baudrate diff --git a/cores/esp8266/uart.h b/cores/esp8266/uart.h index 7f9dce0f0a..a8733050dc 100644 --- a/cores/esp8266/uart.h +++ b/cores/esp8266/uart.h @@ -33,7 +33,7 @@ extern "C" { #define UART1 1 #define UART_NO -1 -// Options for `config` argument of uart_init + // Options for `config` argument of uart_init #define UART_NB_BIT_MASK 0B00001100 #define UART_NB_BIT_5 0B00000000 #define UART_NB_BIT_6 0B00000100 @@ -110,43 +110,49 @@ extern "C" { #define UART_TX_FIFO_SIZE 0x80 -struct uart_; -typedef struct uart_ uart_t; + typedef void(*uartInterruptHandler)(void* arg); // ICACHE_RAM_ATTR required -uart_t* uart_init(int uart_nr, int baudrate, int config, int mode, int tx_pin, size_t rx_size); -void uart_uninit(uart_t* uart); + struct uart_; + typedef struct uart_ uart_t; -void uart_swap(uart_t* uart, int tx_pin); -void uart_set_tx(uart_t* uart, int tx_pin); -void uart_set_pins(uart_t* uart, int tx, int rx); -bool uart_tx_enabled(uart_t* uart); -bool uart_rx_enabled(uart_t* uart); + uart_t* uart_init(int uart_nr, int baudrate, int config, int mode, int tx_pin, size_t rx_size); + void uart_uninit(uart_t* uart); -void uart_set_baudrate(uart_t* uart, int baud_rate); -int uart_get_baudrate(uart_t* uart); + void uart_swap(uart_t* uart, int tx_pin); + void uart_set_tx(uart_t* uart, int tx_pin); + void uart_set_pins(uart_t* uart, int tx, int rx); + bool uart_tx_enabled(uart_t* uart); + bool uart_rx_enabled(uart_t* uart); -size_t uart_resize_rx_buffer(uart_t* uart, size_t new_size); -size_t uart_get_rx_buffer_size(uart_t* uart); + void uart_set_baudrate(uart_t* uart, int baud_rate); + int uart_get_baudrate(uart_t* uart); -size_t uart_write_char(uart_t* uart, char c); -size_t uart_write(uart_t* uart, const char* buf, size_t size); -int uart_read_char(uart_t* uart); -int uart_peek_char(uart_t* uart); -size_t uart_read(uart_t* uart, char* buffer, size_t size); -size_t uart_rx_available(uart_t* uart); -size_t uart_tx_free(uart_t* uart); -void uart_wait_tx_empty(uart_t* uart); -void uart_flush(uart_t* uart); + size_t uart_resize_rx_buffer(uart_t* uart, size_t new_size); + size_t uart_get_rx_buffer_size(uart_t* uart); -bool uart_has_overrun (uart_t* uart); // returns then clear overrun flag -bool uart_has_rx_error (uart_t* uart); // returns then clear rxerror flag + size_t uart_write_char(uart_t* uart, char c); + size_t uart_write(uart_t* uart, const char* buf, size_t size); + int uart_read_char(uart_t* uart); + int uart_peek_char(uart_t* uart); + size_t uart_read(uart_t* uart, char* buffer, size_t size); + size_t uart_rx_available(uart_t* uart); + size_t uart_tx_free(uart_t* uart); + void uart_wait_tx_empty(uart_t* uart); + void uart_flush(uart_t* uart); -void uart_set_debug(int uart_nr); -int uart_get_debug(); + bool uart_has_overrun(uart_t* uart); // returns then clear overrun flag + bool uart_has_rx_error(uart_t* uart); // returns then clear rxerror flag -void uart_start_detect_baudrate(int uart_nr); -int uart_detect_baudrate(int uart_nr); + void uart_set_debug(int uart_nr); + int uart_get_debug(); + void uart_start_detect_baudrate(int uart_nr); + int uart_detect_baudrate(int uart_nr); + + // attach MUST be called within a + // ETS_UART_INTR_DISABLE()/ETS_UART_INTR_ENABLE() sandwich + void uart_subscribeInterrupt(int uart_nr, uartInterruptHandler callback, void* param); + void uart_unsubscribeInterrupt(int uart_nr, uartInterruptHandler callback); #if defined (__cplusplus) } // extern "C" From 5bc2ea5e82d53c96d2c2e81381fdbb272f651dff Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Mon, 7 Jan 2019 15:27:56 -0800 Subject: [PATCH 2/5] subscribe and unsubscribe consistent Since subscribe can not be automatically disable and enable interrupts due to other external uart setup must be done at the same time within the disabled interrupts, then unsubscribe should be made consistent --- cores/esp8266/uart.c | 30 ++++++++++++++++-------------- cores/esp8266/uart.h | 9 ++++++--- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/cores/esp8266/uart.c b/cores/esp8266/uart.c index 71deb60851..24fedf418a 100644 --- a/cores/esp8266/uart.c +++ b/cores/esp8266/uart.c @@ -72,13 +72,15 @@ struct uart_ struct uart_rx_buffer_ * rx_buffer; }; -struct uartIntContext_t +struct uartIsrContext_t { uartInterruptHandler callback; void* arg; }; -static volatile struct uartIntContext_t s_uartInterruptContext[2]; +// NOTE: GCC will generated an invalid warning for the following line +// see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119 +static volatile struct uartIsrContext_t s_uartInterruptContext[2] = { 0 }; /* In the context of the naming conventions in this file, "_unsafe" means two things: @@ -91,8 +93,6 @@ static volatile struct uartIntContext_t s_uartInterruptContext[2]; wrap the unsafe ones with disabling/enabling of the uart interrupt for safe public use. */ - - // called by ISR inline size_t ICACHE_RAM_ATTR uart_rx_fifo_available(const int uart_nr) @@ -100,7 +100,6 @@ uart_rx_fifo_available(const int uart_nr) return (USS(uart_nr) >> USRXC) & 0xFF; } - /**********************************************************/ /************ UNSAFE FUNCTIONS ****************************/ /**********************************************************/ @@ -312,7 +311,8 @@ uart_isr(void* arg) } else { - // Clear all interrupts flags (just in case) + // No registered handler, so + // clear all interrupts flags (just in case) USIE(uartNum) = 0; USIC(uartNum) = 0xffff; } @@ -355,11 +355,9 @@ uart_subscribeInterrupt(int uart_nr, uartInterruptHandler callback, void* param) } } -void +bool uart_unsubscribeInterrupt(int uart_nr, uartInterruptHandler callback) { - ETS_UART_INTR_DISABLE(); - if (s_uartInterruptContext[uart_nr].callback == callback) { // turn off uart @@ -376,12 +374,13 @@ uart_unsubscribeInterrupt(int uart_nr, uartInterruptHandler callback) // detach our ISR ETS_UART_INTR_ATTACH(NULL, NULL); - // return so we don't enable interrupts since there is no ISR anymore - return; + // return false so we don't enable interrupts + // since there is no need for the ISR anymore + return false; } } - ETS_UART_INTR_ENABLE(); + return true; } @@ -485,7 +484,6 @@ uart_wait_tx_empty(uart_t* uart) while (uart_tx_fifo_available(uart->uart_nr) > 0) delay(0); - } void @@ -652,7 +650,11 @@ uart_uninit(uart_t* uart) if (uart->rx_enabled) { - uart_unsubscribeInterrupt(uart->uart_nr, uart_isrDefault); + ETS_UART_INTR_DISABLE(); + if (uart_unsubscribeInterrupt(uart->uart_nr, uart_isrDefault)) + { + ETS_UART_INTR_ENABLE(); + } free(uart->rx_buffer->buffer); free(uart->rx_buffer); } diff --git a/cores/esp8266/uart.h b/cores/esp8266/uart.h index a8733050dc..283a47dd08 100644 --- a/cores/esp8266/uart.h +++ b/cores/esp8266/uart.h @@ -149,10 +149,13 @@ extern "C" { void uart_start_detect_baudrate(int uart_nr); int uart_detect_baudrate(int uart_nr); - // attach MUST be called within a - // ETS_UART_INTR_DISABLE()/ETS_UART_INTR_ENABLE() sandwich + // uart_subscribeInterrupt & uart_unsubscribeInterrupt are not safe and must + // be called within ETS_UART_INTR_DISABLE()/ETS_UART_INTR_ENABLE() protection + // void uart_subscribeInterrupt(int uart_nr, uartInterruptHandler callback, void* param); - void uart_unsubscribeInterrupt(int uart_nr, uartInterruptHandler callback); + // if uart_unsubscribeInterrupt returns false, then ETS_UART_INTR_ENABLE() doesn't + // need to be called after it + bool uart_unsubscribeInterrupt(int uart_nr, uartInterruptHandler callback); #if defined (__cplusplus) } // extern "C" From ae7903dfb4484770f59415a6eb6cfa726bd648cd Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Mon, 7 Jan 2019 22:39:33 -0800 Subject: [PATCH 3/5] Apply review feeedback --- cores/esp8266/uart.c | 15 +++++++++++---- cores/esp8266/uart.h | 4 ++-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/cores/esp8266/uart.c b/cores/esp8266/uart.c index 24fedf418a..b6ac2bb85e 100644 --- a/cores/esp8266/uart.c +++ b/cores/esp8266/uart.c @@ -80,7 +80,10 @@ struct uartIsrContext_t // NOTE: GCC will generated an invalid warning for the following line // see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wmissing-braces" static volatile struct uartIsrContext_t s_uartInterruptContext[2] = { 0 }; +#pragma GCC diagnostic pop /* In the context of the naming conventions in this file, "_unsafe" means two things: @@ -343,11 +346,13 @@ uart_isrDefault(void * arg) } void -uart_subscribeInterrupt(int uart_nr, uartInterruptHandler callback, void* param) +uart_subscribeInterrupt_unsafe(int uart_nr, uartInterruptHandler callback, void* param) { s_uartInterruptContext[uart_nr].callback = callback; s_uartInterruptContext[uart_nr].arg = param; + // check if we are already attached to the interrupt for + // the other uart and only attach if not int other_nr = (uart_nr + 1) % 2; if (s_uartInterruptContext[other_nr].callback == NULL) { @@ -356,7 +361,7 @@ uart_subscribeInterrupt(int uart_nr, uartInterruptHandler callback, void* param) } bool -uart_unsubscribeInterrupt(int uart_nr, uartInterruptHandler callback) +uart_unsubscribeInterrupt_unsafe(int uart_nr, uartInterruptHandler callback) { if (s_uartInterruptContext[uart_nr].callback == callback) { @@ -368,6 +373,8 @@ uart_unsubscribeInterrupt(int uart_nr, uartInterruptHandler callback) s_uartInterruptContext[uart_nr].callback = NULL; s_uartInterruptContext[uart_nr].arg = NULL; + // check if we are also attached to the interrupt for + // the other uart and only deattach if not int other_nr = (uart_nr + 1) % 2; if (s_uartInterruptContext[other_nr].callback == NULL) { @@ -410,7 +417,7 @@ uart_start_isr(uart_t* uart) // UIPE: parity error // UITO: rx fifo timeout USIE(uart->uart_nr) = (1 << UIFF) | (1 << UIOF) | (1 << UIFR) | (1 << UIPE) | (1 << UITO); - uart_subscribeInterrupt(uart->uart_nr, uart_isrDefault, (void *)uart); + uart_subscribeInterrupt_unsafe(uart->uart_nr, uart_isrDefault, (void *)uart); } @@ -651,7 +658,7 @@ uart_uninit(uart_t* uart) if (uart->rx_enabled) { ETS_UART_INTR_DISABLE(); - if (uart_unsubscribeInterrupt(uart->uart_nr, uart_isrDefault)) + if (uart_unsubscribeInterrupt_unsafe(uart->uart_nr, uart_isrDefault)) { ETS_UART_INTR_ENABLE(); } diff --git a/cores/esp8266/uart.h b/cores/esp8266/uart.h index 283a47dd08..0773cc3cbf 100644 --- a/cores/esp8266/uart.h +++ b/cores/esp8266/uart.h @@ -152,10 +152,10 @@ extern "C" { // uart_subscribeInterrupt & uart_unsubscribeInterrupt are not safe and must // be called within ETS_UART_INTR_DISABLE()/ETS_UART_INTR_ENABLE() protection // - void uart_subscribeInterrupt(int uart_nr, uartInterruptHandler callback, void* param); + void uart_subscribeInterrupt_unsafe(int uart_nr, uartInterruptHandler callback, void* param); // if uart_unsubscribeInterrupt returns false, then ETS_UART_INTR_ENABLE() doesn't // need to be called after it - bool uart_unsubscribeInterrupt(int uart_nr, uartInterruptHandler callback); + bool uart_unsubscribeInterrupt_unsafe(int uart_nr, uartInterruptHandler callback); #if defined (__cplusplus) } // extern "C" From 6564edb8031646eda8a32341b172f3ce1a59f3f2 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Fri, 11 Jan 2019 13:23:19 -0800 Subject: [PATCH 4/5] static initialize fixed --- cores/esp8266/uart.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/cores/esp8266/uart.c b/cores/esp8266/uart.c index b6ac2bb85e..db64be9f93 100644 --- a/cores/esp8266/uart.c +++ b/cores/esp8266/uart.c @@ -78,12 +78,7 @@ struct uartIsrContext_t void* arg; }; -// NOTE: GCC will generated an invalid warning for the following line -// see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119 -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wmissing-braces" -static volatile struct uartIsrContext_t s_uartInterruptContext[2] = { 0 }; -#pragma GCC diagnostic pop +static struct uartIsrContext_t s_uartInterruptContext[2]; /* In the context of the naming conventions in this file, "_unsafe" means two things: From c96209db911043d6a323721941d432b968bd05a5 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sun, 20 Jan 2019 12:23:14 -0800 Subject: [PATCH 5/5] better function name --- cores/esp8266/uart.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cores/esp8266/uart.c b/cores/esp8266/uart.c index db64be9f93..baf6032d2f 100644 --- a/cores/esp8266/uart.c +++ b/cores/esp8266/uart.c @@ -387,7 +387,7 @@ uart_unsubscribeInterrupt_unsafe(int uart_nr, uartInterruptHandler callback) static void -uart_start_isr(uart_t* uart) +uart_init_default_isr(uart_t* uart) { if (uart == NULL || !uart->rx_enabled) return; @@ -413,7 +413,6 @@ uart_start_isr(uart_t* uart) // UITO: rx fifo timeout USIE(uart->uart_nr) = (1 << UIFF) | (1 << UIOF) | (1 << UIFR) | (1 << UIPE) | (1 << UITO); uart_subscribeInterrupt_unsafe(uart->uart_nr, uart_isrDefault, (void *)uart); - } /* @@ -614,7 +613,7 @@ uart_init(int uart_nr, int baudrate, int config, int mode, int tx_pin, size_t rx USIC(uart->uart_nr) = 0xffff; USIE(uart->uart_nr) = 0; if (uart->uart_nr == UART0 && uart->rx_enabled) - uart_start_isr(uart); + uart_init_default_isr(uart); ETS_UART_INTR_ENABLE();