vshymanskyy / TinyGSM

A small Arduino library for GSM modules, that just works
GNU Lesser General Public License v3.0
1.95k stars 723 forks source link

`getGPS` vs `getGPSraw` accuracy issue/bug #255

Closed BracketJohn closed 4 years ago

BracketJohn commented 5 years ago

TinyGSM version: 0.3.5 Hardware: SIM868 R14.18, Arduino Uno for prototyping, atmega328p for deployment

Scenario, steps to reproduce

Trying to get the exact position:

modem.enableGPS();

String rawGps = modem.getGPSraw();

float lat, lng, speed;
int alt, vsat, usat;
modem.getGPS(&lat, &lng, &speed, &alt, &vsat, &usat);

Serial.println(rawGps);

Serial.print(lat, 6);
Serial.print(", ");
Serial.println(lng, 6);

Expected result

> 1,1,20190318170547.000,50.XXXXXX,6.XXXXXX,227.847,0.00,157.3,1,,1.6,1.9,1.0,,12,6,,,30,,
> 50.XXXXXX, 6.XXXXXX

Actual result

> 1,1,20190318170547.000,50.XXXXXX,6.XXXXXX,227.847,0.00,157.3,1,,1.6,1.9,1.0,,12,6,,,30,,
> 50.XX0000, 6.XX0000

(note the changed number of decimal places X)

Possible solution/Source of issue

The Arduino toFloat method used truncates floats rather strongly. Using atof or something similar should fix the issue. This issue should also arise for the SIM7000 module code should also be affected.

If this is an actual issue, and if my suspicion is confirmed by one of the maintainers, I'm more than open to create a PR addressing this, as I also need the higher accuracy at work 😄 For now I will parse getGPSraw myself.

I also suggest to improve getGPS further, e.g., C/N0_MAX is not retrieved by getGPS. C/N0_MAX is an important ratio that can tell the user how "close" he is to locking on GPS signal.

Edit: As per @djcobylevy's comment, I had to use , 6 in the Serial.print in order to correctly print the erroneous output. This is fixed now, thanks!

djcobylevy commented 4 years ago

try:

Serial.print(lat, 6); Serial.print(", "); Serial.println(lng, 6);

BracketJohn commented 4 years ago

This will produce:

> 50.XX0000, 6.XX0000

The problem is not the printing, but the underlying use of toFloat, which, as documented on the page that I linked, rounds to 2 places.

Although you're correct, that this is problematic in my problem description, will update this, thanks!

djcobylevy commented 4 years ago

it does works for me. this is my output: 19:40:17.482 -> 37.077618, 31.825180

BracketJohn commented 4 years ago

Which module are you using?

BracketJohn commented 4 years ago

And, can you show me the code that produces this? (or rather, the most boiled down version of you code that produces this? See my example code for a small example)

djcobylevy commented 4 years ago

sim868

BracketJohn commented 4 years ago

Also, maybe you should edit your comments, so that your exact location is hidden.

djcobylevy commented 4 years ago

Also, maybe you should edit your comments, so that your exact location is hidden, you call! i deleted the post and riposted it earlier haha.

the relevant code is (based on yours): float lat, lng, speed; int alt, vsat, usat; String GpsLat,GpsLong; modem.getGPS(&lat, &lng, &speed, &alt, &vsat, &usat); //convert float to string GpsLat=String(lat, 6); GpsLong=String(lng, 6);

  Serial.print(GpsLat);
  Serial.print(", ");
  Serial.println(GpsLong);
BracketJohn commented 4 years ago

I think what's going on here, is that you're casting un-initialized memory to String. The Arduino String lib is very buggy and has many problems (heap allocation, arguably, could be one of them).

Is the location that is printed out to you, your actual location?

BracketJohn commented 4 years ago

@djcobylevy please have a look at https://github.com/vshymanskyy/TinyGSM/issues/352 for more on this topic. I think you should totally continue to use this library, just be aware of its shortcomings.

djcobylevy commented 4 years ago

Is the location that is printed out to you, your actual location? yes, its correct but the last digit (rounds up the 6th decimal 0.00000X). i checked on the map and it comes down to a few meter which is good enough for my usage.

SRGDamia1 commented 4 years ago

I believe this has been addressed in the update to 0.10.