version 0.8 ignores GPS location info

Bug #579419 reported by droidguy
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenSatNav
Fix Released
Medium
Kieran Fleming

Bug Description

I discovered and downloaded OSN two days ago. Great project. One problem I see with it right now is that it ignores the location from my GPS in term of Autofollowing, although it seems to correctly use the coarse location info from cell network.

Revision history for this message
chris_debian (cjhandrew) wrote :

Droidguy,

Thanks for the compliment about the project; the guys are doing a great job.

Please can you mention this bug on the list also, just in-case it's something we can sort-out quickly for you.

Cheers,

Chris.

Revision history for this message
Kieran Fleming (kieran-fleming) wrote :

Hi,
This shouldn't happen. Please check that GPS is enabled in the settings and that you have a GPS fix. You can see if you have a GPS fix by looking at the satellite dish icon on the top of the screen. If it is flashing the GPS is trying to get a fix, and if it is solid it has a fix.

Revision history for this message
Murphy (murphy2712+launchpad) wrote :

I agree with droidguy:
when using network/cell *and* gps positionning,
when starting OSN and the gps has not yet a fix, osn follows only the network/cell location.
Even when the gps finally got a fix.

Revision history for this message
chris_debian (cjhandrew) wrote :

Murphy,

Thanks for your clarification of the comments. Is this something you may be able to look at?

Chris.

Revision history for this message
Kieran Fleming (kieran-fleming) wrote :

Yep, confirming.
In line 230 of OpenStreetMapActivity.java there is bit that I have commented out - re-enabling this fixes the problem.
There is a bug where sometimes the location stops updating for no reason which led me to comment this bit out, but the problem happens either way. I will put the fix into SVN as the second problem is rarer.

Revision history for this message
chris_debian (cjhandrew) wrote :

Kieran,

Is it worth rolling an APK, so we can test this?

Chris.

Revision history for this message
Murphy (murphy2712+launchpad) wrote :

kizza wrote:
> Yep, confirming.
> In line 230 of OpenStreetMapActivity.java there is bit that I have commented out - re-enabling this fixes the problem.
> There is a bug where sometimes the location stops updating for no reason which led me to comment this bit out, but the problem happens either way. I will put the fix into SVN as the second problem is rarer.

For this location problem (it stops updating), I got it very often on motorway, maybe due to the speed.
I think it's worth opening an issue.

chris_debian wrote:
> Is it worth rolling an APK, so we can test this?

Here's a new apk with the uncommented code, but I don't see any difference (but now that I've a fix I must wait to do another test),
could you test it?

Revision history for this message
chris_debian (cjhandrew) wrote :

Will try to test new APK in the next 24hrs.

Thanks,

Chris.

Revision history for this message
chris_debian (cjhandrew) wrote :

Kieran,

I have just tried the latest APK that has been published. I am pleased to report that I didn't have the same problem. Sometimes the map tiles didn't load, but this was because I was in a new area and had downloaded the tiles previously. Although the map tiles didn't show, I could see the background moving relative to the pointer, suggesting that everything was ok.

I have re-assigned this to you, so that you can decide the way forward. From my perspective, this bug has been fixed.

Cheers,

Chris.

Revision history for this message
chris_debian (cjhandrew) wrote :

EDIT:

Sometimes the map tiles didn't load,

### when I had lost my GPRS signal ###

but this was because I was in a new area and had downloaded the tiles on a previous accasion.

Revision history for this message
chris_debian (cjhandrew) wrote :

Apologies for spelling on previous post. Additionally, my preceeding hashes were interpreted as a '1.'

Cheers,

Chris.

Revision history for this message
chris_debian (cjhandrew) wrote :

Please ignore comment 9 to this one. I'm at work and edited the wrong bug!!

Sorry,

Chris.

Revision history for this message
chris_debian (cjhandrew) wrote :

Target version awarded= 1.2. Due to severity, an earlier release would be preferred.

Chris.

Revision history for this message
Murphy (murphy2712+launchpad) wrote :

I'm working on it. Uncommented "line 230 of OpenStreetMapActivity.java" doesn't change the issue.

Who wrote function onLocationChanged in SatNavActivity?
There's a weird "if currentLocation.distanceTo(newLocation) < 100" which means that 2 following locations could not have distance superior at 100m!
If we have enabled network+gps, when we start osn we rely on network location, but when we have a fix the gps is very likely to be more distant that 100m!
I've commented this test and the bug is now fixed :)

I think this could also correct bug 15:
technically the gps chipset runs at 1Hz (1 position per second) but the onLocationChanged is called "when" the gps wants so it could be every second, sometimes every 2 seconds or sometimes every 3 seconds...
The code I removed detects a distance of 100meter for following the next position, so if the gps call onLocationChanged every:
- 1 second it corresponds at 100m/s (360km/h),
- 2 seconds => 100m/2s (180km/h),
- 3 seconds => 100m/3s (120km/h).
It corresponds at my case that on highway (>120km/h) sometimes the gps stops following, it's simply because the gps has called onLocationChanged 3 seconds after the last call.

Revision history for this message
Murphy (murphy2712+launchpad) wrote :

Sorry for the patchs, there's a lot of wind inside :)
Here's more appropriate ones.

Revision history for this message
chris_debian (cjhandrew) wrote :

Murphy,

That's reall yencouraging. Does anyone know about the < 100 question. Did we write it or is it a hangover from the AndNav code? Mistakes happen, but if we wrote it, it may make sense to someone.

Can anyone clarify?

Cheers,

Chris.

Revision history for this message
Kieran Fleming (kieran-fleming) wrote :

I added the < 100 thing.

It was a way of fixing this problem where for one reading the GPS reported a location that was very far away (~1km).
It was really only meant to be a temporary thing of course :)

It would be good to keep this in as the false reading would make the auto-rerouter kick in. The fix would be simply to allow the update if it's been > 1 second since the last reading or the distance is within the threshold.

Revision history for this message
Murphy (murphy2712+launchpad) wrote :

For the tunnel problem it should work with your "> 1 second" test.

But what about the switch between network and gps location?
Or if the gps fix begin with non accurate value (indoors) and follows with more accurate ones within a second?

I will reintegrate your code with the "> 1 second", it will be better for now.

Also, I tested the auto-rerouter last saturday and it doesn't worked at all (I went many kilometers away from the calculated route and it doesn't recalculate).

Revision history for this message
evolvedlight (steve-evolvedlight) wrote :

Kizza: Is the < 100 thing to combat inaccurate GPS fixes?

I had this same problem when writing the code to collect GPS traces - I fixed it by testing for aLocation.accuracy, and that worked a treat.

Steve

Revision history for this message
Murphy (murphy2712+launchpad) wrote :

Also I love recording my trips by plane (it works if you're near a window!).
With this "< 100" system we're stuck at 360km/h.

Revision history for this message
Murphy (murphy2712+launchpad) wrote :

kizza wrote:
> I added the < 100 thing.
>
> It was a way of fixing this problem where for one reading the GPS reported a location that was very far away (~1km).

Does anyone had a far away location using the gps? It's not likely possible, except if...

It was introduced by you, with the lines "230 of OpenStreetMapActivity.java"?
These lines seems to deactivate the current provider and reenable the gps one, useful in case the current location is network and gps is more accurate. But very inconvenient in all other cases. Changing a locationProvider has nothing to do in a route recording code.

All these lines were not in the original OpenStreetMapActivity.java .

I don't understand why there's no much difference in your custom file (OpenStreetMapActivity.java):
you currently use 2 location providers (network AND gps) which means that you get twice updates in the same onLocationChanged(SatNavActivity)!
That could correspond to your far away positions sometimes (network interfering with gps location).

I think we must deactivate the network provider as soon as we get a gps fix, in the beginning of onLocationChanged(SatNavActivity.java).

Revision history for this message
evolvedlight (steve-evolvedlight) wrote :

Hey

I'm back now. Tried for 2 days to implement vector drawing in opengl using the source data, but kept being stupid, so giving up until I have some more free time.

I would recommend the following changes to SatNavActivity onLocationChanged():

The first line should be just
<pre>
 if (newLocation != null) {

</pre>
and then the line
<pre>

if (this.routeOverlay != null && this.autoFollowing && this.mOsmv.getZoomLevel()>14) {
</pre>

should actually have a check for accuracy instead:

<pre>
if (this.routeOverlay != null && this.autoFollowing && this.mOsmv.getZoomLevel()>14 && aLocation.getAccuracy < 20) {
</pre>

that means if the accuracy is less than 20m, dont try calculating a new route overlay.

Personally, in terms of the two location providers, I fully agree that when the GPS location provider is active, the network provider should be immediately deactivated, and that solves other problems.

Steve

Revision history for this message
Kieran Fleming (kieran-fleming) wrote :

Yep, checking for accuracy makes sense.
The original OpenStreetMapActivity.java in osmdroid only uses the GPS service so it didn't have to deal with multiple sources.
Here's how I intended it for work - obviously there's a bug somewhere :)

Request data for both network and GPS:
<pre>
getLocationManager().requestLocationUpdates(LocationManager.NETWORK_PROVIDER, 0, 0, this.mLocationListener);
getLocationManager().requestLocationUpdates(LocationManager.GPS_PROVIDER, 0, 0, this.mLocationListener);
</pre>
When location changes, check if the data came from the GPS:
<pre>
if(loc.getProvider()==LocationManager.GPS_PROVIDER) {
</pre>
Remove location updates and request only from GPS:
    <pre>
    getLocationManager().removeUpdates(mLocationListener);
    getLocationManager().requestLocationUpdates(LocationManager.GPS_PROVIDER,0, 0, mLocationListener);
</pre>
It's done this way as I couldn't find a way to disable updates just from the network.
Finally, if the location is lost, request from both sources again as above.

Possible bugs:
I'm not sure if onLocationLost really gets called
Sometimes the GPS accuracy will be worse than the network but it is used anyway.

I'm going to remove the distance test and see if I get the 'jumping' problem again tomorrow. If we can be sure that it is causing problems it should be removed regardless.

Revision history for this message
Murphy (murphy2712+launchpad) wrote :

kizza wrote:
> When location changes, check if the data came from the GPS:
> [...]
> Remove location updates and request only from GPS:
> [...]
> It's done this way as I couldn't find a way to disable updates just from the network.

What's worrying me is that there's no check on what's the previous provider (it must be network), so this "remove/request" is done each onLocationChanged?

I've tested the onLocationLost, it isn't called when the location is lost. Actually, it's not in the API!

Revision history for this message
Murphy (murphy2712+launchpad) wrote :

evolvedlight made the changes in his apk/patch at Feature #30.
Could someone test it so we can close this bug and bug #15? (or just my apk/patch in comment 15, for me the bugs are gone)

Revision history for this message
Kieran Fleming (kieran-fleming) wrote :

Suggested changes committed into SVN as revision 53.
I'm putting this into 'Feedback' as I think we need to be sure that this fixes all the issues.

Revision history for this message
Murphy (murphy2712+launchpad) wrote :

I used OSN today and noticed a bug (using last evolvedlight version): sometimes I got a far away position.
It must be the network positioning (not deactivated?!) which disturb the gps.

We're currently using 2 positioning systems simultaneously:
- when onLocationChanged is called can we know which one it is?
- how to disable only the network positionning?

In OpenStreetMapActivity.java we use the same mLocationListener for both network and gps location,
maybe we must use 2 different SampleLocationListener in order to not mix them?

Revision history for this message
droidguy (stan-berka) wrote :

To avoid the regression with this issue and other, what do you think to start using unit tests? Android has native unit tests starting from SDK 1.5, if I remember.

Revision history for this message
Kieran Fleming (kieran-fleming) wrote :

Murphy wrote:
> We're currently using 2 positioning systems simultaneously:
> - when onLocationChanged is called can we know which one it is?
> - how to disable only the network positionning?
>
> In OpenStreetMapActivity.java we use the same mLocationListener for both network and gps location,
> maybe we must use 2 different SampleLocationListener in order to not mix them?

In the comment above I tried to explain how this doesn't really happen in actual use and I don't think I can explain it any better. If you can show me that the we are definitely getting a network location in between GPS locations then I'll happily admit that I was wrong. Please know that I am definitely not saying that this code is good - it has to be fixed but at the moment I'm trying to work out the best way to approach it.

If you want to test without using network based locations you can disable it in the preferences.

Revision history for this message
Murphy (murphy2712+launchpad) wrote :

Just added few lines in the beginning of onLocationChanged:
<pre>
if (newLocation.getProvider().equalsIgnoreCase(LocationManager.NETWORK_PROVIDER)){
 Toast.makeText(this, "Location="+newLocation.getProvider(), Toast.LENGTH_SHORT).show();
}
</pre>

...and guess what? Every minute you get the message "Location=network", even after a gps fix.

And I saw THE error: you used "==" to compare two strings!!!
In OpenStreetMapActivity.java, class SampleLocationListener:
<pre>
if(loc.getProvider()==LocationManager.GPS_PROVIDER) {
 getLocationManager().removeUpdates(mLocationListener);
 getLocationManager().requestLocationUpdates(LocationManager.GPS_PROVIDER,0, 0, mLocationListener);
}
</pre>

I corrected with:
<pre>
if(loc.getProvider()*.equalsIgnoreCase(*LocationManager.GPS_PROVIDER)*)* {
 getLocationManager().removeUpdates(mLocationListener);
 getLocationManager().requestLocationUpdates(LocationManager.GPS_PROVIDER,0, 0, mLocationListener);
}
</pre>
but it acts like I thought: the gps enable, disable, enable, disable...
It's clearly not the good way to stop the network location.

As I said, we should use 2 different mLocationListener in order to kill only the network when the gps has a fix (using something like "getLocationManager().removeUpdates(myNetworkLocationListener);")

Revision history for this message
Murphy (murphy2712+launchpad) wrote :

Sorry for the stars in the previous third code part: the editor doesn't like bold in code.
Here's the apk with the "Location=network" message if you'd like to try.

Revision history for this message
chris_debian (cjhandrew) wrote :

Downloading.

Thanks,

Chris.

Revision history for this message
chris_debian (cjhandrew) wrote :

Yikes, all the OSM trace stuff has disappeared. Did 'Contribute' get commented out?

Chris.

Revision history for this message
Guillaume Rosaire (zerog) wrote :

chris_debian wrote:
> Yikes, all the OSM trace stuff has disappeared. Did 'Contribute' get commented out?
>
> Chris.

The contribute stuff is still not in Subversion I guess, we should merge the patches once it is ready.

When devs attach .apk files, it's apk files generated from their local version which can be in a different revision from one to another.

Revision history for this message
chris_debian (cjhandrew) wrote :

Phew! I look forward to seeing that back :-).

Cheers,

Chris.

Revision history for this message
Murphy (murphy2712+launchpad) wrote :

Zero_G is right, here's your new friend: latest svn + evolvedlight's patch + my network location popup ;)

Revision history for this message
chris_debian (cjhandrew) wrote :

Bug 15 & 50 replaced by more refined bug 56. Assigned to Lead Coder Kieran for closure if satisfied with bug 56.

Chris.

Revision history for this message
Murphy (murphy2712+launchpad) wrote :

My last apk didn't integrate well evolvelight's 'Contribute' patch, use this one instead.
(his patch needs svn 52 to apply)

Revision history for this message
Kieran Fleming (kieran-fleming) wrote :

I've been testing the popup code for a while now and have yet to see it after I have a GPS fix. Can anyone confirm if it happens for them?

Revision history for this message
chris_debian (cjhandrew) wrote :

kizza wrote:
> I've been testing the popup code for a while now and have yet to see it after I have a GPS fix. Can anyone confirm if it happens for them?

I can second Kieran's comment. I have not seen the pop-up after a GPS fix. Is that good or bad?

Chris.

Revision history for this message
chris_debian (cjhandrew) wrote :

Murphy stated the following:

Yes, issues 15 & 50 are already fixed in svn 53.

I've closed 15 and need confirmation for this one, as not sure that it is ready, yet.

Chris.

Revision history for this message
Kieran Fleming (kieran-fleming) wrote :

Yep, I'm happy to close this one. My comment should have been for bug 56.

Revision history for this message
Murphy (murphy2712+launchpad) wrote :

chris_debian wrote:
> kizza wrote:
> > I've been testing the popup code for a while now and have yet to see it after I have a GPS fix. Can anyone confirm if it happens for them?
>
> I can second Kieran's comment. I have not seen the pop-up after a GPS fix. Is that good or bad?
>
> Chris.

I'm sorry, apk in comment#39 already included my patch (bug #56) so you can't see the popup anymore :)
You need earlier version (comment#36 or comment#31) to see the popup bug.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.