From 7ffcd398eef5002792ebdfbeafb251fba297c95d Mon Sep 17 00:00:00 2001 From: david gauchard Date: Tue, 20 Mar 2018 01:47:27 +0100 Subject: [PATCH 1/7] unefficient Print:write(data,len) shows message if used (only in debug mode) +fix HardwareSerial that used it --- cores/esp8266/HardwareSerial.cpp | 12 ++++++------ cores/esp8266/HardwareSerial.h | 2 +- cores/esp8266/Print.cpp | 12 +++++++++++- cores/esp8266/uart.c | 23 +++++++++++++++-------- cores/esp8266/uart.h | 4 ++-- 5 files changed, 35 insertions(+), 18 deletions(-) diff --git a/cores/esp8266/HardwareSerial.cpp b/cores/esp8266/HardwareSerial.cpp index e5b0d79a8c..6475a01a9f 100644 --- a/cores/esp8266/HardwareSerial.cpp +++ b/cores/esp8266/HardwareSerial.cpp @@ -167,14 +167,14 @@ void HardwareSerial::flush() size_t HardwareSerial::write(uint8_t c) { - if(!_uart || !uart_tx_enabled(_uart)) { - return 0; - } - - uart_write_char(_uart, c); - return 1; + return uart_write_char(_uart, c); } +size_t HardwareSerial::write(const uint8_t *buffer, size_t size) +{ + return uart_write(_uart, (const char*)buffer, size); +} + int HardwareSerial::baudRate(void) { // Null pointer on _uart is checked by SDK diff --git a/cores/esp8266/HardwareSerial.h b/cores/esp8266/HardwareSerial.h index 76c442ec74..52f20d3883 100644 --- a/cores/esp8266/HardwareSerial.h +++ b/cores/esp8266/HardwareSerial.h @@ -129,7 +129,7 @@ class HardwareSerial: public Stream { return write((uint8_t) n); } - using Print::write; // pull in write(str) and write(buf, size) from Print + size_t write(const uint8_t *buffer, size_t size); operator bool() const; void setDebugOutput(bool); diff --git a/cores/esp8266/Print.cpp b/cores/esp8266/Print.cpp index 786c594065..14ffcf7218 100644 --- a/cores/esp8266/Print.cpp +++ b/cores/esp8266/Print.cpp @@ -33,10 +33,20 @@ /* default implementation: may be overridden */ size_t Print::write(const uint8_t *buffer, size_t size) { + +#ifdef DEBUG_ESP_CORE + static char not_the_best_way [] ICACHE_RODATA_ATTR STORE_ATTR = "Print::write(data,len) should be overridden for better efficiency\r\n"; + static bool once = false; + if (!once) { + once = true; + os_printf_plus(not_the_best_way); + } +#endif + size_t n = 0; while (size--) { size_t ret = write(*buffer++); - if (ret == 0) { + if (ret <= 0) { // Write of last byte didn't complete, abort additional processing break; } diff --git a/cores/esp8266/uart.c b/cores/esp8266/uart.c index cad8e77a79..765d931c0a 100644 --- a/cores/esp8266/uart.c +++ b/cores/esp8266/uart.c @@ -215,23 +215,30 @@ void uart_stop_isr(uart_t* uart) } -void uart_write_char(uart_t* uart, char c) +void uart_do_write_char(uart_t* uart, char c) { - if(uart == NULL || !uart->tx_enabled) { - return; - } while((USS(uart->uart_nr) >> USTXC) >= 0x7f); USF(uart->uart_nr) = c; } -void uart_write(uart_t* uart, const char* buf, size_t size) +size_t uart_write_char(uart_t* uart, char c) { if(uart == NULL || !uart->tx_enabled) { - return; + return 0; + } + uart_do_write_char(uart, c); + return 1; +} + +size_t uart_write(uart_t* uart, const char* buf, size_t size) +{ + if(uart == NULL || !uart->tx_enabled) { + return 0; } - while(size--) { - uart_write_char(uart, *buf++); + for (size_t s = size + 1; --s; ) { + uart_do_write_char(uart, *buf++); } + return size; } size_t uart_tx_free(uart_t* uart) diff --git a/cores/esp8266/uart.h b/cores/esp8266/uart.h index c3aa0426d0..127c5d0ebb 100644 --- a/cores/esp8266/uart.h +++ b/cores/esp8266/uart.h @@ -127,8 +127,8 @@ int uart_get_baudrate(uart_t* uart); size_t uart_resize_rx_buffer(uart_t* uart, size_t new_size); -void uart_write_char(uart_t* uart, char c); -void uart_write(uart_t* uart, const char* buf, size_t size); +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_rx_available(uart_t* uart); From 96e2fa282aef3bcad51bb408293ee61e51bdb9e0 Mon Sep 17 00:00:00 2001 From: David Gauchard Date: Tue, 20 Mar 2018 09:48:12 +0100 Subject: [PATCH 2/7] misleadedly assumed ::write() could return -1 like arduino's ::read does --- cores/esp8266/Print.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp8266/Print.cpp b/cores/esp8266/Print.cpp index 14ffcf7218..c276d8e982 100644 --- a/cores/esp8266/Print.cpp +++ b/cores/esp8266/Print.cpp @@ -46,7 +46,7 @@ size_t Print::write(const uint8_t *buffer, size_t size) { size_t n = 0; while (size--) { size_t ret = write(*buffer++); - if (ret <= 0) { + if (ret == 0) { // Write of last byte didn't complete, abort additional processing break; } From 4a680a065998007a507545f01fcf5a6c5d1a8a90 Mon Sep 17 00:00:00 2001 From: David Gauchard Date: Tue, 20 Mar 2018 09:59:11 +0100 Subject: [PATCH 3/7] uart: using inline for a not trivial function actually saves 16 flash bytes --- cores/esp8266/uart.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cores/esp8266/uart.c b/cores/esp8266/uart.c index 765d931c0a..7dc853f0fa 100644 --- a/cores/esp8266/uart.c +++ b/cores/esp8266/uart.c @@ -214,8 +214,7 @@ void uart_stop_isr(uart_t* uart) ETS_UART_INTR_ATTACH(NULL, NULL); } - -void uart_do_write_char(uart_t* uart, char c) +inline void uart_do_write_char(uart_t* uart, char c) { while((USS(uart->uart_nr) >> USTXC) >= 0x7f); USF(uart->uart_nr) = c; From c02762d510216c7c03836ef0ea620aa59f9cfc8d Mon Sep 17 00:00:00 2001 From: David Gauchard Date: Tue, 20 Mar 2018 10:09:07 +0100 Subject: [PATCH 4/7] uart: let gcc decide if function should be inlined --- 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 7dc853f0fa..b84cc56e9a 100644 --- a/cores/esp8266/uart.c +++ b/cores/esp8266/uart.c @@ -214,7 +214,7 @@ void uart_stop_isr(uart_t* uart) ETS_UART_INTR_ATTACH(NULL, NULL); } -inline void uart_do_write_char(uart_t* uart, char c) +static void uart_do_write_char(uart_t* uart, char c) { while((USS(uart->uart_nr) >> USTXC) >= 0x7f); USF(uart->uart_nr) = c; From de24e9bdc374b9e7a7b819e77c62a6575f1ebee3 Mon Sep 17 00:00:00 2001 From: David Gauchard Date: Tue, 20 Mar 2018 14:08:06 +0100 Subject: [PATCH 5/7] uart: more readable loop --- cores/esp8266/uart.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cores/esp8266/uart.c b/cores/esp8266/uart.c index b84cc56e9a..278939ee66 100644 --- a/cores/esp8266/uart.c +++ b/cores/esp8266/uart.c @@ -234,10 +234,11 @@ size_t uart_write(uart_t* uart, const char* buf, size_t size) if(uart == NULL || !uart->tx_enabled) { return 0; } - for (size_t s = size + 1; --s; ) { + size_t ret = size; + while (size--) { uart_do_write_char(uart, *buf++); } - return size; + return ret; } size_t uart_tx_free(uart_t* uart) From fc50a68a3bf109215433e7f1196ac71e1b237a82 Mon Sep 17 00:00:00 2001 From: David Gauchard Date: Wed, 21 Mar 2018 17:27:15 +0100 Subject: [PATCH 6/7] big but minor changes: remove lots of duplicate code, move trivial calls to .h to give gcc a chance to optimize them --- cores/esp8266/HardwareSerial.cpp | 85 +------------------------------- cores/esp8266/HardwareSerial.h | 64 +++++++++++++++++++----- 2 files changed, 53 insertions(+), 96 deletions(-) diff --git a/cores/esp8266/HardwareSerial.cpp b/cores/esp8266/HardwareSerial.cpp index 6475a01a9f..4eb2366fa5 100644 --- a/cores/esp8266/HardwareSerial.cpp +++ b/cores/esp8266/HardwareSerial.cpp @@ -55,10 +55,8 @@ void HardwareSerial::end() uart_set_debug(UART_NO); } - if (_uart) { - uart_uninit(_uart); - _uart = NULL; - } + uart_uninit(_uart); + _uart = NULL; } size_t HardwareSerial::setRxBufferSize(size_t size){ @@ -70,30 +68,6 @@ size_t HardwareSerial::setRxBufferSize(size_t size){ return _rx_size; } -void HardwareSerial::swap(uint8_t tx_pin) -{ - if(!_uart) { - return; - } - uart_swap(_uart, tx_pin); -} - -void HardwareSerial::set_tx(uint8_t tx_pin) -{ - if(!_uart) { - return; - } - uart_set_tx(_uart, tx_pin); -} - -void HardwareSerial::pins(uint8_t tx, uint8_t rx) -{ - if(!_uart) { - return; - } - uart_set_pins(_uart, tx, rx); -} - void HardwareSerial::setDebugOutput(bool en) { if(!_uart) { @@ -113,16 +87,6 @@ void HardwareSerial::setDebugOutput(bool en) } } -bool HardwareSerial::isTxEnabled(void) -{ - return _uart && uart_tx_enabled(_uart); -} - -bool HardwareSerial::isRxEnabled(void) -{ - return _uart && uart_rx_enabled(_uart); -} - int HardwareSerial::available(void) { int result = static_cast(uart_rx_available(_uart)); @@ -132,27 +96,6 @@ int HardwareSerial::available(void) return result; } -int HardwareSerial::peek(void) -{ - // this may return -1, but that's okay - return uart_peek_char(_uart); -} - -int HardwareSerial::read(void) -{ - // this may return -1, but that's okay - return uart_read_char(_uart); -} - -int HardwareSerial::availableForWrite(void) -{ - if(!_uart || !uart_tx_enabled(_uart)) { - return 0; - } - - return static_cast(uart_tx_free(_uart)); -} - void HardwareSerial::flush() { if(!_uart || !uart_tx_enabled(_uart)) { @@ -165,33 +108,9 @@ void HardwareSerial::flush() delayMicroseconds(11000000 / uart_get_baudrate(_uart) + 1); } -size_t HardwareSerial::write(uint8_t c) -{ - return uart_write_char(_uart, c); -} - -size_t HardwareSerial::write(const uint8_t *buffer, size_t size) -{ - return uart_write(_uart, (const char*)buffer, size); -} - -int HardwareSerial::baudRate(void) -{ - // Null pointer on _uart is checked by SDK - return uart_get_baudrate(_uart); -} - - -HardwareSerial::operator bool() const -{ - return _uart != 0; -} - - #if !defined(NO_GLOBAL_INSTANCES) && !defined(NO_GLOBAL_SERIAL) HardwareSerial Serial(UART0); #endif #if !defined(NO_GLOBAL_INSTANCES) && !defined(NO_GLOBAL_SERIAL1) HardwareSerial Serial1(UART1); #endif - diff --git a/cores/esp8266/HardwareSerial.h b/cores/esp8266/HardwareSerial.h index 52f20d3883..dfc2c98fcf 100644 --- a/cores/esp8266/HardwareSerial.h +++ b/cores/esp8266/HardwareSerial.h @@ -93,26 +93,50 @@ class HardwareSerial: public Stream { swap(1); } - void swap(uint8_t tx_pin); //toggle between use of GPIO13/GPIO15 or GPIO3/GPIO(1/2) as RX and TX + void swap(uint8_t tx_pin) //toggle between use of GPIO13/GPIO15 or GPIO3/GPIO(1/2) as RX and TX + { + uart_swap(_uart, tx_pin); + } /* * Toggle between use of GPIO1 and GPIO2 as TX on UART 0. * Note: UART 1 can't be used if GPIO2 is used with UART 0! */ - void set_tx(uint8_t tx_pin); + void set_tx(uint8_t tx_pin) + { + uart_set_tx(_uart, tx_pin); + } /* * UART 0 possible options are (1, 3), (2, 3) or (15, 13) * UART 1 allows only TX on 2 if UART 0 is not (2, 3) */ - void pins(uint8_t tx, uint8_t rx); + void pins(uint8_t tx, uint8_t rx) + { + uart_set_pins(_uart, tx, rx); + } int available(void) override; - int peek(void) override; - int read(void) override; - int availableForWrite(void); + + int peek(void) override + { + // this may return -1, but that's okay + return uart_peek_char(_uart); + } + int read(void) override + { + // this may return -1, but that's okay + return uart_read_char(_uart); + } + int availableForWrite(void) + { + return static_cast(uart_tx_free(_uart)); + } void flush(void) override; - size_t write(uint8_t) override; + size_t write(uint8_t c) override + { + return uart_write_char(_uart, c); + } inline size_t write(unsigned long n) { return write((uint8_t) n); @@ -129,13 +153,27 @@ class HardwareSerial: public Stream { return write((uint8_t) n); } - size_t write(const uint8_t *buffer, size_t size); - operator bool() const; - + size_t write(const uint8_t *buffer, size_t size) + { + return uart_write(_uart, (const char*)buffer, size); + } + operator bool() const + { + return _uart != 0; + } void setDebugOutput(bool); - bool isTxEnabled(void); - bool isRxEnabled(void); - int baudRate(void); + bool isTxEnabled(void) + { + return uart_tx_enabled(_uart); + } + bool isRxEnabled(void) + { + return _uart && uart_rx_enabled(_uart); + } + int baudRate(void) + { + return uart_get_baudrate(_uart); + } bool hasOverrun(void) { From b02b98fcebf5a61b0e2f6b6d3d5caa777f6dc2db Mon Sep 17 00:00:00 2001 From: David Gauchard Date: Wed, 21 Mar 2018 17:34:55 +0100 Subject: [PATCH 7/7] fix PROGMEM str declaration (ld section conflics) --- 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 278939ee66..c5acc9977d 100644 --- a/cores/esp8266/uart.c +++ b/cores/esp8266/uart.c @@ -103,7 +103,7 @@ inline size_t uart_rx_fifo_available(uart_t* uart) { return (USS(uart->uart_nr) >> USRXC) & 0x7F; } -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"; // Copy all the rx fifo bytes that fit into the rx buffer inline void uart_rx_copy_fifo_to_buffer(uart_t* uart) {