Suggestion for HardwareSerial.cpp - peek(), read()

[GoogleCodeExporter]

I suggest removing the default behavior of returning -1 in peek() & read(), as 
this may cause unexpected behavior (unexpected values / corruption), especially 
for beginning developers.

Regardless, to avoid this issue a call to available() must be made.
I suggest making those two function default to a blocking behavior to wait for 
more data to arrive. The user can then use available() beforehand to prevent 
waiting if they don't want to block, which they should be doing anyway.
I feel forcing a block is safer than possible corruption, if the user fails to 
call available() beforehand or fails to check the -1 return value.

The new functions would look like: (unified diff file also attached):

int HardwareSerial::peek(void)
{
  //If the head isn't ahead of the tail, wait for more characters.
  //This seems safer than returning. The user can call available() to avoid this
  while(_rx_buffer->head == _rx_buffer->tail);

  return _rx_buffer->buffer[_rx_buffer->tail];
}

int HardwareSerial::read(void)
{
  unsigned char c;

  //If the head isn't ahead of the tail, wait for more characters.
  //This seems safer than returning. The user can call available() to avoid this
  while(_rx_buffer->head == _rx_buffer->tail);

  //read the next character in the queue
  c = _rx_buffer->buffer[_rx_buffer->tail];
  _rx_buffer->tail = (unsigned int)(_rx_buffer->tail + 1) % SERIAL_BUFFER_SIZE;

  return c;
}

Original issue reported on code.google.com by blackwel...@gmail.com on 17 Aug 2012 at 11:47

Attachments:

• HardwareSerial.cpp.patch

[GoogleCodeExporter]

This is an interesting approach and does seem to offer more flexibility (i.e. 
you can get a blocking read() or, by using available() first, a non-blocking 
one). At this point, however, we've been using the other approach for so long 
that I don't think it makes sense to change now.

Original comment by dmel...@gmail.com on 28 Aug 2012 at 2:01

• Changed state: WontFix
• Added labels: Component-Core