Implemented serial transmit buffering.

Now Serial.write() places characters in the transmit buffer, and the data register empty interrupt reads and transmit them.  Based loosely on the implementation here: ftp://wookey.org.uk/arduino.

http://code.google.com/p/arduino/issues/detail?id=262

2 files changed, 138 insertions, 29 deletions

diff --git a/cores/arduino/HardwareSerial.cpp b/cores/arduino/HardwareSerial.cpp
index 7cea350..1cd8cc5 100644
--- a/cores/arduino/HardwareSerial.cpp
+++ b/cores/arduino/HardwareSerial.cpp
@@ -34,46 +34,50 @@
 #include "HardwareSerial.h"
 
 // Define constants and variables for buffering incoming serial data.  We're
-// using a ring buffer (I think), in which rx_buffer_head is the index of the
-// location to which to write the next incoming character and rx_buffer_tail
-// is the index of the location from which to read.
+// using a ring buffer (I think), in which head is the index of the location
+// to which to write the next incoming character and tail is the index of the
+// location from which to read.
 #if (RAMEND < 1000)
-  #define RX_BUFFER_SIZE 32
+  #define SERIAL_BUFFER_SIZE 16
 #else
-  #define RX_BUFFER_SIZE 128
+  #define SERIAL_BUFFER_SIZE 64
 #endif
 
 struct ring_buffer
 {
-  unsigned char buffer[RX_BUFFER_SIZE];
-  int head;
-  int tail;
+  unsigned char buffer[SERIAL_BUFFER_SIZE];
+  volatile int head;
+  volatile int tail;
 };
 
 #if defined(UBRRH) || defined(UBRR0H)
   ring_buffer rx_buffer  =  { { 0 }, 0, 0 };
+  ring_buffer tx_buffer  =  { { 0 }, 0, 0 };
 #endif
 #if defined(UBRR1H)
   ring_buffer rx_buffer1  =  { { 0 }, 0, 0 };
+  ring_buffer tx_buffer1  =  { { 0 }, 0, 0 };
 #endif
 #if defined(UBRR2H)
   ring_buffer rx_buffer2  =  { { 0 }, 0, 0 };
+  ring_buffer tx_buffer2  =  { { 0 }, 0, 0 };
 #endif
 #if defined(UBRR3H)
   ring_buffer rx_buffer3  =  { { 0 }, 0, 0 };
+  ring_buffer tx_buffer3  =  { { 0 }, 0, 0 };
 #endif
 
-inline void store_char(unsigned char c, ring_buffer *rx_buffer)
+inline void store_char(unsigned char c, ring_buffer *buffer)
 {
-  int i = (unsigned int)(rx_buffer->head + 1) % RX_BUFFER_SIZE;
+  int i = (unsigned int)(buffer->head + 1) % SERIAL_BUFFER_SIZE;
 
   // if we should be storing the received character into the location
   // just before the tail (meaning that the head would advance to the
   // current location of the tail), we're about to overflow the buffer
   // and so we don't write the character or advance the head.
-  if (i != rx_buffer->tail) {
-    rx_buffer->buffer[rx_buffer->head] = c;
-    rx_buffer->head = i;
+  if (i != buffer->tail) {
+    buffer->buffer[buffer->head] = c;
+    buffer->head = i;
   }
 }
 
@@ -167,16 +171,105 @@ inline void store_char(unsigned char c, ring_buffer *rx_buffer)
 #endif
 
 
+#if !defined(UART0_UDRE_vect) && !defined(UART_UDRE_vect) && !defined(USART0_UDRE_vect) && !defined(USART_UDRE_vect)
+  #error Don't know what the Data Register Empty vector is called for the first UART
+#else
+#if defined(UART0_UDRE_vect)
+ISR(UART0_UDRE_vect)
+#elif defined(UART_UDRE_vect)
+ISR(UART_UDRE_vect)
+#elif defined(USART0_UDRE_vect)
+ISR(USART0_UDRE_vect)
+#elif defined(USART_UDRE_vect)
+ISR(USART_UDRE_vect)
+#endif
+{
+  if (tx_buffer.head == tx_buffer.tail) {
+	// Buffer empty, so disable interrupts
+#if defined(UCSR0B)
+    cbi(UCSR0B, UDRIE0);
+#else
+    cbi(UCSRB, UDRIE);
+#endif
+  }
+  else {
+    // There is more data in the output buffer. Send the next byte
+    unsigned char c = tx_buffer.buffer[tx_buffer.tail];
+    tx_buffer.tail = (tx_buffer.tail + 1) % SERIAL_BUFFER_SIZE;
+	
+  #if defined(UDR0)
+    UDR0 = c;
+  #elif defined(UDR)
+    UDR = c;
+  #else
+    #error UDR not defined
+  #endif
+  }
+}
+#endif
+
+#ifdef USART1_UDRE_vect
+ISR(USART1_UDRE_vect)
+{
+  if (tx_buffer1.head == tx_buffer1.tail) {
+	// Buffer empty, so disable interrupts
+    cbi(UCSR1B, UDRIE1);
+  }
+  else {
+    // There is more data in the output buffer. Send the next byte
+    unsigned char c = tx_buffer1.buffer[tx_buffer1.tail];
+    tx_buffer1.tail = (tx_buffer1.tail + 1) % SERIAL_BUFFER_SIZE;
+	
+    UDR1 = c;
+  }
+}
+#endif
+
+#ifdef USART2_UDRE_vect
+ISR(USART2_UDRE_vect)
+{
+  if (tx_buffer2.head == tx_buffer2.tail) {
+	// Buffer empty, so disable interrupts
+    cbi(UCSR2B, UDRIE2);
+  }
+  else {
+    // There is more data in the output buffer. Send the next byte
+    unsigned char c = tx_buffer2.buffer[tx_buffer2.tail];
+    tx_buffer2.tail = (tx_buffer2.tail + 1) % SERIAL_BUFFER_SIZE;
+	
+    UDR2 = c;
+  }
+}
+#endif
+
+#ifdef USART3_UDRE_vect
+ISR(USART3_UDRE_vect)
+{
+  if (tx_buffer3.head == tx_buffer3.tail) {
+	// Buffer empty, so disable interrupts
+    cbi(UCSR3B, UDRIE3);
+  }
+  else {
+    // There is more data in the output buffer. Send the next byte
+    unsigned char c = tx_buffer3.buffer[tx_buffer3.tail];
+    tx_buffer3.tail = (tx_buffer3.tail + 1) % SERIAL_BUFFER_SIZE;
+	
+    UDR3 = c;
+  }
+}
+#endif
+
 
 // Constructors ////////////////////////////////////////////////////////////////
 
-HardwareSerial::HardwareSerial(ring_buffer *rx_buffer,
+HardwareSerial::HardwareSerial(ring_buffer *rx_buffer, ring_buffer *tx_buffer,
   volatile uint8_t *ubrrh, volatile uint8_t *ubrrl,
   volatile uint8_t *ucsra, volatile uint8_t *ucsrb,
   volatile uint8_t *udr,
-  uint8_t rxen, uint8_t txen, uint8_t rxcie, uint8_t udre, uint8_t u2x)
+  uint8_t rxen, uint8_t txen, uint8_t rxcie, uint8_t udrie, uint8_t u2x)
 {
   _rx_buffer = rx_buffer;
+  _tx_buffer = tx_buffer;
   _ubrrh = ubrrh;
   _ubrrl = ubrrl;
   _ucsra = ucsra;
@@ -185,7 +278,7 @@ HardwareSerial::HardwareSerial(ring_buffer *rx_buffer,
   _rxen = rxen;
   _txen = txen;
   _rxcie = rxcie;
-  _udre = udre;
+  _udrie = udrie;
   _u2x = u2x;
 }
 
@@ -220,18 +313,21 @@ void HardwareSerial::begin(long baud)
   sbi(*_ucsrb, _rxen);
   sbi(*_ucsrb, _txen);
   sbi(*_ucsrb, _rxcie);
+  cbi(*_ucsrb, _udrie); // XXX: what if there's already data in the tx buffer?
 }
 
+// XXX: should we empty the rx and tx buffers here?
 void HardwareSerial::end()
 {
   cbi(*_ucsrb, _rxen);
   cbi(*_ucsrb, _txen);
   cbi(*_ucsrb, _rxcie);  
+  cbi(*_ucsrb, _udrie);
 }
 
 int HardwareSerial::available(void)
 {
-  return (unsigned int)(RX_BUFFER_SIZE + _rx_buffer->head - _rx_buffer->tail) % RX_BUFFER_SIZE;
+  return (unsigned int)(SERIAL_BUFFER_SIZE + _rx_buffer->head - _rx_buffer->tail) % SERIAL_BUFFER_SIZE;
 }
 
 int HardwareSerial::peek(void)
@@ -250,7 +346,7 @@ int HardwareSerial::read(void)
     return -1;
   } else {
     unsigned char c = _rx_buffer->buffer[_rx_buffer->tail];
-    _rx_buffer->tail = (unsigned int)(_rx_buffer->tail + 1) % RX_BUFFER_SIZE;
+    _rx_buffer->tail = (unsigned int)(_rx_buffer->tail + 1) % SERIAL_BUFFER_SIZE;
     return c;
   }
 }
@@ -271,18 +367,30 @@ void HardwareSerial::flush()
 
 void HardwareSerial::write(uint8_t c)
 {
-  while (!((*_ucsra) & (1 << _udre)))
+  bool empty = (_tx_buffer->head == _tx_buffer->tail);
+  int i = (_tx_buffer->head + 1) % SERIAL_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)
     ;
-
-  *_udr = c;
+	
+  _tx_buffer->buffer[_tx_buffer->head] = c;
+  _tx_buffer->head = i;
+	
+  if (empty) {
+    // The buffer was empty, so enable interrupt on
+    // USART Data Register empty. The interrupt handler will take it from there
+    sbi(*_ucsrb, _udrie);
+  }
 }
 
 // Preinstantiate Objects //////////////////////////////////////////////////////
 
 #if defined(UBRRH) && defined(UBRRL)
-  HardwareSerial Serial(&rx_buffer, &UBRRH, &UBRRL, &UCSRA, &UCSRB, &UDR, RXEN, TXEN, RXCIE, UDRE, U2X);
+  HardwareSerial Serial(&rx_buffer, &tx_buffer, &UBRRH, &UBRRL, &UCSRA, &UCSRB, &UDR, RXEN, TXEN, RXCIE, UDRIE, U2X);
 #elif defined(UBRR0H) && defined(UBRR0L)
-  HardwareSerial Serial(&rx_buffer, &UBRR0H, &UBRR0L, &UCSR0A, &UCSR0B, &UDR0, RXEN0, TXEN0, RXCIE0, UDRE0, U2X0);
+  HardwareSerial Serial(&rx_buffer, &tx_buffer, &UBRR0H, &UBRR0L, &UCSR0A, &UCSR0B, &UDR0, RXEN0, TXEN0, RXCIE0, UDRIE0, U2X0);
 #elif defined(USBCON)
   #warning no serial port defined  (port 0)
 #else
@@ -290,13 +398,13 @@ void HardwareSerial::write(uint8_t c)
 #endif
 
 #if defined(UBRR1H)
-  HardwareSerial Serial1(&rx_buffer1, &UBRR1H, &UBRR1L, &UCSR1A, &UCSR1B, &UDR1, RXEN1, TXEN1, RXCIE1, UDRE1, U2X1);
+  HardwareSerial Serial1(&rx_buffer1, &tx_buffer1, &UBRR1H, &UBRR1L, &UCSR1A, &UCSR1B, &UDR1, RXEN1, TXEN1, RXCIE1, UDRIE1, U2X1);
 #endif
 #if defined(UBRR2H)
-  HardwareSerial Serial2(&rx_buffer2, &UBRR2H, &UBRR2L, &UCSR2A, &UCSR2B, &UDR2, RXEN2, TXEN2, RXCIE2, UDRE2, U2X2);
+  HardwareSerial Serial2(&rx_buffer2, &tx_buffer2, &UBRR2H, &UBRR2L, &UCSR2A, &UCSR2B, &UDR2, RXEN2, TXEN2, RXCIE2, UDRIE2, U2X2);
 #endif
 #if defined(UBRR3H)
-  HardwareSerial Serial3(&rx_buffer3, &UBRR3H, &UBRR3L, &UCSR3A, &UCSR3B, &UDR3, RXEN3, TXEN3, RXCIE3, UDRE3, U2X3);
+  HardwareSerial Serial3(&rx_buffer3, &tx_buffer3, &UBRR3H, &UBRR3L, &UCSR3A, &UCSR3B, &UDR3, RXEN3, TXEN3, RXCIE3, UDRIE3, U2X3);
 #endif
 
 #endif // whole file
diff --git a/cores/arduino/HardwareSerial.h b/cores/arduino/HardwareSerial.h
index 3efa775..d229373 100644
--- a/cores/arduino/HardwareSerial.h
+++ b/cores/arduino/HardwareSerial.h
@@ -32,6 +32,7 @@ class HardwareSerial : public Stream
 {
   private:
     ring_buffer *_rx_buffer;
+    ring_buffer *_tx_buffer;
     volatile uint8_t *_ubrrh;
     volatile uint8_t *_ubrrl;
     volatile uint8_t *_ucsra;
@@ -40,14 +41,14 @@ class HardwareSerial : public Stream
     uint8_t _rxen;
     uint8_t _txen;
     uint8_t _rxcie;
-    uint8_t _udre;
+    uint8_t _udrie;
     uint8_t _u2x;
   public:
-    HardwareSerial(ring_buffer *rx_buffer,
+    HardwareSerial(ring_buffer *rx_buffer, ring_buffer *tx_buffer,
       volatile uint8_t *ubrrh, volatile uint8_t *ubrrl,
       volatile uint8_t *ucsra, volatile uint8_t *ucsrb,
       volatile uint8_t *udr,
-      uint8_t rxen, uint8_t txen, uint8_t rxcie, uint8_t udre, uint8_t u2x);
+      uint8_t rxen, uint8_t txen, uint8_t rxcie, uint8_t udrie, uint8_t u2x);
     void begin(long);
     void end();
     virtual int available(void);