Skip to content

Commit e8ac9c0

Browse files
authored
Merge pull request #2 from espreng/Serial_bugfix
Serial bugfix
2 parents 1b274ff + fe7f131 commit e8ac9c0

File tree

8 files changed

+100
-61
lines changed

8 files changed

+100
-61
lines changed

Diff for: cores/arduino/UART.cpp

+78-46
Original file line numberDiff line numberDiff line change
@@ -101,24 +101,38 @@ void UartClass::_tx_data_empty_irq(void)
101101
// buffer. Send the next byte
102102
unsigned char c = _tx_buffer[_tx_buffer_tail];
103103
_tx_buffer_tail = (_tx_buffer_tail + 1) % SERIAL_TX_BUFFER_SIZE;
104-
105-
(*_hwserial_module).TXDATAL = c;
106-
104+
107105
// clear the TXCIF flag -- "can be cleared by writing a one to its bit
108106
// location". This makes sure flush() won't return until the bytes
109107
// actually got written
110-
(*_hwserial_module).STATUS |= USART_TXCIF_bm;
111-
112-
if (_tx_buffer_head == _tx_buffer_tail) {
113-
// Buffer empty, so disable "data register empty" interrupt
114-
(*_hwserial_module).CTRLA &= (~USART_DREIE_bm);
115-
}
108+
(*_hwserial_module).STATUS = USART_TXCIF_bm;
109+
110+
(*_hwserial_module).TXDATAL = c;
111+
112+
while(!((*_hwserial_module).STATUS & USART_DREIF_bm));
113+
114+
if (_tx_buffer_head == _tx_buffer_tail) {
115+
// Buffer empty, so disable "data register empty" interrupt
116+
(*_hwserial_module).CTRLA &= (~USART_DREIE_bm);
117+
118+
//Take the DRE interrupt back no normal priority level if it has been elevated
119+
if(_hwserial_dre_interrupt_elevated){
120+
CPUINT.LVL1VEC = _prev_lvl1_interrupt_vect;
121+
_hwserial_dre_interrupt_elevated = 0;
122+
}
123+
}
116124
}
117125

118126
// Public Methods //////////////////////////////////////////////////////////////
119127

120128
void UartClass::begin(unsigned long baud, uint16_t config)
121129
{
130+
// Make sure no transmissions are ongoing and USART is disabled in case begin() is called by accident
131+
// without first calling end()
132+
if(_written){
133+
this->end();
134+
}
135+
122136
//uint16_t baud_setting = 0;
123137
int32_t baud_setting = 0;
124138
uint8_t error = 0;
@@ -127,16 +141,6 @@ void UartClass::begin(unsigned long baud, uint16_t config)
127141
uint8_t oldSREG = SREG;
128142
cli();
129143

130-
//Set up the rx and rx pins
131-
pinMode(_hwserial_rx_pin, INPUT_PULLUP);
132-
133-
pinMode(_hwserial_tx_pin, OUTPUT);
134-
digitalWrite(_hwserial_tx_pin, HIGH);
135-
136-
// Make sure no transmissions are ongoing in case begin() is called by accident
137-
// without first calling end()
138-
flush();
139-
140144
// ********Check if desired baud rate is within the acceptable range for using CLK2X RX-mode********
141145
// Condition from datasheet
142146
// This limits the minimum baud_setting value to 64 (0x0040)
@@ -178,7 +182,7 @@ void UartClass::begin(unsigned long baud, uint16_t config)
178182
error = 1;
179183
}
180184

181-
// Do nothing if an invalid baud rate is requested
185+
// Do nothing if an invalid baud rate is requested
182186
if(!error) {
183187

184188
#ifdef PERFORM_BAUD_CORRECTION
@@ -199,15 +203,25 @@ void UartClass::begin(unsigned long baud, uint16_t config)
199203
}
200204
#endif
201205

202-
// assign the baud_setting, a.k.a. BAUD (USART Baud Rate Register)
203-
(*_hwserial_module).BAUD = (int16_t) baud_setting;
204-
205206
_written = false;
206207

207-
(*_hwserial_module).CTRLC = config;
208+
//Set up the rx pin
209+
pinMode(_hwserial_rx_pin, INPUT_PULLUP);
210+
211+
//Set up the tx pin
212+
digitalWrite(_hwserial_tx_pin, HIGH);
213+
pinMode(_hwserial_tx_pin, OUTPUT);
208214

215+
// assign the baud_setting, a.k.a. BAUD (USART Baud Rate Register)
216+
(*_hwserial_module).BAUD = (int16_t) baud_setting;
217+
218+
// Set USART mode of operation
219+
(*_hwserial_module).CTRLC = config;
220+
221+
// Enable transmitter and receiver
222+
(*_hwserial_module).CTRLB |= (USART_RXEN_bm | USART_TXEN_bm);
223+
209224
(*_hwserial_module).CTRLA |= USART_RXCIE_bm;
210-
(*_hwserial_module).CTRLB |= (USART_RXEN_bm | USART_TXEN_bm);
211225
}
212226

213227
// Restore SREG content
@@ -277,16 +291,25 @@ void UartClass::flush()
277291
if (!_written) {
278292
return;
279293
}
294+
295+
//Check if we are inside an ISR already (e.g. connected to a different peripheral then UART), in which case the UART ISRs will not be called.
296+
//Temporarily elevate the DRE interrupt to allow it to run.
297+
if(CPUINT.STATUS & CPUINT_LVL0EX_bm){
298+
//Elevate the priority level of the Data Register Empty Interrupt vector
299+
//and copy whatever vector number that might be in the register already.
300+
_prev_lvl1_interrupt_vect = CPUINT.LVL1VEC;
301+
CPUINT.LVL1VEC = _hwserial_dre_interrupt_vect_num;
302+
303+
_hwserial_dre_interrupt_elevated = 1;
304+
}
280305

281306
// Spin until the data-register-empty-interrupt is disabled and TX complete interrupt flag is raised
282307
while ( ((*_hwserial_module).CTRLA & USART_DREIE_bm) || (!((*_hwserial_module).STATUS & USART_TXCIF_bm)) ) {
283308

284309
// If interrupts are globally disabled or the and DR empty interrupt is disabled,
285310
// poll the "data register empty" interrupt flag to prevent deadlock
286-
if ( (!(SREG & CPU_I_bm)) || (!((*_hwserial_module).CTRLA & USART_DREIE_bm)) ){ //TODO: Verify that || instead of && is correct here
287-
if ( (*_hwserial_module).STATUS & USART_DREIF_bm ){
288-
_tx_data_empty_irq();
289-
}
311+
if ( (!(SREG & CPU_I_bm)) || (!((*_hwserial_module).CTRLA & USART_DREIE_bm)) ){
312+
_tx_data_empty_irq();
290313
}
291314
}
292315
// If we get here, nothing is queued anymore (DREIE is disabled) and
@@ -303,7 +326,7 @@ size_t UartClass::write(uint8_t c)
303326
// 500kbit/s) bit rates, where interrupt overhead becomes a slowdown.
304327
if ( (_tx_buffer_head == _tx_buffer_tail) && ((*_hwserial_module).STATUS & USART_DREIF_bm) ) {
305328
(*_hwserial_module).TXDATAL = c;
306-
(*_hwserial_module).STATUS |= USART_TXCIF_bm;
329+
(*_hwserial_module).STATUS = USART_TXCIF_bm;
307330

308331
// Make sure data register empty interrupt is disabled to avoid
309332
// that the interrupt handler is called in this situation
@@ -312,24 +335,33 @@ size_t UartClass::write(uint8_t c)
312335
return 1;
313336
}
314337

338+
//Check if we are inside an ISR already (could be from by a source other than UART),
339+
// in which case the UART ISRs will be blocked.
340+
if(CPUINT.STATUS & CPUINT_LVL0EX_bm){
341+
//Elevate the priority level of the Data Register Empty Interrupt vector
342+
//and copy whatever vector number that might be in the register already.
343+
_prev_lvl1_interrupt_vect = CPUINT.LVL1VEC;
344+
CPUINT.LVL1VEC = _hwserial_dre_interrupt_vect_num;
345+
346+
_hwserial_dre_interrupt_elevated = 1;
347+
}
348+
315349
tx_buffer_index_t i = (_tx_buffer_head + 1) % SERIAL_TX_BUFFER_SIZE;
316350

317-
// If the output buffer is full, there's nothing for it other than to
318-
// wait for the interrupt handler to empty it a bit
319-
while (i == _tx_buffer_tail) {
320-
321-
if ( (!(SREG & CPU_I_bm)) || (!((*_hwserial_module).CTRLA & USART_DREIE_bm)) ) {//TODO: Verify addition of DREIE-check
322-
// Interrupts are disabled either globally or for data register empty,
323-
// so we'll have to poll the "data register empty" flag ourselves.
324-
// If it is set, pretend an interrupt has happened and call the handler
325-
//to free up space for us.
326-
if( (*_hwserial_module).STATUS & USART_DREIF_bm) {
327-
_tx_data_empty_irq();
328-
}
329-
} else {
330-
// nop, the interrupt handler will free up space for us
331-
}
332-
}
351+
//If the output buffer is full, there's nothing for it other than to
352+
//wait for the interrupt handler to empty it a bit
353+
while (i == _tx_buffer_tail) {
354+
if ( ( !(SREG & CPU_I_bm) ) || ( !((*_hwserial_module).CTRLA & USART_DREIE_bm) ) ){
355+
// Interrupts are disabled either globally or for data register empty,
356+
// so we'll have to poll the "data register empty" flag ourselves.
357+
// If it is set, pretend an interrupt has happened and call the handler
358+
//to free up space for us.
359+
360+
_tx_data_empty_irq();
361+
} else {
362+
// nop, the interrupt handler will free up space for us
363+
}
364+
}
333365

334366
_tx_buffer[_tx_buffer_head] = c;
335367
_tx_buffer_head = i;

Diff for: cores/arduino/UART.h

+6-2
Original file line numberDiff line numberDiff line change
@@ -135,15 +135,19 @@ class UartClass : public HardwareSerial
135135
volatile rx_buffer_index_t _rx_buffer_tail;
136136
volatile tx_buffer_index_t _tx_buffer_head;
137137
volatile tx_buffer_index_t _tx_buffer_tail;
138-
138+
139+
volatile uint8_t _hwserial_dre_interrupt_vect_num;
140+
volatile uint8_t _hwserial_dre_interrupt_elevated;
141+
volatile uint8_t _prev_lvl1_interrupt_vect;
142+
139143
// Don't put any members after these buffers, since only the first
140144
// 32 bytes of this struct can be accessed quickly using the ldd
141145
// instruction.
142146
unsigned char _rx_buffer[SERIAL_RX_BUFFER_SIZE];
143147
unsigned char _tx_buffer[SERIAL_TX_BUFFER_SIZE];
144148

145149
public:
146-
inline UartClass(volatile USART_t *hwserial_module, uint8_t hwserial_rx_pin, uint8_t hwserial_tx_pin);
150+
inline UartClass(volatile USART_t *hwserial_module, uint8_t hwserial_rx_pin, uint8_t hwserial_tx_pin, uint8_t dre_vect_num);
147151
void begin(unsigned long baud) { begin(baud, SERIAL_8N1); }
148152
void begin(unsigned long, uint16_t);
149153
void end();

Diff for: cores/arduino/UART0.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ ISR(HWSERIAL0_RXC_VECTOR)
4545
#error "Don't know what the Data Received interrupt vector is called for Serial"
4646
#endif
4747

48-
//!!BUG in headerfile. The RXC and DRE vectors are swapped!!
4948
#if defined(HWSERIAL0_DRE_VECTOR)
5049
ISR(HWSERIAL0_DRE_VECTOR)
5150
{
@@ -56,7 +55,7 @@ ISR(HWSERIAL0_DRE_VECTOR)
5655
#endif
5756

5857
#if defined(HWSERIAL0)
59-
UartClass Serial(HWSERIAL0, PIN_WIRE_HWSERIAL0_RX, PIN_WIRE_HWSERIAL0_TX);
58+
UartClass Serial(HWSERIAL0, PIN_WIRE_HWSERIAL0_RX, PIN_WIRE_HWSERIAL0_TX, HWSERIAL0_DRE_VECTOR_NUM);
6059
#endif
6160

6261
// Function that can be weakly referenced by serialEventRun to prevent

Diff for: cores/arduino/UART1.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ ISR(HWSERIAL1_DRE_VECTOR)
5555
#endif
5656

5757
#if defined(HWSERIAL1)
58-
UartClass Serial1(HWSERIAL1, PIN_WIRE_HWSERIAL1_RX, PIN_WIRE_HWSERIAL1_TX);
58+
UartClass Serial1(HWSERIAL1, PIN_WIRE_HWSERIAL1_RX, PIN_WIRE_HWSERIAL1_TX, HWSERIAL1_DRE_VECTOR_NUM);
5959
#endif
6060

6161
// Function that can be weakly referenced by serialEventRun to prevent

Diff for: cores/arduino/UART2.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ ISR(HWSERIAL2_DRE_VECTOR)
5555
#endif
5656

5757
#if defined(HWSERIAL2)
58-
UartClass Serial2(HWSERIAL2, PIN_WIRE_HWSERIAL2_RX, PIN_WIRE_HWSERIAL2_TX);
58+
UartClass Serial2(HWSERIAL2, PIN_WIRE_HWSERIAL2_RX, PIN_WIRE_HWSERIAL2_TX, HWSERIAL2_DRE_VECTOR_NUM);
5959
#endif
6060

6161
// Function that can be weakly referenced by serialEventRun to prevent

Diff for: cores/arduino/UART3.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ ISR(HWSERIAL3_DRE_VECTOR)
5555
#endif
5656

5757
#if defined(HWSERIAL3)
58-
UartClass Serial3(HWSERIAL3, PIN_WIRE_HWSERIAL3_RX, PIN_WIRE_HWSERIAL3_TX);
58+
UartClass Serial3(HWSERIAL3, PIN_WIRE_HWSERIAL3_RX, PIN_WIRE_HWSERIAL3_TX, HWSERIAL3_DRE_VECTOR_NUM);
5959
#endif
6060

6161
// Function that can be weakly referenced by serialEventRun to prevent

Diff for: cores/arduino/UART_private.h

+7-3
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,16 @@
3232
UartClass::UartClass(
3333
volatile USART_t *hwserial_module,
3434
volatile uint8_t hwserial_rx_pin,
35-
volatile uint8_t hwserial_tx_pin) :
35+
volatile uint8_t hwserial_tx_pin,
36+
volatile uint8_t hwserial_dre_interrupt_vect_num) :
3637
_hwserial_module(hwserial_module),
3738
_hwserial_rx_pin(hwserial_rx_pin),
3839
_hwserial_tx_pin(hwserial_tx_pin),
40+
_hwserial_dre_interrupt_vect_num(hwserial_dre_interrupt_vect_num),
3941
_rx_buffer_head(0), _rx_buffer_tail(0),
40-
_tx_buffer_head(0), _tx_buffer_tail(0)
42+
_tx_buffer_head(0), _tx_buffer_tail(0),
43+
_hwserial_dre_interrupt_elevated(0),
44+
_prev_lvl1_interrupt_vect(0)
4145
{
4246
}
4347

@@ -63,7 +67,7 @@ void UartClass::_rx_complete_irq(void)
6367
} else {
6468
// Parity error, read byte but discard it
6569
(*_hwserial_module).RXDATAL;
66-
};
70+
}
6771
}
6872

6973
#endif // whole file

Diff for: variants/uno2018/pins_arduino.h

+5-5
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ static const uint8_t MOSI = PIN_SPI_MOSI;
4646
static const uint8_t MISO = PIN_SPI_MISO;
4747
static const uint8_t SCK = PIN_SPI_SCK;
4848

49-
#define PIN_WIRE_SDA (18)
50-
#define PIN_WIRE_SCL (19)
49+
#define PIN_WIRE_SDA (20)
50+
#define PIN_WIRE_SCL (21)
5151

5252
static const uint8_t SDA = PIN_WIRE_SDA;
5353
static const uint8_t SCL = PIN_WIRE_SCL;
@@ -56,8 +56,8 @@ static const uint8_t SCL = PIN_WIRE_SCL;
5656
// USART1 on mega4809 (alternative pins)
5757
// Mapped to HWSERIAL0 in Serial library
5858
#define HWSERIAL0 (&USART1)
59-
//!!BUG in device header file. The RXC and DRE vectors are swapped!!
6059
#define HWSERIAL0_DRE_VECTOR (USART1_DRE_vect)
60+
#define HWSERIAL0_DRE_VECTOR_NUM (USART1_DRE_vect_num)
6161
#define HWSERIAL0_RXC_VECTOR (USART1_RXC_vect)
6262
#define PIN_WIRE_HWSERIAL0_RX (0)
6363
#define PIN_WIRE_HWSERIAL0_TX (1)
@@ -66,8 +66,8 @@ static const uint8_t SCL = PIN_WIRE_SCL;
6666
// USART3 on mega4809 (alternative pins)
6767
// Mapped to HWSERIAL1 in Serial library
6868
#define HWSERIAL1 (&USART3)
69-
//!!BUG in device header file. The RXC and DRE vectors are swapped!!
7069
#define HWSERIAL1_DRE_VECTOR (USART3_DRE_vect)
70+
#define HWSERIAL1_DRE_VECTOR_NUM (USART3_DRE_vect_num)
7171
#define HWSERIAL1_RXC_VECTOR (USART3_RXC_vect)
7272
#define PIN_WIRE_HWSERIAL1_RX (26)
7373
#define PIN_WIRE_HWSERIAL1_TX (27)
@@ -76,8 +76,8 @@ static const uint8_t SCL = PIN_WIRE_SCL;
7676
// USART0 on mega4809 (alternative pins)
7777
// Mapped to HWSERIAL2 in Serial library
7878
#define HWSERIAL2 (&USART0)
79-
//!!BUG in device header file. The RXC and DRE vectors are swapped!!
8079
#define HWSERIAL2_DRE_VECTOR (USART0_DRE_vect)
80+
#define HWSERIAL2_DRE_VECTOR_NUM (USART0_DRE_vect_num)
8181
#define HWSERIAL2_RXC_VECTOR (USART0_RXC_vect)
8282
#define PIN_WIRE_HWSERIAL2_RX (23)
8383
#define PIN_WIRE_HWSERIAL2_TX (24)

0 commit comments

Comments
 (0)