anasalawneh / android-protips-location

Automatically exported from code.google.com/p/android-protips-location
0 stars 0 forks source link

Wrong tests in LegacyLastLocationFinder.getLastBestLocation() #6

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
In the LegacyLastLocationFinder.getLastBestLocation() method, the tests on the 
fix time are inverted!

if ((time < minTime && accuracy < bestAccuracy))
should be:
if ((time > minTime && accuracy < bestAccuracy))

if (time > minTime && bestAccuracy == Float.MAX_VALUE && time < bestTime)
should be:
if (time < minTime && bestAccuracy == Float.MAX_VALUE && time > bestTime)

and
if (locationListener != null && (bestTime > minTime || bestAccuracy > 
minDistance))
should be:
if (locationListener != null && (bestTime < minTime || bestAccuracy > 
minDistance))

It is less important but the comment for the minTime is misleading.

I also think the test to get the latest fix no matter its accuracy in both 
LegacyLastLocationFinder and GingerbreadLastLocationFinder should not include a 
check on minTime. Then,
if (time < minTime && bestAccuracy == Float.MAX_VALUE && time > bestTime)
should actually be:
if (bestAccuracy == Float.MAX_VALUE && time > bestTime)

Original issue reported on code.google.com by pbp...@gmail.com on 16 Aug 2011 at 5:17

GoogleCodeExporter commented 9 years ago
I thought it over and the last part of my report should be ignored.

Original comment by pbp...@gmail.com on 16 Aug 2011 at 5:52

GoogleCodeExporter commented 9 years ago
Worse yet. minTime is defined as "Minimum time required between location 
updates" but it is being compared with Location.getTime() which is defined as 
"Returns the UTC time of this fix, in milliseconds since January 1, 1970".

We shouldn't be comparing minTime with Location.getTime(). We should be 
comparing it with timeElapsed (System.currentTimeMillis() - Location.getTime()).

This bug applies to GingerbreadLastLocationFinder as well.

Original comment by cow...@bbs.darktech.org on 7 May 2012 at 3:58

GoogleCodeExporter commented 9 years ago
Actually, the correct and simple fix would be to just edit the line
long time = location.getTime();
to
long time = System.currentTimeMillis() - location.getTime();
That will solve all issues.
Also minTime should be defined something like "Minimum time since last location 
update"

Original comment by lionscr...@gmail.com on 27 Dec 2012 at 10:36