search

arduino / ArduinoCore-samd

Arduino Core for SAMD21 CPU
GNU Lesser General Public License v2.1
468 stars 714 forks source link

Serial1.available() returns wrong value #139

Open NickWaterton opened 8 years ago

NickWaterton commented 8 years ago

In a tight timing loop (without delays, such as Serial.print()), Serial1.available() returns a lower value than is correct.

For example Serial1.readBytes(buf, Serial1.available())); does not read the whole buffer.

If you use int a=Serial.readBytes(buf, SERIAL_BUFFER_SIZE); then a will report correctly how many bytes were read, this is a different value than Serial1.available(); - usually Serial1.available(); is one byte less.

This leads to the bytes in buf being truncated.

If there is a slight delay in the loop, the problem goes away, so it is hard to debug.

GrumpyOldPizza commented 8 years ago

Not a bug.

Serial1.available() reports the available bytes at the point when you call the function. If you call Serial1.readBytes(buf. Serial1.available()), then you read only those bytes which had been available when Serial1.available() was called. Because of the asynchronous nature of the Uart, there may be more bytes available.

If on the other hand you use Serial.readBytes(buf, SERIAL_BUFFER_SIZE), then you block up to 1000ms waiting for SERIAL_BUFFER_SIZE bytes.

In any case this is not portable because SERIAL_BUFFER_SIZE may not apply to all Serial ports, especially not SerialUSB (which might buffer more).

NickWaterton commented 8 years ago

It doesn't block, if you set Serial1.setTimeout(0);

Also it only happens on Serial1, Serial returns 64 bytes on a full buffer (which I know could be more externally buffered).

I know it's not portable, it's a workaround for a bug. If Serial1.available() actually reported the number of bytes available in the buffer (instead of one less), I wouldn't do this.

Still sounds like a bug to me as Serial.available() always returns a value of 63 or less on a 64 byte buffer. I know there are 64 bytes in the buffer, but I can only read 63, unless more data enters the buffer, or I force it to read 64.

If you change the buffer size in RingBuffer.h, you still always get 1 less byte reported by Serial1.available().

This is at least inconsistent behaviour between Serial1 and Serial. More to the point, the last byte in the buffer is inaccessible, using Serial1.available(), I'm sure that is not how it is supposed to work.

PaulStoffregen commented 8 years ago

Still sounds like a bug to me as Serial.available() always returns a value of 63 or less on a 64 byte buffer.

The way the head/tail scheme works, 1 place in the buffer can't be used. That's a very small price to pay for the huge benefit of inherently atomic in accessing this way of managing the buffer.

But if you really feel the waste of 1 byte in the buffer is that important, perhaps try to invent a better way....

NickWaterton commented 8 years ago

I don't want to get into the pros and cons of ring buffers. My point is that it is inconsistent.

I have other boards using the Arduino IDE, they work fine. The same Sketch compiled on an M0 does not work because of this issue, and it took a while to figure out why.

its not the loss of 1 byte that's the problem, it's the fact that its different from every other implementation, and you don't know about it until you fall in the trap.

I can fix it myself, but that won't help other people running into the same problem, so I thought I'd report the issue.

GrumpyOldPizza commented 8 years ago

I might not be the expert here, but looking at the AVR source, it's using a ringbuffer with with 64 entries, where one is alway left empty (same as Zero). The SAM port is using a 128 entry ring buffer, with one empty. The 101 code is using a ringbuffer with 256 entries, with one empty. The ESP8266 is using the HW FIFO of the UART. The Energia port (LM4F / MSP432) using a 256 entry ringbuffer with no overwrite check.

So in reality all implementations are different, and the official documentation that neither specify the presence of SERIAL_BUFFER_SIZE (actually the original AVR code does have separate RX/TX sizes), no does it imply a specific behavior there, other than assuming some for of buffering takes place.

If by chance a read operation returns SERIAL_BUFFER_SIZE, or Serial.available() returns SERIAL_BUFFER_SIZE -1, then you pretty much overflowed the internal buffer and are about to lose (or already have lost) data.

Regarding the extra byte that is not used in the traditional ringbuffer scheme. There is a simple scheme to avoid that. The main trick is to cleverly use the index values (_iHead, _iTail) to distinguish between buffer emptry and buffer full. But at the end of the day it requires more code for a case that should not matter.

On Fri, Apr 29, 2016 at 4:17 PM, NickWaterton notifications@github.com wrote:

I don't want to get into the pros and cons of ring buffers. My point is that it is inconsistent.

I have other boards using the Arduino IDE, they work fine. The same Sketch compiled on an M0 does not work because of this issue, and it took a while to figure out why.

its not the loss of 1 byte that's the problem, it's the fact that its different from every other implementation, and you don't know about it until you fall in the trap.

I can fix it myself, but that won't help other people running into the same problem, so I thought I'd report the issue.

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/arduino/ArduinoCore-samd/issues/139#issuecomment-215896314