mikalhart / TinyGPSPlus

A new, customizable Arduino NMEA parsing library
http://arduiniana.org
1.08k stars 490 forks source link

Fix compile problem #118

Closed JakubRybakowski closed 1 year ago

TD-er commented 1 year ago

Your "performance fix" commit is just changing double to float. This may indeed be a performance fix, but it is also a reduction in precision.

It may be good enough for most, but it is for sure not something that should be replaced for any use case.

JakubRybakowski commented 1 year ago

Sorry. Only "Update compile problem" was supposed to be sent as PR.

"Performance fix" is not ready to merge with master. However, for esp32, which only has units for float and double is simulated, this is a significant performance improvement.

TD-er commented 1 year ago

You pushed it to the same pull request, so it is now part of the code to be merged.

svdrummer commented 1 year ago

Your "performance fix" commit is just changing double to float. This may indeed be a performance fix, but it is also a reduction in precision.

It may be good enough for most, but it is for sure not something that should be replaced for any use case.

100% agree. Float is no good compared to double. Some time ago, extensive testing was done by a third party to show a double was a major fix rather than float. MOST need the double.

TD-er commented 1 year ago

At least for lat/long you must use double. Not sure about the direction.

Speed/altitude, etc. will obviously not gain any useful resolution when using double.

JakubRybakowski commented 1 year ago

Yes, I understand float's accuracy is a lot less than double's. However, on ESP32 float is much faster than double. Where float is not too fast anyway. On my system, dobule handling of GPS became the slowest part of the code. I'm working on a solution to get rid of as many doubles and floats as possible from the code where possible. Of course, as I mentioned, this fix was wrongly sent to PR. I'll come back here when my fix is ​​ready. The change to float was just a proof of concept. To test the performance increase.

TD-er commented 1 year ago

I wonder how many computations you need to make to notice such a significant change in performance.

Typically it is only a few position updates per second and you can perform quite a lot of double operations per second on an ESP32, even though it is purely in software.

svdrummer commented 1 year ago

My two cents worth. As the TinyGPSPlus is a mature library, used in a significant number of projects. If you are having speed issues, maybe your main code is overworking the library. The difference between float and double in LAT long is a sub metre to a house block or more difference. GPS modules can be told what NMEA sentences to send. If you only want certain values, why not turn off unused sentences? ESP8266 and 32 have no speed issues, even with commercial quad copter systems etc. Just dont mess with the double on LAT LONG in the library.

TD-er commented 1 year ago

The difference between float and double in LAT long is a sub metre to a house block or more difference.

Depends on where you are located :) With single digit degrees the difference isn't that much, but when you're close to 180 deg longitude, or closer to the poles, you will see a more significant difference.

Just some estimate on worst-case resolution when using floats: Typically you can store 6-ish decimal digits in a float. With longitude close to 180 degree you're left with about 4 decimals. Just as a rule of thumb, 1 degree is rougly 100 km around the equator. (40'000 km / 400 degree :) ) So with only 3 - 4 decimal digits left in the worst case, your resolution is about 10 - 100 meter when using a float.

JakubRybakowski commented 1 year ago

I looked at the library. I think rawLat(), rawLing() is fine for me and what I was thinking of doing by getting rid of the double. So there is no scope here to improve performance.

In most cases, there is no need to convert to double values. For example, when creating a gps tracker, we finally transform the data into an http query or other communication method that will accept a string of characters. So raw data will be the direction it wants to go.

When it comes to performance for battery-powered devices, any reduction in time spent on repetitive tasks is good.

svdrummer commented 1 year ago

The difference between float and double in LAT long is a sub metre to a house block or more difference.

Depends on where you are located :) With single digit degrees the difference isn't that much, but when you're close to 180 deg longitude, or closer to the poles, you will see a more significant difference.

Just some estimate on worst-case resolution when using floats: Typically you can store 6-ish decimal digits in a float. With longitude close to 180 degree you're left with about 4 decimals. Just as a rule of thumb, 1 degree is roughly 100 km around the equator. (40'000 km / 400 degree :) ) So with only 3 - 4 decimal digits left in the worst case, your resolution is about 10 - 100 meter when using a float.

There is actually a few white papers written on the subject. TinyGPS was originally a float. When a white paper made specific references to TinyGPS with testing and formulas, is was shown that the Double which comes out of NMEA4,5,6,7,8, etc was being truncated, the accuracy of the test wis improved. I am glad you spotted the potential change. I will lock my library version. I would have missed that one.

NEO series 8, 9 has a non-NMEA0183 HEX code output option, maybe this would be a faster option for the original poster. It doesnt work with tiny GPS, but being hex codes and such a small string length, maybe this would work. Some quad copters aka drones, use this faster format as they update 15 times per second. Calculating satellite drift is more of an issue for me, not speed. cheers

svdrummer commented 1 year ago

I looked at the library. I think rawLat(), rawLing() is fine for me and what I was thinking of doing by getting rid of the double. So there is no scope here to improve performance.

In most cases, there is no need to convert to double values. For example, when creating a gps tracker, we finally transform the data into an http query or other communication method that will accept a string of characters. So raw data will be the direction it wants to go.

When it comes to performance for battery-powered devices, any reduction in time spent on repetitive tasks is good.

Hope you have better luck than me on battery power GPS. The GPS has to be on all the time. I did look at how commercial products claim to have GPS running on a coin cell. Turns out, most of the false claims in fact use an ap to access the users cell phone. A quick Bluetooth burst grabbed the "GPS" location and used that data. Dog collars, keyring finders etc, all have the ability to turn on ANYONES iphone without permission, to send location data. I digress, however, my battery powered GPS items have to use a rechargeable battery. The GPS module draws the power, while the micro is in deep sleep.