Go Down

Topic: need advice on how to be more effecient with Strings (Read 435 times) previous topic - next topic

fbriggs4

I am using IDE 1.0 and trying to develop a sketch to work with a GPS. I am aware of TinyGPS library as well as the example on playground. I decided I wanted to write my own sketch in order to learn as well as create some additional functionality. The sketch I created works great. However, as I started to add more and more features I started having problems with the Arduino resetting which I am pretty sure is related to memory. So I have decided to start from scratch and try to create a sketch that uses the memory more effectively. I used a LOT of Strings and everything I am reading now says DON'T use Strings. You live and learn I guess..... On top of using inefficient Strings I am changing them to chars and then to int's which really bloats things. With that said what is the most effeceint way? To give you an idea of my existing work here is some code. I stripped out a lot of the extra stuff but this shows you how I am grabbing the serial data and parsing it. The full version is over 14,000 bytes and I had quite a bit more stuff that I wanted to add. Thanks in advance for any suggestions on how to create more effective code.

Code: [Select]
#include <MemoryFree.h>

#include <SoftwareSerial.h> //used for the GPS port 5 which is GPS TX
SoftwareSerial mySerial(5, 6); // RX, TX
#include <EEPROM.h>          //stores the duty cycle when powered off
#include <eepromanything.h>  //stores the duty cycle when powered off
#include <avr/sleep.h> //Needed for sleep_mode
//#include <avr/power.h> //Needed for powering down perihperals such as the ADC/TWI and Timers


volatile int seconds = 0; //the incrementing counter used to keep time
volatile int timer = 0;  //timer increments for serial delay so it waits for new data
//volatile int battery = 0; //battery delay if voltage is below certain threshold

//The very important 32.686kHz interrupt handler
//increments the variables based on the 32.768 timer2 external crystal
SIGNAL(TIMER2_OVF_vect){
  seconds++;
  timer++;

}

int test = 1; //dummy variable for testing

//varibales for the GPS
char inByte; //gets one byte of serial data
String serialData; //string of serial data
String parsedData; //copy of serial data for parsing
int commaPosition; //gets the comma position in a strng so I can get the substrings
int i = 0; //global incrementing variable
String latitude;
String vertical;
String longitude;
char longitudeC[10];
float longitudeF;
String horizontal;
int TTF;
String time;
String date;
String knots;
String HDOP;
char HDOPC[4];
int HDOPI;
String VDOP;
String heading;
String alt; //altitude
String PDOP; //dilution of precision
String sats; //number of satellites in the solution
char satsC;
int satsI; //integer with number of satellites for qualifying the lock
boolean threeD = false; //check for a 3D fix quality.
String fixType; //2D 3D etc
char fixTypeC[2];
int fixTypeI;
String GPGGAS;
String GPGSAS;
String GPRMCS;
int increment = 0; //used to increment how many times it has been through the serial loop so I can authenticate the GPRMC data as being first.
int start; //used as a static point to compare against the increment variable.






void setup(){

  //start the serial port
  Serial.begin(9600); //hardware UART for debug and Telit
  mySerial.begin(4800); //soft serial GPS port on pin 5
 
  //turn on the transistor
  pinMode(A2, OUTPUT);
  digitalWrite(A2, LOW); // needs to be LOW to be on, HIGH is off
  delay(1000); //delay to let the GPS power up fully

  //toggle the on/off pin on the GPS to tuen it on
  pinMode(9, OUTPUT);
  digitalWrite(9, HIGH); // needs to be LOW to be on, HIGH is off
  delay(20);
  pinMode(9, OUTPUT);
  digitalWrite(9, LOW); // needs to be LOW to be on, HIGH is off


  sei(); //Enable global interrupts

}

void loop(){

  GPS_DATA();

}



void GPS_DATA(){
  timer = 0;
  increment = 0;;
  start = 0;
  while(timer < 500){
    if(mySerial.available()){
      inByte = mySerial.read();
      serialData += inByte;
      //looks for CR and then checks the serial data
      if(inByte == 13){
        Serial.print("freeMemory()=");
        Serial.println(freeMemory());
        Serial.println("got a 13 and checked the data");
        increment++;
        if(serialData.startsWith("$GPRMC", 1)){
          start = increment; //create a start point to comapre against to see ifthe data came in one block
          GPRMCS = serialData;
         
          Serial.println("data starts with GPRMC");
          GPRMCS = serialData;
        }
        if(serialData.startsWith("$GPGGA", 1) && increment == start +1){
          GPGGAS = serialData;
          Serial.println(start);
          Serial.println(increment);
   
        }
        if(serialData.startsWith("$GPGSA", 1) && increment == start +2){
          GPGSAS = serialData;
          GPRMC(); //parse GPRMC data
          GPGGA(); //parse GPGGA data
          GPGSA(); //pasre GPGSA data
        }
       
        i = 0; //resets the counter used in the parsing
        serialData = "";
      }

    }
  }
}



void GPRMC(){

  Serial.println("this is GPRMCS");
  Serial.println(GPRMCS);
  i = 0;
  while(i < 11){
    commaPosition = GPRMCS.indexOf(',');
    parsedData = GPRMCS.substring(0, commaPosition);
    Serial.println(GPRMCS.substring(0, commaPosition));
    GPRMCS = GPRMCS.substring(commaPosition+1);
    i++;
    if (i == 8){
      knots = parsedData;
    }
    if (i ==9){
      heading = parsedData;
    }
    if (i == 10){
      date = parsedData;
    }
  }
}



void GPGGA(){
  Serial.println("this is GPGGAS");
  Serial.println(GPGGAS);
  i = 0;
  while(i < 10){
    commaPosition = GPGGAS.indexOf(',');
    parsedData = GPGGAS.substring(0, commaPosition);
    Serial.println(GPGGAS.substring(0, commaPosition));
    GPGGAS = GPGGAS.substring(commaPosition+1);
    i++;
    if (i == 2){
      time = parsedData;
      Serial.print("This is time ");
      Serial.println(time);
    }
    if (i == 3){
      latitude = parsedData;
    }
    if (i == 4){
      vertical = parsedData;
    }
    if (i ==5){
      longitude = parsedData;
      longitude.toCharArray(longitudeC, 10);
      longitudeF = atof(longitudeC);
      Serial.print("this is longitude as a float ");
      Serial.println(longitudeF);
    }
    if (i ==6){
      horizontal = parsedData;
    }
   
    if (i == 8){
      sats = parsedData;
    }
    if (i ==9){
      HDOP = parsedData;
    }
    if (i ==10){
      alt = parsedData;
    }
   
  }
}


void GPGSA(){
  Serial.println("this is GPGSAS");
  Serial.println(GPGSAS);
  i = 0;
  while(i < 19){
    commaPosition = GPGSAS.indexOf(',');
    parsedData = GPGSAS.substring(0, commaPosition);
    Serial.println(GPGSAS.substring(0, commaPosition));
    GPGSAS = GPGSAS.substring(commaPosition+1);
    i++;
     if (i ==3){
      fixType = parsedData;
      fixType.toCharArray(fixTypeC, 2);
      fixTypeI = atoi(fixTypeC);
    }
    if (i == 16){
      PDOP = parsedData;
      Serial.print("This is PDOP ");
      Serial.println(PDOP);
    }
    if (i == 17){
      HDOP = parsedData; //HDOP string
      HDOP.toCharArray(HDOPC, 4); //HDOP char array
      HDOPI = atoi(HDOPC); //HDOP int that can be evaulated for lock quality

       
     
    }
    if (i == 18){
      VDOP = parsedData;
    }

  }
}










robtillaart


And often given advice is not to use strings but array's of char, terminated with a '\0' . IIRC the string class uses dynamic memory and there is a bug in the free() call so it eats up your memory.

Consider using PROGMEM if you have fixed strings.
Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

tuxduino

A quick suggestion, put F() around literal strings, so you put them in program memory instead of RAM.

robtillaart


if(serialData.startsWith("$GPGGA", 1)  ...

assuming using a char serialData[..]

if (strncmp(serialData, "$GPGGA", 5) ==0) ...

if(mySerial.available()){
      inByte = mySerial.read();
      serialData += inByte;


if(mySerial.available()){
      inByte = mySerial.read();
      serialData[idx] = inByte;
      idx++;
      serialData[idx] = '\0'; // keep it 0 terminated   idx is the index of the array, you need to do boundary checking ..
Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

fbriggs4

robtillaart, char arrays was going to be my next step but before I spent a bunch of time I wanted to make sure there was not a better way.

tuxduino, that is good advice and I will do that.

Thanks to both of you for the quick replies and good advice!

Go Up