-Fix for Serial bug where it hangs if called from inside an interrupt.

DRE interrupt is temporarily elevated to priority 1. Basic testing done.

-Fixed bug that caused the TX pin to be held low for a short while during
initialization, in effect transmitting some garbage intermittenly.

Author: Bekken, Kristian_saxrud

cores/arduino/UART.cpp

@@ -101,24 +101,38 @@ void UartClass::_tx_data_empty_irq(void)
   // buffer. Send the next byte
   unsigned char c = _tx_buffer[_tx_buffer_tail];
   _tx_buffer_tail = (_tx_buffer_tail + 1) % SERIAL_TX_BUFFER_SIZE;
-
-  (*_hwserial_module).TXDATAL = c;
-
+  
   // clear the TXCIF flag -- "can be cleared by writing a one to its bit
   // location". This makes sure flush() won't return until the bytes
   // actually got written
-  (*_hwserial_module).STATUS |= USART_TXCIF_bm;
-
-  if (_tx_buffer_head == _tx_buffer_tail) {
-    // Buffer empty, so disable "data register empty" interrupt
-    (*_hwserial_module).CTRLA &= (~USART_DREIE_bm);
-  }
+  (*_hwserial_module).STATUS = USART_TXCIF_bm;
+  
+  (*_hwserial_module).TXDATAL = c;
+  
+  while(!((*_hwserial_module).STATUS & USART_DREIF_bm));
+
+   if (_tx_buffer_head == _tx_buffer_tail) {
+     // Buffer empty, so disable "data register empty" interrupt
+     (*_hwserial_module).CTRLA &= (~USART_DREIE_bm);
+	 
+	 //Take the DRE interrupt back no normal priority level if it has been elevated
+	 if(_hwserial_dre_interrupt_elevated){
+		 CPUINT.LVL1VEC = _prev_lvl1_interrupt_vect;
+		 _hwserial_dre_interrupt_elevated = 0;
+	 }	 
+   }
 }
 
 // Public Methods //////////////////////////////////////////////////////////////
 
 void UartClass::begin(unsigned long baud, uint16_t config)
 {
+  // Make sure no transmissions are ongoing and USART is disabled in case begin() is called by accident
+  // without first calling end()
+  if(_written){
+	  this->end();
+  }
+
   //uint16_t baud_setting = 0;
   int32_t baud_setting = 0;
   uint8_t error = 0;
@@ -127,16 +141,6 @@ void UartClass::begin(unsigned long baud, uint16_t config)
   uint8_t oldSREG = SREG;
   cli();
 
-  //Set up the rx and rx pins
-  pinMode(_hwserial_rx_pin, INPUT_PULLUP);
-
-  pinMode(_hwserial_tx_pin, OUTPUT);
-  digitalWrite(_hwserial_tx_pin, HIGH);
-
-  // Make sure no transmissions are ongoing in case begin() is called by accident
-  // without first calling end()
-  flush();
-
   // ********Check if desired baud rate is within the acceptable range for using CLK2X RX-mode********
   // Condition from datasheet
   // This limits the minimum baud_setting value to 64 (0x0040)
@@ -178,7 +182,7 @@ void UartClass::begin(unsigned long baud, uint16_t config)
     error = 1;
   }
 
-  // Do nothing if an invalid baud rate is requested
+// Do nothing if an invalid baud rate is requested
   if(!error) {
 
 #ifdef PERFORM_BAUD_CORRECTION
@@ -199,15 +203,25 @@ void UartClass::begin(unsigned long baud, uint16_t config)
     }
 #endif
 
-    // assign the baud_setting, a.k.a. BAUD (USART Baud Rate Register)
-    (*_hwserial_module).BAUD = (int16_t) baud_setting;
-
     _written = false;
 
-    (*_hwserial_module).CTRLC = config;
+	//Set up the rx pin
+	pinMode(_hwserial_rx_pin, INPUT_PULLUP);
+	
+	//Set up the tx pin
+	digitalWrite(_hwserial_tx_pin, HIGH);
+	pinMode(_hwserial_tx_pin, OUTPUT);	
 
+	// assign the baud_setting, a.k.a. BAUD (USART Baud Rate Register)
+    (*_hwserial_module).BAUD = (int16_t) baud_setting;
+
+	// Set USART mode of operation
+    (*_hwserial_module).CTRLC = config;
+	
+	// Enable transmitter and receiver	
+	(*_hwserial_module).CTRLB |= (USART_RXEN_bm | USART_TXEN_bm);
+		
     (*_hwserial_module).CTRLA |= USART_RXCIE_bm;
-    (*_hwserial_module).CTRLB |= (USART_RXEN_bm | USART_TXEN_bm);
   }
 
   // Restore SREG content
@@ -277,16 +291,25 @@ void UartClass::flush()
   if (!_written) {
     return;
   }
+  
+  //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.
+  //Temporarily elevate the DRE interrupt to allow it to run.
+  if(CPUINT.STATUS & CPUINT_LVL0EX_bm){
+	  //Elevate the priority level of the Data Register Empty Interrupt vector
+	  //and copy whatever vector number that might be in the register already.
+	  _prev_lvl1_interrupt_vect = CPUINT.LVL1VEC;
+	  CPUINT.LVL1VEC = _hwserial_dre_interrupt_vect_num;
+	  
+	  _hwserial_dre_interrupt_elevated = 1;
+  }
 
   // Spin until the data-register-empty-interrupt is disabled and TX complete interrupt flag is raised
   while ( ((*_hwserial_module).CTRLA & USART_DREIE_bm) || (!((*_hwserial_module).STATUS & USART_TXCIF_bm)) ) {
 
     // If interrupts are globally disabled or the and DR empty interrupt is disabled,
     // poll the "data register empty" interrupt flag to prevent deadlock
-    if ( (!(SREG & CPU_I_bm)) || (!((*_hwserial_module).CTRLA & USART_DREIE_bm)) ){ //TODO: Verify that || instead of && is correct here
-      if ( (*_hwserial_module).STATUS & USART_DREIF_bm ){
-        _tx_data_empty_irq();
-      }
+    if ( (!(SREG & CPU_I_bm)) || (!((*_hwserial_module).CTRLA & USART_DREIE_bm)) ){
+      _tx_data_empty_irq();      
     }
   }
   // If we get here, nothing is queued anymore (DREIE is disabled) and
@@ -303,7 +326,7 @@ size_t UartClass::write(uint8_t c)
   // 500kbit/s) bit rates, where interrupt overhead becomes a slowdown.
   if ( (_tx_buffer_head == _tx_buffer_tail) && ((*_hwserial_module).STATUS & USART_DREIF_bm) ) {
     (*_hwserial_module).TXDATAL = c;
-    (*_hwserial_module).STATUS |= USART_TXCIF_bm;
+    (*_hwserial_module).STATUS = USART_TXCIF_bm;
 
     // Make sure data register empty interrupt is disabled to avoid
     // that the interrupt handler is called in this situation
@@ -312,24 +335,33 @@ size_t UartClass::write(uint8_t c)
     return 1;
   }
 
+	//Check if we are inside an ISR already (could be from by a source other than UART),
+	// in which case the UART ISRs will be blocked.
+	if(CPUINT.STATUS & CPUINT_LVL0EX_bm){
+		//Elevate the priority level of the Data Register Empty Interrupt vector
+		//and copy whatever vector number that might be in the register already.
+		_prev_lvl1_interrupt_vect = CPUINT.LVL1VEC;
+		CPUINT.LVL1VEC = _hwserial_dre_interrupt_vect_num;
+		
+		_hwserial_dre_interrupt_elevated = 1;
+    }
+
   tx_buffer_index_t i = (_tx_buffer_head + 1) % SERIAL_TX_BUFFER_SIZE;
 
-  // If the output buffer is full, there's nothing for it other than to
-  // wait for the interrupt handler to empty it a bit
-  while (i == _tx_buffer_tail) {
-
-    if ( (!(SREG & CPU_I_bm)) || (!((*_hwserial_module).CTRLA & USART_DREIE_bm)) ) {//TODO: Verify addition of DREIE-check
-      // Interrupts are disabled either globally or for data register empty,
-      // so we'll have to poll the "data register empty" flag ourselves.
-      // If it is set, pretend an interrupt has happened and call the handler 
-      //to free up space for us.
-      if( (*_hwserial_module).STATUS & USART_DREIF_bm) {
-        _tx_data_empty_irq();
-      }
-    } else {
-      // nop, the interrupt handler will free up space for us
-    }
-  }
+   //If the output buffer is full, there's nothing for it other than to
+   //wait for the interrupt handler to empty it a bit
+   while (i == _tx_buffer_tail) {
+	   if ( ( !(SREG & CPU_I_bm) ) || ( !((*_hwserial_module).CTRLA & USART_DREIE_bm) ) ){
+		   // Interrupts are disabled either globally or for data register empty,
+		   // so we'll have to poll the "data register empty" flag ourselves.
+		   // If it is set, pretend an interrupt has happened and call the handler
+		   //to free up space for us.
+		   
+		   _tx_data_empty_irq();
+		   } else {
+		   // nop, the interrupt handler will free up space for us
+	   }
+   }
 
   _tx_buffer[_tx_buffer_head] = c;
   _tx_buffer_head = i;

cores/arduino/UART.h

@@ -135,15 +135,19 @@ class UartClass : public HardwareSerial
     volatile rx_buffer_index_t _rx_buffer_tail;
     volatile tx_buffer_index_t _tx_buffer_head;
     volatile tx_buffer_index_t _tx_buffer_tail;
-
+	
+	volatile uint8_t _hwserial_dre_interrupt_vect_num;
+	volatile uint8_t _hwserial_dre_interrupt_elevated;
+	volatile uint8_t _prev_lvl1_interrupt_vect;
+	
     // Don't put any members after these buffers, since only the first
     // 32 bytes of this struct can be accessed quickly using the ldd
     // instruction.
     unsigned char _rx_buffer[SERIAL_RX_BUFFER_SIZE];
     unsigned char _tx_buffer[SERIAL_TX_BUFFER_SIZE];
 
   public:
-    inline UartClass(volatile USART_t *hwserial_module, uint8_t hwserial_rx_pin, uint8_t hwserial_tx_pin);
+    inline UartClass(volatile USART_t *hwserial_module, uint8_t hwserial_rx_pin, uint8_t hwserial_tx_pin, uint8_t dre_vect_num);
     void begin(unsigned long baud) { begin(baud, SERIAL_8N1); }
     void begin(unsigned long, uint16_t);
     void end();

cores/arduino/UART0.cpp

@@ -45,7 +45,6 @@ ISR(HWSERIAL0_RXC_VECTOR)
 #error "Don't know what the Data Received interrupt vector is called for Serial"
 #endif
 
-//!!BUG in headerfile. The RXC and DRE vectors are swapped!!
 #if defined(HWSERIAL0_DRE_VECTOR)
 ISR(HWSERIAL0_DRE_VECTOR)
 {
@@ -56,7 +55,7 @@ ISR(HWSERIAL0_DRE_VECTOR)
 #endif
 
 #if defined(HWSERIAL0)
-  UartClass Serial(HWSERIAL0, PIN_WIRE_HWSERIAL0_RX, PIN_WIRE_HWSERIAL0_TX);
+  UartClass Serial(HWSERIAL0, PIN_WIRE_HWSERIAL0_RX, PIN_WIRE_HWSERIAL0_TX, HWSERIAL0_DRE_VECTOR_NUM);
 #endif
 
 // Function that can be weakly referenced by serialEventRun to prevent

cores/arduino/UART1.cpp

@@ -55,7 +55,7 @@ ISR(HWSERIAL1_DRE_VECTOR)
 #endif
 
 #if defined(HWSERIAL1)
-  UartClass Serial1(HWSERIAL1, PIN_WIRE_HWSERIAL1_RX, PIN_WIRE_HWSERIAL1_TX);
+  UartClass Serial1(HWSERIAL1, PIN_WIRE_HWSERIAL1_RX, PIN_WIRE_HWSERIAL1_TX, HWSERIAL1_DRE_VECTOR_NUM);
 #endif
 
 // Function that can be weakly referenced by serialEventRun to prevent

cores/arduino/UART2.cpp

@@ -55,7 +55,7 @@ ISR(HWSERIAL2_DRE_VECTOR)
 #endif
 
 #if defined(HWSERIAL2)
-  UartClass Serial2(HWSERIAL2, PIN_WIRE_HWSERIAL2_RX, PIN_WIRE_HWSERIAL2_TX);
+  UartClass Serial2(HWSERIAL2, PIN_WIRE_HWSERIAL2_RX, PIN_WIRE_HWSERIAL2_TX, HWSERIAL2_DRE_VECTOR_NUM);
 #endif
 
 // Function that can be weakly referenced by serialEventRun to prevent

cores/arduino/UART3.cpp

@@ -55,7 +55,7 @@ ISR(HWSERIAL3_DRE_VECTOR)
 #endif
 
 #if defined(HWSERIAL3)
-  UartClass Serial3(HWSERIAL3, PIN_WIRE_HWSERIAL3_RX, PIN_WIRE_HWSERIAL3_TX);
+  UartClass Serial3(HWSERIAL3, PIN_WIRE_HWSERIAL3_RX, PIN_WIRE_HWSERIAL3_TX, HWSERIAL3_DRE_VECTOR_NUM);
 #endif
 
 // Function that can be weakly referenced by serialEventRun to prevent

cores/arduino/UART_private.h

@@ -32,12 +32,16 @@
 UartClass::UartClass(
   volatile USART_t *hwserial_module,
   volatile uint8_t hwserial_rx_pin,
-  volatile uint8_t hwserial_tx_pin) :
+  volatile uint8_t hwserial_tx_pin,
+  volatile uint8_t hwserial_dre_interrupt_vect_num) :
     _hwserial_module(hwserial_module),
     _hwserial_rx_pin(hwserial_rx_pin),
     _hwserial_tx_pin(hwserial_tx_pin),
+    _hwserial_dre_interrupt_vect_num(hwserial_dre_interrupt_vect_num),
     _rx_buffer_head(0), _rx_buffer_tail(0),
-    _tx_buffer_head(0), _tx_buffer_tail(0)
+    _tx_buffer_head(0), _tx_buffer_tail(0),
+    _hwserial_dre_interrupt_elevated(0),
+    _prev_lvl1_interrupt_vect(0)
 {
 }
 
@@ -63,7 +67,7 @@ void UartClass::_rx_complete_irq(void)
   } else {
     // Parity error, read byte but discard it
     (*_hwserial_module).RXDATAL;
-  };
+  }
 }
 
 #endif // whole file