Problem with Serial read w/Mega1284 & Megacore

Hey guys,

I'm moving some code I had working on the 328 over to the 1284 using MegaCore (need larger Flash space to expand), and while most things are working, I have two parts that are not. The one part is a Serial input read for date entry for changing the DS3231 that was working just fine, but now just acts like I entered nothing in the terminal (enter 22, press enter, its like nothing was entered) and won't proceed to the next readaByte() statement for further date inputs.. Does anyone know what the problem is? I've ripped my brains out trying to figure out what is wrong. Changing the syntax over to brackets makes no difference either just incase it was a syntax issue with the compiler.

Code:

byte readaByte() {
  while (!Serial.available()) delay(10);
  byte reading = 0;
  byte incomingByte = Serial.read();
  while (incomingByte != '\n') {
    if (incomingByte >= '0' && incomingByte <= '9')
      reading = reading * 10 + (incomingByte - '0');
    else;
    incomingByte = Serial.read();
  }
  return reading;
}

It would be called like this (it's for entering dates for setting a DS3231 RTC):

  Serial.print(F("Hour (0-23): "));
  byte shour = readaByte();

and on about 5 times for the entire date string to be created from the different byte values.

if incomingByte is in the range '0' to '9' once you are in the while loop() you will never exit as incomingByte cannot change its value
try

  byte incomingByte = 0;
  while (( incomingByte = Serial.read()) != '\n') {
    if (incomingByte >= '0' && incomingByte <= '9')
      reading = reading * 10 + (incomingByte - '0');
  }

incomingByte is changing it's value inside the while loop. You might have missed the semicolon after "else" but I agree that using "else" that way is bad programming style.

You do know that Serial.read() always returns immediately but returns -1 if no bytes are in the buffer, don't you?
In your case incomingByte will be 255, so reading will be increased by 207 which probably wasn't intended.
As you don't provide complete code I assume the actual error is in that part of the code you're hiding from us.

I also agree... poor coding as it does nothing but confuse... why have an else at all? Compiler throws a warning if turned on.

Not sure about that, as reading only gets updated if incomingByte is between 0-9?

@azchrisf You sure you have CR or LF/CR as a line ending in the monitor?

This code works...

void setup()
{
  Serial.begin(115200);
}

void loop()
{
  Serial.println(readaByte());
}


byte readaByte() {
  while (!Serial.available()) delay(10);
  byte reading = 0;
  byte incomingByte = Serial.read();
  while (incomingByte != '\n') {
    if (incomingByte >= '0' && incomingByte <= '9')
      reading = reading * 10 + (incomingByte - '0');
    else;
    incomingByte = Serial.read();
  }
  return reading;
}

Yep, you're right, I was wrong.

I want to thank all of you for your input and solutions. Coding in C is alot more literal and sensitive to errors than VB.NET that I am used to.

@pylon you taught me something new - I was not aware it returns a -1 if no bytes are received. I'll keep that in mind.

Both @horace and @red_car's solutions worked.

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.