waitForSentence assumes you are not using interrupts to read from the GPS

[srob1]

The waitForSentence method of the Adafruit_GPS class invokes read() which assumes that you are not using interrupts to read from the GPS.

It should probably have a third argument so that it doesn't invoke read() when is true.

Something like this:

//
/*!
@brief Wait for a specified sentence from the device
@param wait4me Pointer to a string holding the desired response
@param max How long to wait, default is MAXWAITSENTENCE
@param usingInterrupts True if using interrupts to read from the GPS (default is false)
@return True if we got what we wanted, false otherwise
*/
//
boolean Adafruit_GPS::waitForSentence(const char *wait4me, uint8_t max, boolean usingInterrupts) {
uint8_t i=0;
while (i < max) {
if (!usingInterrupts)
read();

if (newNMEAreceived()) {
  char *nmea = lastNMEA();
  i++;

  if (strStartsWith(nmea, wait4me))
      return true;
}

}

return false;
}

In this version I have also eliminated the copying of the start of the NMEA sentence to a local buffer which is unnecessary and inefficient. The strStartsWith utility very efficiently checks for the desired prefix on the NMEA sentence. (This assumes that the other bugs I've reported about the read routine returning NMEA sentences starting with LF characters from the previous NMEA sentence have been fixed.)

The strStartsWith routine looks like this:

//
/*!
@brief Checks whether a string starts with a specified prefix
@param str Pointer to a string
@param prefix Pointer to the prefix
@return True if str starts with prefix, false otherwise
*/
//
boolean strStartsWith(const char* str, const char* prefix)
{
while (*prefix) {
if (*prefix++ != *str++)
return false;
}
return true;
}

[ladyada]

thanks - please submit a PR fix if you can - great issues :)

[srob1]

not sure how to do that. (is PR a pull request?) haven't really used github much. I only posted here because when I posted some of the issues to the forums on the adafruit website I was instructed to post the issues on github. I'm happy to try if you can explain what I need to do, or point me to something that explains what to do.

[ladyada]

Great question. Its easy and a great skill to learn since almost all code these days is on github :)
Check out https://learn.adafruit.com/an-introduction-to-collaborating-with-version-control and https://learn.adafruit.com/contribute-to-circuitpython-with-git-and-github

[srob1]

Thanks so much! I'll take a look at those and then submit my version of the library that includes fixes for the various issues I've reported.

I have a couple of concerns though. First, changing the format of the nmea sentences that the library returns (to eliminate the LF from the beginning of the sentences) might break user code that has been relying on the current erroneous behavior.

Also, I don't use the LOCUS stuff at all, so while I will look through those routines to spot any obvious issues, I probably can't test those.

Hopefully you have some sort of test suite for library changes that you can run it through before incorporating the changes in the actual library.

[ladyada]

theres no way to really test without wiring it up, but its easier to test all at once instead of one at a time :)

[srob1]

ok, well, I tried my best. read those tutorials on git, installed git, got to the point of the pull request. however, the travis ci thing is complaining about some "sam.h" include file. I'm a bit lost at this point, because I have no idea what that file is. It's not one of mine, and not one in the library. Perhaps you could take a look and let me know what I need to change.

[ladyada]

oh thats us. hold on a sec.

[srob1]

do I need to do anything else?

[drak7]

Nope, you're all set! Thanks for contributing, your PR is merged.