Why does order matter here?

Discussion in 'Mac Programming' started by ArtOfWarfare, Mar 25, 2014.

  1. ArtOfWarfare macrumors 604

    ArtOfWarfare

    Joined:
    Nov 26, 2007
    #1
    I'm writing some Arduino code here (it's basically just CPP). The issue is that when I call dgps.updategga() matters - if I call it before dips.Status(), that method fails, but if I call it afterwards everything works fine.

    The thing is, I want to remove the public facing updategga function and instead just have a single (public) update function which runs both update and updategga one after another. I didn't write the existing update or updategga function, so I'm not sure why the order they're run would matter. I'd love to hear what you guys think.

    Code:
    void loop() {
        dgps.update();    // Calling this updates the GPS data.  The data in dGPS variables stays the same unless
                                      // this function is called.  When this function is called, the data is updated.
      [b]//dgps.updategga(); // I can't place this here instead of later or things break. Why?[/b]
      if (!dgps.Status()) {
          Serial.println("GPS couldn't find enough valid satelites!");
      } else if (dgps.Checks() != dgps.Checked()) {
          ...
          [b]dgps.updategga(); // It works fine if I have it here instead.[/b]
          ...
      }
    }
    The implementations of the above dgps methods are:
    Code:
    bool dGPS::Status(){
        return (strcmp(cStatus, "A") == 0);
    }
    
    bool dGPS::update(){
     while(true){
      int byteGPS=-1;
      byteGPS=gpsSerial.read();      //read a byte from the serial port
      if (byteGPS == -1) {           // See if the port is empty yet
        delay(5); 
      } 
      else {
        linea[conta]=byteGPS;        // If there is serial port data, it is put in the buffer
        conta++;                      
        if (byteGPS==13){            // If the received byte is = to 13, end of transmission 
          cont=0;
          bien=0;
          for (int i=1;i<7;i++){     // Verifies if the received command starts with $GPR
            if (linea[i]==comandoGPR[i-1]){
              bien++;
            }
          }
          if(bien==6){               // If yes, continue and process the data
            Serial.print("");        // For some reason, this needs to be here or you won't populate variables.  If anyone solves this problem, let us know.
            Serial.print("");
            Serial.print("-");
            Serial.println(linea);   //Print out the GPRMC string (just for reference)
    
              //------------------------------------------          
              // Get Time (UTC)
              char *data = "";
              memset(data, 0, sizeof(data));
              data = dGPS::subStr(linea, ",", 2);
              fTime = atol(data);
              delay(5);
            
              //------------------------------------------
              // Get Status
              memset(data, 0, sizeof(data));
              cStatus = dGPS::subStr(linea, ",", 3);
              delay(5);
              
              //------------------------------------------
              // Get Latitude (degrees)
              char *hemi;
              memset(data, 0, sizeof(data));
              char *cLat = "";
              cLat = dGPS::subStr(linea, ",", 4);
              fLat = strtod(cLat, NULL);
              fLat = conv_coords(fLat);
              hemi = subStr(linea, ",", 5);    // Sets the hemisphere.
              if(*hemi == 'S') {fLat = fLat*-1;}        // Setting the hemisphere.
              //Serial.println(hemi);
              delay(5);
              
              //------------------------------------------          
              // Get Longitude (degrees)
              memset(data, 0, sizeof(data));
              char *cLon = "";
              cLon = dGPS::subStr(linea, ",", 6);
              fLon = strtod(cLon, NULL);
              fLon = conv_coords(fLon);
              hemi = subStr(linea, ",", 7);    // Sets the hemisphere.
              if(*hemi == 'W') {fLon = fLon*-1;}        // Setting the hemisphere.
              //Serial.println(*hemi);
              delay(5);          
              
              //------------------------------------------
              // Get Velocity (knots)
              memset(data, 0, sizeof(data));
              char *cVel = "";
              cVel = dGPS::subStr(linea, ",", 8);
              fVel = strtod(cVel, NULL);
              delay(5);
              
              //------------------------------------------
              // Get Heading (degrees)
              memset(data, 0, sizeof(data));
              char *cHead = "";
              cHead = dGPS::subStr(linea, ",", 9);
              fHead = strtod(cHead, NULL);
              delay(5);          
    
              //------------------------------------------
              // Get Date (UTC)
              memset(data, 0, sizeof(data));
              char *cDate = "";
              cDate = dGPS::subStr(linea, ",", 10);
              lDate = strtod(cDate, NULL);
              delay(5);
              
              //------------------------------------------
              // Get Mode Indicator 
              memset(data, 0, sizeof(data));
              char *cstr= subStr(linea,",",11);       // Takes the whole substring at the end of the GPRMC string (eg:A*0C)
              //Serial.println(cstr);
              mode=subStr(cstr, "*", 1);              // picks up mode indicator A from A*0C
              delay(5);          
              
              //------------------------------------------
              // Get Checksum value from packet and compute checksum from packet. 
              // Printed only if flag (*fla) is Y or y
              memset(data, 0, sizeof(data));
              checkSum = subStr(cstr, "*", 2);       // picks up checksum 0C from A*0C
              delay(5);
              computedSum=Csum(linea);               // Compute checksum from the incoming  GPRMC string
              
              conta=0;                               // Reset the buffer
              memset(linea, 0, sizeof(linea));
    	  break;
              
          }
          conta=0;                    // Reset the buffer
          memset(linea, 0, sizeof(linea));  
        }
      }
     } // END WHILE STATEMENT
     return true;  
    }
    
    // Function to update Number of Satellites used, HDOP, Altitude etc.
    bool dGPS::updategga(){ 
      comandoGPG[0] = '$';
      comandoGPG[1] = 'G';
      comandoGPG[2] = 'P';
      comandoGPG[3] = 'G';
      comandoGPG[4] = 'G';
      comandoGPG[5] = 'A';
      while(true){
      int byteGPS=-1;
      byteGPS=gpsSerial.read();      //read a byte from the serial port
      if (byteGPS == -1) {           // See if the port is empty yet
        delay(5); 
      } 
      else {
        linea[conta]=byteGPS;        // If there is serial port data, it is put in the buffer
        conta++;                      
        if (byteGPS==13){            // If the received byte is = to 13, end of transmission 
          cont=0;
          bien=0;
          for (int i=1;i<7;i++){     // Verifies if the received command starts with $GPGGA
            if (linea[i]==comandoGPG[i-1]){
              bien++;
            }
          }
          if(bien==6){               // If yes, continue and process the data        
            Serial.print("");        // For some reason, this needs to be here or you won't populate variables.  If anyone solves this problem, let us know.
            Serial.print("");
            Serial.print("-");
            Serial.println(linea);   //Print out the GPGGA string (just for reference)
              
              //------------------------------------------          
              // Get Number of satellites in use
              char *data = "";
              memset(data, 0, sizeof(data));
              data = dGPS::subStr(linea, ",", 8);
              fSat = atoi(data);
              delay(5);
            
              //------------------------------------------
              // Get HDOP
              memset(data, 0, sizeof(data));
              data = dGPS::subStr(linea, ",", 9);
              hDop=atof(data);
              delay(5);
              
              //------------------------------------------
              // Get Altitude above sea-level (meters)
              memset(data, 0, sizeof(data));
              data = dGPS::subStr(linea, ",", 10);
              fAlt=atof(data);          
              delay(5);
              
              conta=0;                    // Reset the buffer
              memset(linea, 0, sizeof(linea));
    	  break;
              
          }
          conta=0;                    // Reset the buffer
          memset(linea, 0, sizeof(linea));  
        }
      }
     } // END WHILE STATEMENT
     return true;  
    }
    I can share more of the code or answer any questions you have. I haven't modified any of the code from what I'm actually trying to run (other than I took out some lines and replaced them with ... in the loop() function... and obviously there's a lot of dGPS functions that I left out.)
     
  2. ghellquist macrumors regular

    Joined:
    Aug 21, 2011
    Location:
    Stockholm Sweden
    #2
    Hard to say. You forgot to include the code of dgps.Checks() and dgps.Checked().
    They might have som effect. Especially on the variable conta which should be zero at the entry, but is not set in updategga.

    I also find this code quite interesting:
    char *data = "";
    memset(data, 0, sizeof(data));

    char *data ="" allocates a one character string having only the character nil (end of string)

    sizeof(data) will return the size of char * which normally would be two bytes on the Arduino.

    So what you do is to allocate a one byte string, and fill it with two bytes. Later you fill it with even more bytes.
     
  3. Sander macrumors 6502

    Joined:
    Apr 24, 2008
    #3
    I agree with ghellquist. You declare the "data" variable to assign the result from the dGPS::subStr() function to it. There's no need to explicitly clear it in the mean time, and actually also not necessary to assign an empty string to it.

    Getting rid of this would already remove some of the aspects of your code which I like to call the "and suddenly, nothing happened" idiom. That's code which makes the reader scratch their head, thinking "what's going on?", then looking up the author, seeing it's not farmerdoug and continuing to think "Hmm... ArtOfWarfare probably didn't put that stuff in there for nothing", then pondering it for a long time and reaching the conclusion "maybe it's there for naught anyway?"

    There's more of this: Sometimes you clear this data variable (the wrong way, as ghellquist pointed out) only to not use it at all afterwards (see the code for Get Latitude (degrees)). After this piece of code, "data" is cleared again, only to not be used again :)

    Clean this up and let us know if it helps - if anything, it'll help us focus on the real issue...
     

Share This Page