make serial.available, peek, read atomic

when SERIAL_RX_BUFFER_SIZE is defined as greater than 256, the current HardwareSerial.cpp code almost works correctly. but not quite. the function serial.available may return an incorrect number of characters waiting (though when treated purely as a boolean it seems not to fault).serial.peek has a similar issue detecting if a character is waiting, while serial.read may also leave _rx_buffer_tail corrupt to an interrupt occurring mid-update (this corruption may cause the ISR to be mistaken about how much free is available in the Rx buffer).

the proposed changes to these three functions add cli/sti pairings around the critical pieces of code. i have tested and verified the change made to serial.available as correcting the problem, while the change to serial.peek follows the exact same pattern. the changes to serial.read have been confirmed as not breaking the function, but i have not been in a position to test the failure of the original (non-atomic) version to make comparisons.

multiple testing was conducted at 115,200 baud and 500,000 baud using data streams of 1,000,000 characters sent over a 2 minute interval. any error causing loss of character would have resulted in a catastrophic (ie, very obvious) failure.

cheers,
rob   :-)

-> Squash and rebase of arduino/Arduino#3848

Author: robert-rozee

cores/arduino/HardwareSerial.cpp

@@ -151,26 +151,62 @@ void HardwareSerial::end()
 
 int HardwareSerial::available(void)
 {
-  return ((unsigned int)(SERIAL_RX_BUFFER_SIZE + _rx_buffer_head - _rx_buffer_tail)) % SERIAL_RX_BUFFER_SIZE;
+#if (SERIAL_RX_BUFFER_SIZE>256)
+  uint8_t oldSREG = SREG;                                  // save interrupt flag
+  cli();                                                   // disable interrupts
+#endif
+  rx_buffer_index_t head = _rx_buffer_head;                // retrieve Rx head index
+  rx_buffer_index_t tail = _rx_buffer_tail;                // retrieve Rx tail index
+#if (SERIAL_RX_BUFFER_SIZE>256)
+  SREG = oldSREG;                                          // restore the interrupt flag
+#endif
+  return ((unsigned int)(SERIAL_RX_BUFFER_SIZE + head - tail)) % SERIAL_RX_BUFFER_SIZE;
 }
 
 int HardwareSerial::peek(void)
 {
-  if (_rx_buffer_head == _rx_buffer_tail) {
+#if (SERIAL_RX_BUFFER_SIZE>256)
+  uint8_t oldSREG = SREG;                                  // save interrupt flag
+  cli();                                                   // disable interrupts
+#endif
+  rx_buffer_index_t head = _rx_buffer_head;                // retrieve Rx head index
+  rx_buffer_index_t tail = _rx_buffer_tail;                // retrieve Rx tail index
+#if (SERIAL_RX_BUFFER_SIZE>256)
+  SREG = oldSREG;                                          // restore the interrupt flag
+#endif
+  if (head == tail) {
     return -1;
   } else {
-    return _rx_buffer[_rx_buffer_tail];
+    return _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 (SERIAL_RX_BUFFER_SIZE>256)
+  uint8_t oldSREG = SREG;                                  // save interrupt flag
+  cli();                                                   // disable interrupts
+#endif
+  rx_buffer_index_t head = _rx_buffer_head;                // retrieve Rx head index
+  rx_buffer_index_t tail = _rx_buffer_tail;                // retrieve Rx tail index
+#if (SERIAL_RX_BUFFER_SIZE>256)
+  SREG = oldSREG;                                          // restore the interrupt flag
+#endif
+
+  if (head == tail) {
     return -1;
   } else {
-    unsigned char c = _rx_buffer[_rx_buffer_tail];
-    _rx_buffer_tail = (rx_buffer_index_t)(_rx_buffer_tail + 1) % SERIAL_RX_BUFFER_SIZE;
+   unsigned char c = _rx_buffer[tail];
+
+#if (SERIAL_RX_BUFFER_SIZE>256)
+    uint8_t oldSREG = SREG;                                // save interrupt flag
+    cli();                                                 // disable interrupts
+#endif
+    _rx_buffer_tail = (tail + 1) % SERIAL_RX_BUFFER_SIZE;
+#if (SERIAL_RX_BUFFER_SIZE>256)
+    SREG = oldSREG;                                        // restore the interrupt flag
+#endif
     return c;
   }
 }