search

sparkfun / SparkFun_Ublox_Arduino_Library

Library to control UBX binary protocol and NMEA over I2C on Ublox GPS modules
Other
144 stars 84 forks source link

Condition is always false #150

Closed rubienr closed 4 years ago

rubienr commented 4 years ago

With const int checksumFailurePin = -1; the condition is always false in several places: https://github.com/sparkfun/SparkFun_Ublox_Arduino_Library/blob/97bd455b0e7a05b92bfe5a528c28cc47f2e2d289/src/SparkFun_Ublox_Arduino_Library.cpp#L50 https://github.com/sparkfun/SparkFun_Ublox_Arduino_Library/blob/97bd455b0e7a05b92bfe5a528c28cc47f2e2d289/src/SparkFun_Ublox_Arduino_Library.cpp#L352 https://github.com/sparkfun/SparkFun_Ublox_Arduino_Library/blob/97bd455b0e7a05b92bfe5a528c28cc47f2e2d289/src/SparkFun_Ublox_Arduino_Library.cpp#L386 https://github.com/sparkfun/SparkFun_Ublox_Arduino_Library/blob/97bd455b0e7a05b92bfe5a528c28cc47f2e2d289/src/SparkFun_Ublox_Arduino_Library.cpp#L446 https://github.com/sparkfun/SparkFun_Ublox_Arduino_Library/blob/97bd455b0e7a05b92bfe5a528c28cc47f2e2d289/src/SparkFun_Ublox_Arduino_Library.cpp#L894

Suggested fix in: https://github.com/sparkfun/SparkFun_Ublox_Arduino_Library/blob/97bd455b0e7a05b92bfe5a528c28cc47f2e2d289/src/SparkFun_Ublox_Arduino_Library.h#L73

#if defined(SPARKFUN_UBLOX_CHECKSUM_PIN)
const int checksumFailurePin {SPARKFUN_UBLOX_CHECKSUM_PIN};
#endif

and additionally gate the mentioned locations in cpp with

#if defined(SPARKFUN_UBLOX_CHECKSUM_PIN)
...
#endif
PaulZC commented 4 years ago

Hi @rubienr , It is standard practice for debugging pins to be defined as "-1" when they are not being used. If we want to look for checksum failures, we set the pin numbers to a true physical pin. Then we set it back to -1 again before releasing the code. I'm afraid I don't agree with your suggested fix. I would like to leave the code as it is. Best wishes, Paul

rubienr commented 4 years ago

Hi,

Unfortunately to debug with the CRC pin it is necessary to touch the library code. With the #if defined() approach (aside of saving some instructions and bytes of flash when not debugging) it would be possible to define for example build_flags = -DSPARKFUN_UBLOX_CHECKSUM_PIN=2 in the platformio.ini and the same result would be achieved without touching code. Since the variable is const it is not intended to be modified at run time, which nicely fits in the macro concept (resolved at compile time). Another benefit is that one cannot forget to set the value back to -1 before merging the RC branch to master :)