Serial.readBytesUntil() does not work with Termination values > 127

Using Arduino Programming Questions
#1

Let's say I have the following 8 bytes in the Serial input buffer:

0x7A 0x7B 0x7C 0x7D 0x7E 0x7F 0x80 0x81

Let's examine the results of different read operations, [u]each operated on the same buffer contents[/u] as shown above.

int bytesRead = Serial.readBytesUntil(0x7E, readBytes, 8); // bytesRead == 4. First 4 bytes are read into readBytes (0x7A, 0x7B, 0x7C, 0x7D)

int bytesRead = Serial.readBytesUntil(0x7F, readBytes, 8); // bytesRead == 5. First 5 bytes are read into readBytes (0x7A, 0x7B, 0x7C, 0x7D, 0x7E)

int bytesRead = Serial.readBytesUntil(0x80, readBytes, 8); // bytesRead == 8. Yes 8, and not 6 as one might expect! All the bytes are read (0x7A, 0x7B, 0x7C, 0x7D, 0x7E, 0x7F, 0x80, 0x81)

Why? It appears that the function readBytesUntil() does not recognize a termination character if b7 is set in that value. This is unfortunate. For example, imagine a protocol where packets are terminated with a value >= 0x80. The function readBytesUntil() can not be used with such a protocol, forcing the programmer to do a great deal of extra programming.

Does anyone else see this as an issue that needs to be addressed?

#2

While I haven’t worked through that test in detail, I suspect that at some point you’re getting sign extension which is causing two values which you expect to be the same, to not compare as equal. If you pass in explicitly typed values to Serial.readBytesUntil using the types defined in the method signature, do you get the same problem?

#3

Looks like a bug to me. The problem is here...

https://github.com/arduino/Arduino/blob/master/hardware/arduino/cores/arduino/Stream.cpp#L240

c is either -1 (indicating a time-out) or a value less than 0x0100 (indicating a valid character). In your example, terminator is sign extended to 0xFF80 before the comparison so it can never match c.

#4

Obvious bug in Stream.cpp - the terminator char argument is cast to int before comparing to the result of timedRead() - the result of the read should be cast to char before the comparison.

    if (c < 0 || c == terminator) break;

should be:

    if (c < 0 || ((char)c) == terminator) break;

Or the clearer:

    if (c == -1 || ((char)c) == terminator) break;

since -1 is the timeout marker returned by timedRead

#5

Thank you for the info.

I did not realize I had the code to fix the problem myself. But, at least people will be aware of the issue now.

#6

sarallo: But, at least people will be aware of the issue now.

If you'd like this problem fixed in a future Arduino release, please create an issue here... http://code.google.com/p/arduino/issues/list

#7