move ringbuffer definition to header and allow protected access to Hardware serial for subclassing

[GoogleCodeExporter]

These changes are suggested to allow for subclassing and access to ringbuffer 
from subclass methods. 

This should to the best of my knowledge be a "Mostly harmless" change, but e.g. 
projects like (github) ErikZalm/Marlin could stop copying large parts of the 
Arduino libraries for modifications. 

--- HardwareSerial.h    Tue Jun  5 13:07:28 2012
+++ HardwareSerial_update.h     Tue Jun  5 13:23:29 2012
@@ -26,11 +26,26 @@

 #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
+
+struct ring_buffer
+{
+  unsigned char buffer[SERIAL_BUFFER_SIZE];
+  volatile unsigned int head;
+  volatile unsigned int tail;
+};

 class HardwareSerial : public Stream
 {
-  private:
+  protected:
     ring_buffer *_rx_buffer;
     ring_buffer *_tx_buffer;
     volatile uint8_t *_ubrrh;

$ diff HardwareSerial.cpp HardwareSerial_update.cpp  -u
--- HardwareSerial.cpp  Tue Jun  5 13:07:28 2012
+++ HardwareSerial_update.cpp   Tue Jun  5 13:23:15 2012
@@ -33,23 +33,6 @@

 #include "HardwareSerial.h"

-// 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 unsigned int head;
-  volatile unsigned int tail;
-};
-
 #if defined(USBCON)
   ring_buffer rx_buffer = { { 0 }, 0, 0};
   ring_buffer tx_buffer = { { 0 }, 0, 0};

Original issue reported on code.google.com by l...@soerup.org on 5 Jun 2012 at 11:32

[GoogleCodeExporter]

Can you say a bit more about what the subclasses of Serial do (and why they're 
important)?  That will provide the motivation for this change (which may be 
simple, but still needs to be tested, etc.).  

Original comment by dmel...@gmail.com on 5 Jun 2012 at 2:04

[GoogleCodeExporter]

The application is a 3D printer firmware and the author has implemented this 
function as the only addition to the standard HardwareSerial class 
(https://github.com/ErikZalm/Marlin/blob/Marlin_v1/Marlin/MarlinSerial.h) :

FORCE_INLINE void checkRx(void)
    {
      if((UCSR0A & (1<<RXC0)) != 0) {
        unsigned char c  =  UDR0;
        int i = (unsigned int)(_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 != _rx_buffer->tail) {
          _rx_buffer->buffer[_rx_buffer->head] = c;
          _rx_buffer->head = i;
        }
      }
    }

I'm not the original author, but has cloned this project to use a custom 
display (Adafruit), but run in the problems with the copied versions vs. the 
stock version being utilized by the Adafruit libraries (multiple definitions 
etc.), so in looking into the code this seemed like a reasonable safe way to 
remove a larger issue in having partial copies of stock libraries in a project. 

Original comment by l...@soerup.org on 5 Jun 2012 at 8:26

[GoogleCodeExporter]

Done. 

Look here for the list of relevant commits:

https://github.com/arduino/Arduino/issues/947

C

Original comment by c.mag...@arduino.cc on 30 Jul 2013 at 9:43

• Changed state: Fixed