You are most likely reading the data out faster than it can come in, so Serial.available is false and the loop exits quickly.
int serialRead(char *text)
{
int i = 0;
while( i < 16)
{
while( !Serial.available() )
{
delay(1);
}
text[i] = (char)Serial.read();
i++;
}
text[i] = '\0';
return i;
}
There's probably a more efficient way to write this, but I was in a hurry. Also, you should have some kind of timeout mechanism in case the serial output stops for some reason.
Without seeing how you are calling this, it is hard to make a good suggestion about how to do it better.
Your function could… return zero every time you call it until such time as it has finished taking in data from the serial port, either because it counts to 16 or sees a carriage return or other delimiter, or whatever.
Only then would the function return non-zero, the number of characters waiting in the array where you have been parking them.
Ok, I see it now, I end up setting 5ms, because 1 millisecond still resulted in broken strings:
int serialRead(char *text)
{
int i = 0;
for (; i < 15 && Serial.available() > 0; i++) {
text[i] = (char)Serial.read();
delay(5); // Waits for next byte to fill the buffer. 1 baud = 1 byte per second.
}
text[i] = '\0';
return i;
}
You probably used Serial.begin(9600); in your setup, but we'd never know because you didn't share that.
With that setting, incoming characters arrive about every 1.3 milliseconds, presuming the sender is able to transmit seamlessly. Serial Monitor does do so, if you type your string and then hit enter.
There's no need to use 9600. Set your SerialMonitor to 115200, and put that number into the begin statement, and you'd likely not have to use delay() at all.
HOWEVER, that doesn't address the fact that your code may compile and work at that higher speed, but was still poor code. That, the others have addressed, though you seem to have ignored their advice.
Ok, about the delimiter, and because I just want the text and not the new line char, here is the adjusted version to also take into account your remarks:
int serialRead(char *text)
{
int i = 0;
for (; i < 15 && Serial.available() > 0; i++) {
text[i] = (char)Serial.read();
if (text[i] == '\n' || text[i] == '\t' || text[i] == '\r')
break;
delay(5); // Waits for next byte to fill the buffer. 1 baud = 1 byte per second.
}
text[i] = '\0';
return i;
}
I end up with a version that is slightly better than the one suggested, because I need to split the input into words so just one delimiter is not enough. This way the alternative bellow can process multiple end chars as the end char and not just one like the one suggested. so, here it is:
size_t readSerialUntil(const char *characters, char *buffer, size_t length)
{
size_t i = 0;
if (Serial.available() > 0)
{
unsigned long last_read_millis = millis();
do
{
if (Serial.available() > 0)
{
buffer[i] = (char)Serial.read();
if (strchr(characters, buffer[i]))
break;
last_read_millis = millis();
i++;
}
} while (millis() - last_read_millis < 3 && i < length - 1);
}
buffer[i] = '\0';
return i;
}
And in my particular case I call it like so: readSerialUntil("\n\t\r \0", serial_read, 16);
Not trying to nitpick, but the variable last_read_millis doesn't even need to be initialized due to the loop being a do while one, so, that line can just be: