Skip to content

Commit ce1b64b

Browse files
igrrigrr
committed
Merge pull request esp8266#1320 from alltheblinkythings/SerialInterframeFixes
Fixes for poor HardwareSerial performance/crashes exacerbated by SDK1.5
2 parents 1581a8f + 9d7c2fd commit ce1b64b

File tree

8 files changed

+131
-73
lines changed

8 files changed

+131
-73
lines changed

‎cores/esp8266/HardwareSerial.cpp

+69-47
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,8 @@ void uart_disarm_tx_interrupt(uart_t* uart);
101101
void uart_set_baudrate(uart_t* uart, int baud_rate);
102102
int uart_get_baudrate(uart_t* uart);
103103

104-
uart_t* uart_init(int uart_nr, int baudrate, byte config);
104+
uart_t* uart_start_init(int uart_nr, int baudrate, byte config);
105+
void uart_finish_init(uart_t* uart);
105106
void uart_uninit(uart_t* uart);
106107
void uart_swap(uart_t* uart);
107108

@@ -116,6 +117,13 @@ int uart_get_debug();
116117
// ####################################################################################################
117118
// ####################################################################################################
118119

120+
// These function internals can be used from interrupt handlers to ensure they
121+
// are in instruction RAM, or anywhere that the uart_nr has been validated.
122+
#define UART_GET_TX_FIFO_ROOM(uart_nr) (UART_TX_FIFO_SIZE - ((USS(uart_nr) >> USTXC) & 0xff))
123+
#define UART_TRANSMIT_CHAR(uart_nr, c) do { USF(uart_nr) = (c); } while(0)
124+
#define UART_ARM_TX_INTERRUPT(uart_nr) do { USIE(uart_nr) |= (1 << UIFE); } while(0)
125+
#define UART_DISARM_TX_INTERRUPT(uart_nr) do { USIE(uart_nr) &= ~(1 << UIFE); } while(0)
126+
119127
void ICACHE_RAM_ATTR uart_interrupt_handler(uart_t* uart) {
120128

121129
// -------------- UART 0 --------------
@@ -133,13 +141,7 @@ void ICACHE_RAM_ATTR uart_interrupt_handler(uart_t* uart) {
133141
}
134142

135143
// -------------- UART 1 --------------
136-
137-
if(Serial1.isRxEnabled()) {
138-
while(U1IS & (1 << UIFF)) {
139-
Serial1._rx_complete_irq((char) (U1F & 0xff));
140-
U1IC = (1 << UIFF);
141-
}
142-
}
144+
// Note: only TX is supported on UART 1.
143145
if(Serial1.isTxEnabled()) {
144146
if(U1IS & (1 << UIFE)) {
145147
U1IC = (1 << UIFE);
@@ -166,7 +168,7 @@ size_t uart_get_tx_fifo_room(uart_t* uart) {
166168
if(uart == 0)
167169
return 0;
168170
if(uart->txEnabled) {
169-
return UART_TX_FIFO_SIZE - ((USS(uart->uart_nr) >> USTXC) & 0xff);
171+
return UART_GET_TX_FIFO_ROOM(uart->uart_nr);
170172
}
171173
return 0;
172174
}
@@ -183,7 +185,7 @@ void uart_transmit_char(uart_t* uart, char c) {
183185
if(uart == 0)
184186
return;
185187
if(uart->txEnabled) {
186-
USF(uart->uart_nr) = c;
188+
UART_TRANSMIT_CHAR(uart->uart_nr, c);
187189
}
188190
}
189191

@@ -247,15 +249,15 @@ void uart_arm_tx_interrupt(uart_t* uart) {
247249
if(uart == 0)
248250
return;
249251
if(uart->txEnabled) {
250-
USIE(uart->uart_nr) |= (1 << UIFE);
252+
UART_ARM_TX_INTERRUPT(uart->uart_nr);
251253
}
252254
}
253255

254256
void uart_disarm_tx_interrupt(uart_t* uart) {
255257
if(uart == 0)
256258
return;
257259
if(uart->txEnabled) {
258-
USIE(uart->uart_nr) &= ~(1 << UIFE);
260+
UART_DISARM_TX_INTERRUPT(uart->uart_nr);
259261
}
260262
}
261263

@@ -272,9 +274,8 @@ int uart_get_baudrate(uart_t* uart) {
272274
return uart->baud_rate;
273275
}
274276

275-
uart_t* uart_init(int uart_nr, int baudrate, byte config, byte mode) {
277+
uart_t* uart_start_init(int uart_nr, int baudrate, byte config, byte mode) {
276278

277-
uint32_t conf1 = 0x00000000;
278279
uart_t* uart = (uart_t*) os_malloc(sizeof(uart_t));
279280

280281
if(uart == 0) {
@@ -294,6 +295,7 @@ uart_t* uart_init(int uart_nr, int baudrate, byte config, byte mode) {
294295
IOSWAP &= ~(1 << IOSWAPU0);
295296
break;
296297
case UART1:
298+
// Note: uart_interrupt_handler does not support RX on UART 1.
297299
uart->rxEnabled = false;
298300
uart->txEnabled = (mode != SERIAL_RX_ONLY);
299301
uart->rxPin = 255;
@@ -309,6 +311,12 @@ uart_t* uart_init(int uart_nr, int baudrate, byte config, byte mode) {
309311
uart_set_baudrate(uart, baudrate);
310312
USC0(uart->uart_nr) = config;
311313

314+
return uart;
315+
}
316+
317+
void uart_finish_init(uart_t* uart) {
318+
uint32_t conf1 = 0x00000000;
319+
312320
uart_flush(uart);
313321
uart_interrupt_enable(uart);
314322

@@ -321,8 +329,6 @@ uart_t* uart_init(int uart_nr, int baudrate, byte config, byte mode) {
321329
}
322330

323331
USC1(uart->uart_nr) = conf1;
324-
325-
return uart;
326332
}
327333

328334
void uart_uninit(uart_t* uart) {
@@ -479,35 +485,48 @@ int uart_get_debug() {
479485
// ####################################################################################################
480486

481487
HardwareSerial::HardwareSerial(int uart_nr) :
482-
_uart_nr(uart_nr), _uart(0), _tx_buffer(0), _rx_buffer(0), _written(false) {
488+
_uart_nr(uart_nr), _uart(0), _tx_buffer(0), _rx_buffer(0) {
483489
}
484490

485491
void HardwareSerial::begin(unsigned long baud, byte config, byte mode) {
492+
InterruptLock il;
486493

487494
// disable debug for this interface
488495
if(uart_get_debug() == _uart_nr) {
489496
uart_set_debug(UART_NO);
490497
}
491498

492-
_uart = uart_init(_uart_nr, baud, config, mode);
499+
if (_uart) {
500+
os_free(_uart);
501+
}
502+
_uart = uart_start_init(_uart_nr, baud, config, mode);
493503

494504
if(_uart == 0) {
495505
return;
496506
}
497507

498-
if(_uart->rxEnabled) {
499-
if(!_rx_buffer)
500-
_rx_buffer = new cbuf(SERIAL_RX_BUFFER_SIZE);
508+
// Disable the RX and/or TX functions if we fail to allocate circular buffers.
509+
// The user can confirm they are enabled with isRxEnabled() and isTxEnabled().
510+
if(_uart->rxEnabled && !_rx_buffer) {
511+
_rx_buffer = new cbuf(SERIAL_RX_BUFFER_SIZE);
512+
if(!_rx_buffer) {
513+
_uart->rxEnabled = false;
514+
}
501515
}
502-
if(_uart->txEnabled) {
503-
if(!_tx_buffer)
504-
_tx_buffer = new cbuf(SERIAL_TX_BUFFER_SIZE);
516+
if(_uart->txEnabled && !_tx_buffer) {
517+
_tx_buffer = new cbuf(SERIAL_TX_BUFFER_SIZE);
518+
if(!_tx_buffer) {
519+
_uart->txEnabled = false;
520+
}
505521
}
506-
_written = false;
507522
delay(1);
523+
524+
uart_finish_init(_uart);
508525
}
509526

510527
void HardwareSerial::end() {
528+
InterruptLock il;
529+
511530
if(uart_get_debug() == _uart_nr) {
512531
uart_set_debug(UART_NO);
513532
}
@@ -541,13 +560,13 @@ void HardwareSerial::setDebugOutput(bool en) {
541560
}
542561
}
543562

544-
bool HardwareSerial::isTxEnabled(void) {
563+
bool ICACHE_RAM_ATTR HardwareSerial::isTxEnabled(void) {
545564
if(_uart == 0)
546565
return false;
547566
return _uart->txEnabled;
548567
}
549568

550-
bool HardwareSerial::isRxEnabled(void) {
569+
bool ICACHE_RAM_ATTR HardwareSerial::isRxEnabled(void) {
551570
if(_uart == 0)
552571
return false;
553572
return _uart->rxEnabled;
@@ -606,71 +625,74 @@ void HardwareSerial::flush() {
606625
return;
607626
if(!_uart->txEnabled)
608627
return;
609-
if(!_written)
610-
return;
611628

629+
const int uart_nr = _uart->uart_nr;
612630
while(true) {
613631
{
614632
InterruptLock il;
615633
if(_tx_buffer->getSize() == 0 &&
616-
uart_get_tx_fifo_room(_uart) >= UART_TX_FIFO_SIZE) {
634+
UART_GET_TX_FIFO_ROOM(uart_nr) >= UART_TX_FIFO_SIZE) {
617635
break;
636+
} else if(il.savedInterruptLevel() > 0) {
637+
_tx_empty_irq();
638+
continue;
618639
}
619640
}
620641
yield();
621642
}
622-
_written = false;
623643
}
624644

625645
size_t HardwareSerial::write(uint8_t c) {
626646
if(_uart == 0 || !_uart->txEnabled)
627647
return 0;
628-
_written = true;
629648

649+
bool tx_now = false;
650+
const int uart_nr = _uart->uart_nr;
630651
while(true) {
631652
{
632653
InterruptLock il;
633654
if(_tx_buffer->empty()) {
634-
if(uart_get_tx_fifo_room(_uart) > 0) {
635-
uart_transmit_char(_uart, c);
655+
if(UART_GET_TX_FIFO_ROOM(uart_nr) > 0) {
656+
tx_now = true;
636657
} else {
637658
_tx_buffer->write(c);
638-
uart_arm_tx_interrupt(_uart);
659+
UART_ARM_TX_INTERRUPT(uart_nr);
639660
}
640661
break;
641662
} else if(_tx_buffer->write(c)) {
642663
break;
664+
} else if(il.savedInterruptLevel() > 0) {
665+
_tx_empty_irq();
666+
continue;
643667
}
644668
}
645669
yield();
646670
}
671+
if (tx_now) {
672+
UART_TRANSMIT_CHAR(uart_nr, c);
673+
}
647674
return 1;
648675
}
649676

650677
HardwareSerial::operator bool() const {
651678
return _uart != 0;
652679
}
653680

654-
void HardwareSerial::_rx_complete_irq(char c) {
655-
if(_rx_buffer) {
656-
_rx_buffer->write(c);
657-
}
681+
void ICACHE_RAM_ATTR HardwareSerial::_rx_complete_irq(char c) {
682+
_rx_buffer->write(c);
658683
}
659684

660-
void HardwareSerial::_tx_empty_irq(void) {
661-
if(_uart == 0)
662-
return;
663-
if(_tx_buffer == 0)
664-
return;
685+
void ICACHE_RAM_ATTR HardwareSerial::_tx_empty_irq(void) {
686+
const int uart_nr = _uart->uart_nr;
665687
size_t queued = _tx_buffer->getSize();
666688
if(!queued) {
667-
uart_disarm_tx_interrupt(_uart);
689+
UART_DISARM_TX_INTERRUPT(uart_nr);
668690
return;
669691
}
670692

671-
size_t room = uart_get_tx_fifo_room(_uart);
693+
size_t room = UART_GET_TX_FIFO_ROOM(uart_nr);
672694
int n = static_cast<int>((queued < room) ? queued : room);
673695
while(n--) {
674-
uart_transmit_char(_uart, _tx_buffer->read());
696+
UART_TRANSMIT_CHAR(uart_nr, _tx_buffer->read());
675697
}
676698
}

‎cores/esp8266/HardwareSerial.h

-1
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,6 @@ class HardwareSerial: public Stream {
117117
uart_t* _uart;
118118
cbuf* _tx_buffer;
119119
cbuf* _rx_buffer;
120-
bool _written;
121120
};
122121

123122
extern HardwareSerial Serial;

‎cores/esp8266/cbuf.cpp

+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*
2+
cbuf.cpp - Circular buffer implementation
3+
Copyright (c) 2014 Ivan Grokhotkov. All rights reserved.
4+
This file is part of the esp8266 core for Arduino environment.
5+
6+
This library is free software; you can redistribute it and/or
7+
modify it under the terms of the GNU Lesser General Public
8+
License as published by the Free Software Foundation; either
9+
version 2.1 of the License, or (at your option) any later version.
10+
11+
This library is distributed in the hope that it will be useful,
12+
but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
Lesser General Public License for more details.
15+
16+
You should have received a copy of the GNU Lesser General Public
17+
License along with this library; if not, write to the Free Software
18+
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
19+
*/
20+
21+
#include "cbuf.h"
22+
#include "c_types.h"
23+
24+
size_t ICACHE_RAM_ATTR cbuf::getSize() const {
25+
if(_end >= _begin) {
26+
return _end - _begin;
27+
}
28+
return _size - (_begin - _end);
29+
}
30+
31+
int ICACHE_RAM_ATTR cbuf::read() {
32+
if(empty()) return -1;
33+
34+
char result = *_begin;
35+
_begin = wrap_if_bufend(_begin + 1);
36+
return static_cast<int>(result);
37+
}
38+
39+
size_t ICACHE_RAM_ATTR cbuf::write(char c) {
40+
if(full()) return 0;
41+
42+
*_end = c;
43+
_end = wrap_if_bufend(_end + 1);
44+
return 1;
45+
}

0 commit comments

Comments
 (0)