Move buffers into HardwareSerial

This removes the need for doing an extra pointer dereference on every
access to the buffers, shrinking the code by around 100 bytes.

The members for these buffers must be public for now, since the
interrupt handlers also need to access them. These can later be made
private again.

Furthermore, the struct ring_buffer was removed. This allows the all
head and tail pointers to be put into the HardwareSerial struct before
the actual buffers, so the pointers all end up in the first 32 bytes of
the struct that can be accessed using a single instruction (ldd).

References: arduino#947

Author: matthijskooijman

hardware/arduino/avr/cores/arduino/HardwareSerial.cpp

@@ -49,59 +49,17 @@
 #endif
 #endif
 
-// Define constants and variables for buffering incoming serial data.  We're
-// 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 SERIAL_BUFFER_SIZE 16
-#else
-  #define SERIAL_BUFFER_SIZE 64
-#endif
-
-struct ring_buffer
-{
-  unsigned char buffer[SERIAL_BUFFER_SIZE];
-  volatile uint8_t head;
-  volatile uint8_t tail;
-};
-
-#if SERIAL_BUFFER_SIZE > 256
-#error Serial buffer size too big for head and tail pointers
-#endif
-
-#if defined(USBCON)
-  ring_buffer rx_buffer = { { 0 }, 0, 0};
-  ring_buffer tx_buffer = { { 0 }, 0, 0};
-#endif
-#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 *buffer)
+inline void store_char(unsigned char c, HardwareSerial *s)
 {
-  int i = (unsigned int)(buffer->head + 1) % SERIAL_BUFFER_SIZE;
+  int i = (unsigned int)(s->_rx_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 != buffer->tail) {
-    buffer->buffer[buffer->head] = c;
-    buffer->head = i;
+  if (i != s->_rx_buffer_tail) {
+    s->_rx_buffer[s->_rx_buffer_head] = c;
+    s->_rx_buffer_head = i;
   }
 }
 
@@ -126,7 +84,7 @@ inline void store_char(unsigned char c, ring_buffer *buffer)
   #if defined(UDR0)
     if (bit_is_clear(UCSR0A, UPE0)) {
       unsigned char c = UDR0;
-      store_char(c, &rx_buffer);
+      store_char(c, &Serial);
     } else {
       unsigned char c = UDR0;
     };
@@ -152,7 +110,7 @@ inline void store_char(unsigned char c, ring_buffer *buffer)
   {
     if (bit_is_clear(UCSR1A, UPE1)) {
       unsigned char c = UDR1;
-      store_char(c, &rx_buffer1);
+      store_char(c, &Serial1);
     } else {
       unsigned char c = UDR1;
     };
@@ -167,7 +125,7 @@ inline void store_char(unsigned char c, ring_buffer *buffer)
   {
     if (bit_is_clear(UCSR2A, UPE2)) {
       unsigned char c = UDR2;
-      store_char(c, &rx_buffer2);
+      store_char(c, &Serial2);
     } else {
       unsigned char c = UDR2;
     };
@@ -182,7 +140,7 @@ inline void store_char(unsigned char c, ring_buffer *buffer)
   {
     if (bit_is_clear(UCSR3A, UPE3)) {
       unsigned char c = UDR3;
-      store_char(c, &rx_buffer3);
+      store_char(c, &Serial3);
     } else {
       unsigned char c = UDR3;
     };
@@ -222,7 +180,7 @@ ISR(USART0_UDRE_vect)
 ISR(USART_UDRE_vect)
 #endif
 {
-  if (tx_buffer.head == tx_buffer.tail) {
+  if (Serial._tx_buffer_head == Serial._tx_buffer_tail) {
 	// Buffer empty, so disable interrupts
 #if defined(UCSR0B)
     cbi(UCSR0B, UDRIE0);
@@ -232,8 +190,8 @@ ISR(USART_UDRE_vect)
   }
   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;
+    unsigned char c = Serial._tx_buffer[Serial._tx_buffer_tail];
+    Serial._tx_buffer_tail = (Serial._tx_buffer_tail + 1) % SERIAL_BUFFER_SIZE;
 
   #if defined(UDR0)
     UDR0 = c;
@@ -250,14 +208,14 @@ ISR(USART_UDRE_vect)
 #ifdef USART1_UDRE_vect
 ISR(USART1_UDRE_vect)
 {
-  if (tx_buffer1.head == tx_buffer1.tail) {
+  if (Serial1._tx_buffer_head == Serial1._tx_buffer_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;
+    unsigned char c = Serial1._tx_buffer[Serial1._tx_buffer_tail];
+    Serial1._tx_buffer_tail = (Serial1._tx_buffer_tail + 1) % SERIAL_BUFFER_SIZE;
 
     UDR1 = c;
   }
@@ -267,14 +225,14 @@ ISR(USART1_UDRE_vect)
 #ifdef USART2_UDRE_vect
 ISR(USART2_UDRE_vect)
 {
-  if (tx_buffer2.head == tx_buffer2.tail) {
+  if (Serial2._tx_buffer_head == Serial2._tx_buffer_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;
+    unsigned char c = Serial2._tx_buffer[Serial2._tx_buffer_tail];
+    Serial2._tx_buffer_tail = (Serial2._tx_buffer_tail + 1) % SERIAL_BUFFER_SIZE;
 
     UDR2 = c;
   }
@@ -284,31 +242,30 @@ ISR(USART2_UDRE_vect)
 #ifdef USART3_UDRE_vect
 ISR(USART3_UDRE_vect)
 {
-  if (tx_buffer3.head == tx_buffer3.tail) {
+  if (Serial3._tx_buffer_head == Serial3._tx_buffer_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;
+    unsigned char c = Serial3._tx_buffer[Serial3._tx_buffer_tail];
+    Serial3._tx_buffer_tail = (Serial3._tx_buffer_tail + 1) % SERIAL_BUFFER_SIZE;
 
     UDR3 = c;
   }
 }
 #endif
 
-
 // Constructors ////////////////////////////////////////////////////////////////
 
-HardwareSerial::HardwareSerial(ring_buffer *rx_buffer, ring_buffer *tx_buffer,
+HardwareSerial::HardwareSerial(
   volatile uint8_t *ubrrh, volatile uint8_t *ubrrl,
   volatile uint8_t *ucsra, volatile uint8_t *ucsrb,
   volatile uint8_t *ucsrc, volatile uint8_t *udr,
   uint8_t rxen, uint8_t txen, uint8_t rxcie, uint8_t udrie, uint8_t u2x)
 {
-  _rx_buffer = rx_buffer;
-  _tx_buffer = tx_buffer;
+  _tx_buffer_head = _tx_buffer_tail = 0;
+  _rx_buffer_head = _rx_buffer_tail = 0;
   _ubrrh = ubrrh;
   _ubrrl = ubrrl;
   _ucsra = ucsra;
@@ -416,7 +373,7 @@ void HardwareSerial::begin(unsigned long baud, byte config)
 void HardwareSerial::end()
 {
   // wait for transmission of outgoing data
-  while (_tx_buffer->head != _tx_buffer->tail)
+  while (_tx_buffer_head != _tx_buffer_tail)
     ;
 
   cbi(*_ucsrb, _rxen);
@@ -425,31 +382,31 @@ void HardwareSerial::end()
   cbi(*_ucsrb, _udrie);
 
   // clear any received data
-  _rx_buffer->head = _rx_buffer->tail;
+  _rx_buffer_head = _rx_buffer_tail;
 }
 
 int HardwareSerial::available(void)
 {
-  return (unsigned int)(SERIAL_BUFFER_SIZE + _rx_buffer->head - _rx_buffer->tail) % SERIAL_BUFFER_SIZE;
+  return (unsigned int)(SERIAL_BUFFER_SIZE + _rx_buffer_head - _rx_buffer_tail) % SERIAL_BUFFER_SIZE;
 }
 
 int HardwareSerial::peek(void)
 {
-  if (_rx_buffer->head == _rx_buffer->tail) {
+  if (_rx_buffer_head == _rx_buffer_tail) {
     return -1;
   } else {
-    return _rx_buffer->buffer[_rx_buffer->tail];
+    return _rx_buffer[_rx_buffer_tail];
   }
 }
 
 int HardwareSerial::read(void)
 {
   // if the head isn't ahead of the tail, we don't have any characters
-  if (_rx_buffer->head == _rx_buffer->tail) {
+  if (_rx_buffer_head == _rx_buffer_tail) {
     return -1;
   } else {
-    unsigned char c = _rx_buffer->buffer[_rx_buffer->tail];
-    _rx_buffer->tail = (unsigned int)(_rx_buffer->tail + 1) % SERIAL_BUFFER_SIZE;
+    unsigned char c = _rx_buffer[_rx_buffer_tail];
+    _rx_buffer_tail = (unsigned int)(_rx_buffer_tail + 1) % SERIAL_BUFFER_SIZE;
     return c;
   }
 }
@@ -463,16 +420,16 @@ void HardwareSerial::flush()
 
 size_t HardwareSerial::write(uint8_t c)
 {
-  int i = (_tx_buffer->head + 1) % SERIAL_BUFFER_SIZE;
+  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
   // ???: return 0 here instead?
-  while (i == _tx_buffer->tail)
+  while (i == _tx_buffer_tail)
     ;
 
-  _tx_buffer->buffer[_tx_buffer->head] = c;
-  _tx_buffer->head = i;
+  _tx_buffer[_tx_buffer_head] = c;
+  _tx_buffer_head = i;
 
   sbi(*_ucsrb, _udrie);
   // clear the TXC bit -- "can be cleared by writing a one to its bit location"
@@ -489,23 +446,23 @@ HardwareSerial::operator bool() {
 // Preinstantiate Objects //////////////////////////////////////////////////////
 
 #if defined(UBRRH) && defined(UBRRL)
-  HardwareSerial Serial(&rx_buffer, &tx_buffer, &UBRRH, &UBRRL, &UCSRA, &UCSRB, &UCSRC, &UDR, RXEN, TXEN, RXCIE, UDRIE, U2X);
+  HardwareSerial Serial(&UBRRH, &UBRRL, &UCSRA, &UCSRB, &UCSRC, &UDR, RXEN, TXEN, RXCIE, UDRIE, U2X);
 #elif defined(UBRR0H) && defined(UBRR0L)
-  HardwareSerial Serial(&rx_buffer, &tx_buffer, &UBRR0H, &UBRR0L, &UCSR0A, &UCSR0B, &UCSR0C, &UDR0, RXEN0, TXEN0, RXCIE0, UDRIE0, U2X0);
+  HardwareSerial Serial(&UBRR0H, &UBRR0L, &UCSR0A, &UCSR0B, &UCSR0C, &UDR0, RXEN0, TXEN0, RXCIE0, UDRIE0, U2X0);
 #elif defined(USBCON)
   // do nothing - Serial object and buffers are initialized in CDC code
 #else
   #error no serial port defined  (port 0)
 #endif
 
 #if defined(UBRR1H)
-  HardwareSerial Serial1(&rx_buffer1, &tx_buffer1, &UBRR1H, &UBRR1L, &UCSR1A, &UCSR1B, &UCSR1C, &UDR1, RXEN1, TXEN1, RXCIE1, UDRIE1, U2X1);
+  HardwareSerial Serial1(&UBRR1H, &UBRR1L, &UCSR1A, &UCSR1B, &UCSR1C, &UDR1, RXEN1, TXEN1, RXCIE1, UDRIE1, U2X1);
 #endif
 #if defined(UBRR2H)
-  HardwareSerial Serial2(&rx_buffer2, &tx_buffer2, &UBRR2H, &UBRR2L, &UCSR2A, &UCSR2B, &UCSR2C, &UDR2, RXEN2, TXEN2, RXCIE2, UDRIE2, U2X2);
+  HardwareSerial Serial2(&UBRR2H, &UBRR2L, &UCSR2A, &UCSR2B, &UCSR2C, &UDR2, RXEN2, TXEN2, RXCIE2, UDRIE2, U2X2);
 #endif
 #if defined(UBRR3H)
-  HardwareSerial Serial3(&rx_buffer3, &tx_buffer3, &UBRR3H, &UBRR3L, &UCSR3A, &UCSR3B, &UCSR3C, &UDR3, RXEN3, TXEN3, RXCIE3, UDRIE3, U2X3);
+  HardwareSerial Serial3(&UBRR3H, &UBRR3L, &UCSR3A, &UCSR3B, &UCSR3C, &UDR3, RXEN3, TXEN3, RXCIE3, UDRIE3, U2X3);
 #endif
 
 #endif // whole file

hardware/arduino/avr/cores/arduino/HardwareSerial.h

@@ -27,13 +27,19 @@
 
 #include "Stream.h"
 
-struct ring_buffer;
+// Define constants and variables for buffering incoming serial data.  We're
+// 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 SERIAL_BUFFER_SIZE 16
+#else
+  #define SERIAL_BUFFER_SIZE 64
+#endif
 
 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;
@@ -46,8 +52,20 @@ class HardwareSerial : public Stream
     uint8_t _udrie;
     uint8_t _u2x;
     bool transmitting;
+
   public:
-    HardwareSerial(ring_buffer *rx_buffer, ring_buffer *tx_buffer,
+    volatile uint8_t _rx_buffer_head;
+    volatile uint8_t _rx_buffer_tail;
+    volatile uint8_t _tx_buffer_head;
+    volatile uint8_t _tx_buffer_tail;
+
+    // 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_BUFFER_SIZE];
+    unsigned char _tx_buffer[SERIAL_BUFFER_SIZE];
+
+    HardwareSerial(
       volatile uint8_t *ubrrh, volatile uint8_t *ubrrl,
       volatile uint8_t *ucsra, volatile uint8_t *ucsrb,
       volatile uint8_t *ucsrc, volatile uint8_t *udr,