Serial read memory leak

I need to have an arduino mega read a long string of chars from a serial input (coming from an arduino mini) and concatenate them into a php string. ( they are pressure readings ) The issue I am having is I keep running out of ram. I have a bit of code to tell me the remaining ram and it dwindles whenever the mega is reading over serial. I think there is something i am missing for getting everything cleared but I cant seem to figure out what that thing is. Help a noob out XD

void serialEvent() {
  while (Serial.available()) {
    // get the new byte:
    char inChar = (char)Serial.read(); 
    // add it to the pressureString:
    if (inChar == '\n') {
      pressureIn = true;
      Serial.flush();
    } 
    else { 
      pressureString += inChar; 
    }
  }
}

Post all your code if you want any help. The Serial.flush is point less get rid.

Mark

It looks like you are using Strings. Have you considered using C style strings instead ?
Posting your whole program instead of a snippet would help. Have you read Read this before posting a programming question ... - Programming Questions - Arduino Forum ?

okay here you go

/********************************
 ** Enviro Data Puller v.04 **
 ********************************/
#include <Ethernet.h>
#include <SPI.h>
#include <DHT.h>

#define DHTPININ 31     // what pin we're connected to
#define DHTPINOUT 30     // what pin we're connected to
#define DHTTYPE DHT11   // DHT 11 
DHT dht(DHTPININ, DHTTYPE);
DHT dht2(DHTPINOUT, DHTTYPE);

const int thumpPin = 32;
//----Outputs------
const int lg = 22;
const int ly = 23;
const int lr = 24;
const int LCDPin = 25;

byte mac[] = {
  0x90, 0xA2, 0xDA, 0x0D, 0xE0, 0x7A}; //Must be changed with each unit to match number on back
IPAddress ip(192,168,0,246);//Needed until we get DHCP working flawlessly
EthernetClient client;
int secMulti;
int p[4][4] = {
};
char pin[4] = {
  A3,A2,A1,A0};
int thumpCounter = 0;
int multiplier = 0;
boolean pressureIn = false;
boolean stringFlag = false;
boolean scaleTempFlag = false;
boolean scaleTempToString = false;
//----upload strings----
String tempData = "http://192.168.0.6/production/iCoat/EnviroInput.php?";
String pressureString = "";

boolean thumping = false;
long lastThump = millis();
long pastTimer;

void setup()
{
  cli();
  Serial.begin(9600);
  Serial1.begin(9600);
  Serial2.begin(9600);
  Serial3.begin(9600);
  Ethernet.begin(mac, ip);
  dht.begin();
  dht2.begin();
  pinMode(LCDPin,OUTPUT);
  pinMode(thumpPin,INPUT);
  digitalWrite(LCDPin,HIGH);
  digitalWrite(thumpPin,LOW);
  timer4();
  timer5();
  sei();
}

ISR(TIMER4_COMPA_vect) {
  unsigned long timer = millis();
  if (timer - pastTimer > 10){
    pastTimer = timer;
    int thumpState = digitalRead(thumpPin);
    if (thumpState == HIGH && thumping == false) {
      digitalWrite(LCDPin, LOW);
      lastThump = millis();
      thumping = true;
      thumpCounter++;
    }
    else if(thumpState == LOW)
    {
      digitalWrite(LCDPin, HIGH);
      thumping = false;
      if (millis() - lastThump > 120000)
      {
        //softReset();
      }
    }
  }

}

ISR(TIMER5_COMPA_vect) { // this event happens once every 1 seconds
  multiplier++;
  if (multiplier == 15){ //once every 15 sec
    stringFlag = true;
    //pressure();
    scaleTempFlag = true;
    multiplier = 0;
  }
}

void loop()
{
  if (scaleTempFlag == true && scaleTempToString == false){
    scales();
    float humIn = dht.readHumidity();
    float tempIn = dht.readTemperature();
    float humOut = dht2.readHumidity();
    float tempOut = dht2.readTemperature();

    char temp1[13] = "&TempIn=";
    dtostrf(tempIn,5,2,&temp1[8]);
    tempData += String(temp1);

    char hum1[12] = "&HumIn=";
    dtostrf(humIn,5,2,&hum1[7]);
    tempData += String(hum1);

    char temp2[14] = "&TempOut=";
    dtostrf(tempOut,5,2,&temp2[9]);
    tempData += String(temp2);

    char hum2[13] = "&HumOut=";
    dtostrf(humOut,5,2,&hum2[8]);
    tempData += String(hum2);
    scaleTempFlag = false;
    scaleTempToString = true; 
  }
  if(stringFlag == true)
  {
    tempData +="&mem=";
    tempData += freeRam();
    tempData += "&Thump=";
    tempData += thumpCounter;
    if(pressureIn == true){
      tempData += pressureString;
      pressureString = "";
      pressureIn = false;
    }
    Serial.println(tempData);//turn this into a send data command
    //Serial.println(millis());
    sendData(tempData);
    tempData = "http://192.168.0.6/production/iCoat/EnviroInput.php?";
    stringFlag = false;
    scaleTempToString = false;
  }
}

void serialEvent() {
  while (Serial.available()) {
    // get the new byte:
    char inChar = (char)Serial.read(); 
    // add it to the pressureString:
    if (inChar == '\n') {
      pressureIn = true;
      Serial.flush();
    } 
    else { 
      pressureString += inChar; 
    }
  }
}

int freeRam () {
  extern int __heap_start, *__brkval; 
  int v; 
  return (int) &v - (__brkval == 0 ? (int) &__heap_start : (int) __brkval); 
}

All i have to do to get this to run stable is comment out the serialEvent() bit and it works great

Adding chars to a String is very inefficient and would cause RAM fragmentation I think. After a few iterations of your (as yet unseen) program that could be the problem. Much better to add chars using a pointer or index into an array.

Also I there's hardly ever a reason to call flush(), and I don't think this is one of them.

Please post all the code for a more informed response. Posts crossed.


Rob

Please post all the code for a more informed response.

All dem codes is posted above. I could take it into an array but if i did i would still need to get that array into a string for sending via php anyhow right? would it prevent the leak to do it after the serial read?

 char hum1[12] = "&HumIn=";
    dtostrf(humIn,5,2,&hum1[7]);
    tempData += String(hum1);

Why not simply do the "dtostrf" into a simple buffer, and then "strcat" the constant string?
This way looks very error-prone and hard to maintain.
(I don't like Strings either)

    char tempBuf [20];
    strcat (tempData,  "&HumIn=");
    dtostrf(humIn,5,2,tempBuf]);
    strcat (tempData, tempBuf);

Your running around loop adding data to tempData but having got to the end of loop you never clear it so tempData just grows and grows and grows and ........

The sei/cli calls are not needed.

the ISR you have but don't use, wont work as they have parameters.

Mark

Why not simply do the "dtostrf" into a simple buffer, and then "strcat" the constant string?
This way looks very error-prone and hard to maintain.
(I don't like Strings either)
Code:

char tempBuf [20];
strcat (tempData, "&HumIn=");
dtostrf(humIn,5,2,tempBuf]);
strcat (tempData, tempBuf);

because im a noob and didnt know you could do that. I will likely be updating that post haste.

you never clear it so tempData just grows and grows

I will now say "nu uh!"
this line should clear it out to its default php line right?

tempData = "http://192.168.0.6/production/iCoat/EnviroInput.php?";

besides that you can watch what temp data contians via the serial monitor so its not growing where i can see.

this line should clear it out to its default php line right?

Wrong.
It would, if were in the main loop, but isn't, so it doesn't.

It would, if were in the main loop, but isn't, so it doesn't.

It can, because it is in the main loop, so it does. (it is also in the setup.)

lets get back on topic. with the simple commenting out of the Serial Event the rest of the code works just fine and leak free. The issue is defiantly contained withing the serial read or something about the way i am going about it.

OK, missed the one in loop (didn't really expect to see the same code multiple times)

I'm still with the "Let's abolish Strings" camp.

The issue is defiantly contained withing the serial read

No.

or something about the way i am going about it.

Exactly. As has been pointed out several times already.

Leave the serialEvent() call uncommented, but comment out

    else { 
      pressureString += inChar; 
    }

See if that makes the problem go away.

if this is the culprit

else { 
      pressureString += inChar; 
    }

how could I go about dealing with the Serial read and still get the same result? (aka getting all the pressure readings into a php string)

aka getting all the pressure readings into a php string

Well, frankly, you can't have a PHP string on the Arduino. PHP doesn't run on the Arduino.

How big is your string? How many characters? Using NULL terminated arrays of chars is something people have been doing for decades before the String abomination came along.

PHP doesn't run on the Arduino

Its not intended to. I need all of this into a string so i can use the Ethernet shield to send that string to an SQL server :slight_smile:

I used a string (pressureString) to store the information for a temporary sake then put it into the tempData

the max length of the string is about 1300 chars or so. It fits nicely into the megas 8k of ram. (if i have no memory leaks.)

I only used a second arduino to do the pressure readings in the beginning because i was unaware of timers and did not believe that i could run the pressure sensors with the rest of the project on a single board and have enough processing power to handle it all. I may just rework the code to run that way and i get the feeling alot of my issues will diminish. This will allow me to have a much better control of the pressure inputs.

the max length of the string is about 1300 chars or so. It fits nicely into the megas 8k of ram. (if i have no memory leaks.)

With a String that size that you’re extending a character at a time, you won’t need a leak to cause trouble. Heap fragmentation could be more than enough.

I have a bit of code to tell me the remaining ram and it dwindles whenever the mega is reading over serial.

I would use some other approach than repeatedly appending to the tempData. As this appears to be client code, the "http://" may be unnecessary and might actually cause issues.

tempData = "http://192.168.0.6/production/iCoat/EnviroInput.php?";
tempData += String(hum1);