moving serial read code into a function doesn't give the expected results

I have some code that works great for reading and parsing the mass from a balance when in the void loop() section but I tried moving it to a function to clean things up a bit and now it is giving strange results. any hekp would be appreciated

In the FUNCTION:

void setup()
{
  Serial1.begin(2400); // Initialize serial port to send and receive at 2400 baud
  Serial.begin(9600);  
} 

void loop()
{Serial.println(getMass());
delay(100);
}
 long getMass(){
  long value=0;
  //long massNow=0;
  if(Serial1.available())//gather mass on balance into massNow
    {
    char ch = Serial1.read();
    if(ch >= '0' && ch <= '9') // is this an ascii digit between 0 and 9?
      {
       value = (value * 10) + (ch - '0'); // yes, accumulate the value
      }
    else if (ch == 10)  // is the character the newline character
      {
       //massNow = value;  // set blinkrate to the accumulated value
       //Serial.println(massNow);
       //value = 0; // reset val to 0 ready for the next sequence of digits
       Serial1.flush();
       return value;
      }   
    }
}

function output:
....
552640984
1394813644
552645636
1394818300
552650280
1394822948
552654940
1394827596
552659580
1394832248
552664232
1394836896
552603344
1394841548
552608004
1394846200
....

in the Loop:

long massNow=0;
long value = 0;

void setup()
{
  Serial1.begin(2400); // Initialize serial port to send and receive at 2400 baud
  Serial.begin(9600);  
} 

void loop()
{  if(Serial1.available())//gather mass on balance into massNow
    {
    char ch = Serial1.read();
    if(ch >= '0' && ch <= '9') // is this an ascii digit between 0 and 9?
      {
       value = (value * 10) + (ch - '0'); // yes, accumulate the value
      }
    else if (ch == 10)  // is the character the newline character
      {
       massNow = value;  // set blinkrate to the accumulated value
       Serial.println(massNow);
       value = 0; // reset val to 0 ready for the next sequence of digits
       }   
    }
}

output is "7766" repeating as expected and what is displayed on balance.

Most of the times it is called, your function isn't returning a value, so randomness is probably to be expected.

thanks,

I changed the if Serial.available to while serial.available and seems to be doing what I need.

can you elaborate on how to implement this. I think I see how the "if serial.available" is not really what I need but I dont know enough about serial connection yet to understand how to have it grab what I need.

thanks

It depends on how you want to use this.
I'd probably want my code to be non-blocking, so the "delay" has to go, and I can't see the point of the "flush" so that may as well go too.
If you're going to call the function every time through "loop", but only want to print when you've got a new value, then you need to signal somehow that reception isn't complete and that you should not print.
You could do this by having the function return status rather than a value, and pass the return value by reference, or by pointer.

Or you could use the Serial event mechanism, and pass the value via a global.

Lots of ways of skinning this feline.

The thing you have to think about is what happens when you call that function and serial is not available. What should it return then? Maybe a -1 would be appropriate since you know mass won't be negative.

Also think about what if the mass is 234 but only the 2 and 3 have arrived when you make the call. In this case your current code doesn't return anything either since the return statement is only found in the if block that checks for a newline character.

And since you aren't sending anything via Serial, the flush call is worthless. Read carefully on what that does and see if you still think you need it.

Thanks guys!
yes the flush() is useless I was trying a few things to try and get things working as 5pm rolled around; same with the delay. The serial.println was just debugging to see what was going on.

What i am trying accomplish is read a weight from a scale as a motor is turning an auger and adding mass to the scale. As the mass approaches the desired weight, slow down the auger and stop when desired weight is reached. The balance will continually output the mass via rs232 in what seems like every half second, ending with a newline. So I envision continually updatign a variable with that data and controlling a if statement based on the weight variable.

The original code, before I tried to put it in a function, seemed to be outputting just what I needed, so I am a bit confused why it would matter once I put it in a function call. BTW this is my first Arduino project, so I am learning a lot here.

I think the serial.event() function may be a better method and will try playing with that tomorrow. Thanks for the lead.

I think the serial.event() function may be a better method

No magic bullet there. The serialEvent() method is called on every pass through loop(), if there is serial data to be read. There is nothing about that call that can't be accomplished in your existing function.