To make it clear: I’ve already solved the problem with my code. But I don’t know why the solution is actually working,… :~
I really don’t know what’s wrong with the first code… Maybe someone can teach me?
Sorry for posting “dirty” code fragments, but I wasn’t able to reproduce the problem nicely and the whole project is a bit to big for posting here. I’ve commented the changes in the working code (just two lines) to make them more visible.
This is the buggy code. The function is always returning “false”!
byte USBcom_class::GetCommand() {
byte n = 0; // originally "byte n;" (modified due to the information in the next posting)
while( Serial.available() && n < SERIALUSB_BUFFERSIZE ) {
char inChar = Serial.read();
if( inChar != CHR_SOH ){
Buffer[ n++ ] = inChar;
if( inChar == CHR_NUL ){
Serial.print( Buffer );
Serial.send_now();
return --n;
}
}
else{
n = 0; // originally "n == 0;" (modified due to the information in the next posting)
}
}
return false;
}
This is the 100% corretly working code. Just one additional and one changed line. But what’s the difference to the first one?!
byte USBcom_class::GetCommand() {
byte n = 0; // originally "byte n;" (modified due to the information in the next posting)
byte falseVar = 0; // <--- ########## Added this line! ##########
while( Serial.available() && n < SERIALUSB_BUFFERSIZE ) {
char inChar = Serial.read();
if( inChar != CHR_SOH ){
Buffer[ n++ ] = inChar;
if( inChar == CHR_NUL ){
Serial.print( Buffer );
Serial.send_now();
return --n;
}
}
else{
n = 0; // originally "n == 0;" (modified due to the information in the next posting)
}
}
return falseVar; // <--- ########## instead of "return false;" ##########
}
We would greatly prefer it if you posted your amended code in a new post, not amend your original one. Now that you corrected your first post, the comments under it (by other posters) don’t make any sense and are confusing.
Thanks AWOL! There are really helpful people around here!
But (referring to the topic) I’m still confused what’s the problem with the second code in my start posting… I realized, that it works when either using both “return” with a constant value or using both with a variable. But when using one variable and one constant (no matter in what order) I’ll alway get zero returned.
I’ve tried to reproduce with simpler code, but the following works exactly as expected (I get “abc” every two seconds):
[quote author=Nick Gammon link=topic=262541.msg1852200#msg1852200 date=1408748231]We would greatly prefer it if you posted your amended code in a new post, not amend your original one. Now that you corrected your first post, the comments under it (by other posters) don’t make any sense and are confusing.[/quote]I’ll keep that in mind for the future! (Different board, different rules…) I’ll add a comment, that it has been edited.
[quote author=Nick Gammon link=topic=262541.msg1852205#msg1852205 date=1408748483]As to your problem, it seems likely to me that you are falling through and executing the "return false" because serial communication is slow.
PaulS:
Do you KNOW what return does? If ENDS the function. Having two return statements makes no sense. The second one is never executed.
Yes, I know what return does. I know (e.g.) about stack pushing and the difference between c and std calls and so on, but i don't know how the parser exactly works. This was merely a "parser test". A test if it gets confused when using constant and variable values with return. The code itself isn't making any sense. Guess I should have pointed that out more clearly...
To my knowledge in general: I'm a quite experienced programmer (although not ingenious and not professional), but I'm a newbie in C/C++ and µCs.
PaulS:
Do you KNOW what return does? If ENDS the function. Having two return statements makes no sense. The second one is never executed.
There are lots of reasons to have multiple return statements in a function. It might be an error return, or two "forks" of processing . It is one of the easy ways to avoid gotos. 8^)
Now multiple entry points - which almost made it into C with the original "entry" reserved keyword, I am not so sure about.
Brduino:
Looks very promising! I’ll post a commentary after studying and testing.
Imagine for a moment that there is one character available on the serial port (which is very likely). Your code will go through the “while” loop once, and then fall through and return false.
If you want to get a whole line (and block whilst doing it) here is an example of doing that:
void getline (char * buf, size_t bufsize)
{
byte i;
// discard any old junk
while (Serial.available ())
Serial.read ();
for (i = 0; i < bufsize - 1; )
{
if (Serial.available ())
{
int c = Serial.read ();
if (c == '\n') // newline terminates
break;
if (!isspace (c)) // ignore spaces, carriage-return etc.
buf [i++] = c;
} // end if available
} // end of for
buf [i] = 0; // terminator
Serial.println (buf); // echo what they typed
} // end of getline
That particular code ignores spaces which may or may not suit you.
It loops until a newline which is the “line terminator” in this case. You pass down as an argument where the data is to go (the buffer) and its size.
You may or may not want to discard any existing input (the “discard any old junk” part).
There are lots of reasons to have multiple return statements in a function. It might be an error return, or two "forks" of processing . It is one of the easy ways to avoid gotos. 8^)
Can you cite a single example where two unconditional return statements in a row are ever useful?
OK! Everything's clear now! I've changed "byte n;" to "byte n = 0;" in both codes (the ones in my start posting) here in the forum, but only changed it in one source code (the already working one) here on my PC. facepalm
Long story short: It really was the uninitialized variable which was causing the trouble!
As so often, the bug was sitting in front of the computer.
Thanks to all for the nice and quick help!!
PaulS:
Parser? What are you talking about?
The C parser. Sorry... one letter was missing to make it understandable... It could have been that using a return with a constant and a return with a variable leads to complining wrong somehow... or so... or not... maybe...
Like said before, I'm not very familiar with C/C++.
@ Nick Gammon: Your "trap door collection" is really nice! Although most seem obvious when reading through I already managed to fall into two of them (uninitalized local variable and mixing up "==" and "=").
Not at all. I use that code in my hex code uploader which sits waiting for a file name. That’s been working for ages now.
for (i = 0; i < bufsize - 1; )
{
if (Serial.available ())
{
int c = Serial.read ();
if (c == '\n') // newline terminates
break;
if (!isspace (c)) // ignore spaces, carriage-return etc.
buf [i++] = c;
} // end if available
} // end of for
The main loop is waiting for the buffer to fill (until bufsize is reached). If there is no data it just loops. The “break” is when a newline is received. So it exits on a newline, or buffer full.
I'd use a while statement, instead of a for loop. One expects a for loop to iterate a fixed number of times, and to increment automatically. One expects a while loop to keep iterating until the end condition is met.
While a for loop can be made to work like a while loop, and a while statement can be made to work like a for loop, the point of coding is to use the best statement possible. That way, later on, one does not have to study the code to pick of the for loop nature of the while statement, or the while nature of the for loop.
