SlashDevin / NeoGPS

NMEA and ublox GPS parser for Arduino, configurable to use as few as 10 bytes of RAM
GNU General Public License v3.0
702 stars 194 forks source link

Library is difficult to configure and use #141

Open bzroom opened 3 years ago

bzroom commented 3 years ago

Thank you for the library but i have some extremely critical feedback.

I've used a TON of arduino libraries and normally I rave about the Arduino platform, libraries, their structure, and how the examples are normally so easy to use. This library breaks that mold.

The define based configs have to be changed in the library, per project. You can't use this library in more than one project? You should never have to modify a library to use it.

All of the configuration is raw. No batteries included. Nothing works out of the box.

Why is this code in an example and not part of the library? Why is so much example code needed to change baud and enable messages?

` //------------------------------------------- // U-blox UBX binary commands

const unsigned char ubxRate1Hz[] PROGMEM = { 0x06,0x08,0x06,0x00,0xE8,0x03,0x01,0x00,0x01,0x00 }; const unsigned char ubxRate5Hz[] PROGMEM = { 0x06,0x08,0x06,0x00,200,0x00,0x01,0x00,0x01,0x00 }; const unsigned char ubxRate10Hz[] PROGMEM = { 0x06,0x08,0x06,0x00,100,0x00,0x01,0x00,0x01,0x00 }; const unsigned char ubxRate16Hz[] PROGMEM = { 0x06,0x08,0x06,0x00,50,0x00,0x01,0x00,0x01,0x00 };

// Disable specific NMEA sentences const unsigned char ubxDisableGGA[] PROGMEM = { 0x06,0x01,0x08,0x00,0xF0,0x00,0x00,0x00,0x00,0x00,0x00,0x01 }; const unsigned char ubxDisableGLL[] PROGMEM = { 0x06,0x01,0x08,0x00,0xF0,0x01,0x00,0x00,0x00,0x00,0x00,0x01 }; const unsigned char ubxDisableGSA[] PROGMEM = { 0x06,0x01,0x08,0x00,0xF0,0x02,0x00,0x00,0x00,0x00,0x00,0x01 }; const unsigned char ubxDisableGSV[] PROGMEM = { 0x06,0x01,0x08,0x00,0xF0,0x03,0x00,0x00,0x00,0x00,0x00,0x01 }; const unsigned char ubxDisableRMC[] PROGMEM = { 0x06,0x01,0x08,0x00,0xF0,0x04,0x00,0x00,0x00,0x00,0x00,0x01 }; const unsigned char ubxDisableVTG[] PROGMEM = { 0x06,0x01,0x08,0x00,0xF0,0x05,0x00,0x00,0x00,0x00,0x00,0x01 }; const unsigned char ubxDisableZDA[] PROGMEM = { 0x06,0x01,0x08,0x00,0xF0,0x08,0x00,0x00,0x00,0x00,0x00,0x01 };

static const uint8_t ubxReset[] __PROGMEM = { ublox::UBX_CFG, ublox::UBX_CFG_RST, UBX_MSG_LEN(ublox::cfg_reset_t), 0, // word length MSB is 0 0,0, // clear bbr section ublox::cfg_reset_t::CONTROLLED_SW_RESET_GPS_ONLY, // reset mode 0x00 // reserved };

void resetGPS() { enum sendUBXmethod_t { FROM_RAM_STRUCT, FROM_PROGMEM_BYTES, FROM_PROGMEM_STRUCT };

const sendUBXmethod_t WAY = FROM_PROGMEM_STRUCT; // pick one

switch (WAY) { case FROM_RAM_STRUCT: { ublox::cfg_reset_t cfg_cold; // RAM cfg_cold.clear_bbr_section = // Hotstart { false };

      // Warmstart
      // { true, false };

      // Coldstart
      // { true, true, true, true, true, true, true, true, true,
      //   true, // reserved1 is 2 bits
      //   true, true, true, 
      //   true, // reserved2 is 1 bit
      //   true };

    cfg_cold.reset_mode = ublox::cfg_reset_t::CONTROLLED_SW_RESET_GPS_ONLY;
    cfg_cold.reserved   = 0;
    gps.send_request( cfg_cold );
  }
  break;

case FROM_PROGMEM_BYTES:
  sendUBX( ubxReset, sizeof(ubxReset) ); // just send the PROGMEM bytes
  break;

case FROM_PROGMEM_STRUCT:
  {
    const ublox::cfg_reset_t *cfg_cold_ptr = (const ublox::cfg_reset_t *) ubxReset;
    gps.send_request_P( *cfg_cold_ptr );
  }
  break;

}

} // resetGPS

//--------------------------

void sendUBX( const unsigned char *progmemBytes, size_t len ) { gpsPort.write( 0xB5 ); // SYNC1 gpsPort.write( 0x62 ); // SYNC2

uint8_t a = 0, b = 0; while (len-- > 0) { uint8_t c = pgm_read_byte( progmemBytes++ ); a += c; b += a; gpsPort.write( c ); }

gpsPort.write( a ); // CHECKSUM A gpsPort.write( b ); // CHECKSUM B

} // sendUBX

//------------------------------------------- // U-blox NMEA text commands

const char disableRMC[] PROGMEM = "PUBX,40,RMC,0,0,0,0,0,0"; const char disableGLL[] PROGMEM = "PUBX,40,GLL,0,0,0,0,0,0"; const char disableGSV[] PROGMEM = "PUBX,40,GSV,0,0,0,0,0,0"; const char disableGSA[] PROGMEM = "PUBX,40,GSA,0,0,0,0,0,0"; const char disableGGA[] PROGMEM = "PUBX,40,GGA,0,0,0,0,0,0"; const char disableVTG[] PROGMEM = "PUBX,40,VTG,0,0,0,0,0,0"; const char disableZDA[] PROGMEM = "PUBX,40,ZDA,0,0,0,0,0,0";

const char enableRMC[] PROGMEM = "PUBX,40,RMC,0,1,0,0,0,0"; const char enableGLL[] PROGMEM = "PUBX,40,GLL,0,1,0,0,0,0"; const char enableGSV[] PROGMEM = "PUBX,40,GSV,0,1,0,0,0,0"; const char enableGSA[] PROGMEM = "PUBX,40,GSA,0,1,0,0,0,0"; const char enableGGA[] PROGMEM = "PUBX,40,GGA,0,1,0,0,0,0"; const char enableVTG[] PROGMEM = "PUBX,40,VTG,0,1,0,0,0,0"; const char enableZDA[] PROGMEM = "PUBX,40,ZDA,0,1,0,0,0,0";

const char baud9600 [] PROGMEM = "PUBX,41,1,3,3,9600,0"; const char baud38400 [] PROGMEM = "PUBX,41,1,3,3,38400,0"; const char baud57600 [] PROGMEM = "PUBX,41,1,3,3,57600,0"; const char baud115200[] PROGMEM = "PUBX,41,1,3,3,115200,0";

//--------------------------

const uint32_t COMMAND_DELAY = 250;

void changeBaud( const char textCommand, unsigned long baud ) { gps.send_P( &tee, (const __FlashStringHelper ) disableRMC ); delay( COMMAND_DELAY ); gps.send_P( &tee, (const FlashStringHelper *) disableGLL ); delay( COMMAND_DELAY ); gps.send_P( &tee, (const FlashStringHelper ) disableGSV ); delay( COMMAND_DELAY ); gps.send_P( &tee, (const __FlashStringHelper ) disableGSA ); delay( COMMAND_DELAY ); gps.send_P( &tee, (const FlashStringHelper *) disableGGA ); delay( COMMAND_DELAY ); gps.send_P( &tee, (const FlashStringHelper ) disableVTG ); delay( COMMAND_DELAY ); gps.send_P( &tee, (const __FlashStringHelper ) disableZDA ); delay( 500 ); gps.send_P( &tee, (const __FlashStringHelper *) textCommand ); gpsPort.flush(); gpsPort.end();

DEBUG_PORT.print( F("All sentences disabled for baud rate ") ); DEBUG_PORT.print( baud ); DEBUG_PORT.println( F(" change. Enter '1' to reenable sentences.") ); delay( 500 ); gpsPort.begin( baud );

} // changeBaud

`

R1DEN commented 3 years ago

I think this is the price you have to pay to be faster and more light-weight than any other library (conditional compilation) https://github.com/SlashDevin/NeoGPS/blob/master/extras/doc/Tradeoffs.md

danalvarez commented 2 years ago

I personally feel it's one of the best GPS libraries out there. Yes, configuration can be difficult, took me a while to understand what had to be configured and what didn't. But eventually got it running, and I love how lightweight it is. Also, since I am using a u-blox GPS, it really helps the library includes built-in support to configure these GPS modules (and adding other commands that are not included isn't too hard, just a matter of defining them in ublox/ubxmsg.h).

Having to define so many things at compilation time can be messy, but it has also helped me reduce much needed flash and RAM in my m328p.