Inefficient Print::write(data,len) shows message if used (only in deb…

…ug mode) (esp8266#4537) * inefficient Print::write(data,len) shows message if used (only in debug mode) * make HardwareSerial's write(data,len) efficient * HardwareSerial: remove duplicate tests, move trivial code from .cpp to .h (cherry picked from commit f8f205d)
bryceschober · Apr 5, 2018 · ce50882 · ce50882
1 parent 0d5ebb7
commit ce50882
Show file tree

Hide file tree

Showing 5 changed files with 81 additions and 107 deletions.
diff --git 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<int>(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<int>(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)
-{
-    if(!_uart || !uart_tx_enabled(_uart)) {
-        return 0;
-    }
-
-    uart_write_char(_uart, c);
-    return 1;
-}
-
-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
@@ -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<int>(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);
     }
-    using Print::write; // pull in write(str) and write(buf, size) from Print
-    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)
     {

diff --git a/cores/esp8266/Print.cpp b/cores/esp8266/Print.cpp
@@ -33,6 +33,16 @@
 
 /* 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++);

diff --git 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) {
@@ -214,24 +214,31 @@ void uart_stop_isr(uart_t* uart)
     ETS_UART_INTR_ATTACH(NULL, NULL);
 }
 
+static void uart_do_write_char(uart_t* uart, char c)
+{
+    while((USS(uart->uart_nr) >> USTXC) >= 0x7f);
+    USF(uart->uart_nr) = c;
+}
 
-void uart_write_char(uart_t* uart, char c)
+size_t uart_write_char(uart_t* uart, char c)
 {
     if(uart == NULL || !uart->tx_enabled) {
-        return;
+        return 0;
     }
-    while((USS(uart->uart_nr) >> USTXC) >= 0x7f);
-    USF(uart->uart_nr) = c;
+    uart_do_write_char(uart, c);
+    return 1;
 }
 
-void 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) {
-        return;
+        return 0;
     }
-    while(size--) {
-        uart_write_char(uart, *buf++);
+    size_t ret = size;
+    while (size--) {
+        uart_do_write_char(uart, *buf++);
     }
+    return ret;
 }
 
 size_t uart_tx_free(uart_t* uart)

diff --git 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);