Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip known-unreasonable network location updates #2367

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

t-m-w
Copy link
Contributor

@t-m-w t-m-w commented May 9, 2024

If we have a GPS location of a given accuracy, and our proposed network location is more than twice as far away as the GPS accuracy, this is considered unreasonable. Stop sending that network location.

As part of this, also keep track of lower-accuracy GPS locations with accuracy as low as 10km.

@mar-v-in
Copy link
Member

mar-v-in commented May 9, 2024

  • I don't think a buffer of GPS locations make sense in this context. It should be enough to just store the latest GPS location and then compare to it if it's recent enough. Doing so will significantly reduce the complexity of the change. The current use of the buffer will also mean that every location will become "reasonable" if it is old enough (aka the buffer is overrun and there is no reference location anymore to compare against). However we don't want to say anything about how reasonable an outdated location was, we only care about how reasonable it is as a current location.
    About the GPS location buffer

    The current GPS location buffer (with accuracy requirements) is meant for learning network locations (and in the future also to submit them to a server). In that case we need the buffer as a reported wifi hotspot might be from a beacon received a few seconds ago, which when travelling at high speeds can mean an error of up to a few hundred meters.

  • I am unsure if locations should be restricted at the network location provider if they mismatch GPS. I would assume it's the task of the network location provider to provide whatever it considers reasonable as a current network location, no matter what the GPS says. It's the purpose of a fused / hybrid location provider to understand what the current location could be in cases where GPS and network data (some of which might be outdated) is available. I would rather want to put more logic like this into our fused location manager (org.microg.gms.location.manager).

@t-m-w
Copy link
Contributor Author

t-m-w commented May 13, 2024

Sounds good, thanks for all the info; happy to look into moving things around and following those suggestions.

You had mentioned the fused location provider before, but I was having trouble identifying it; knowing that it's org.microg.gms.location.manager should be helpful.

@t-m-w t-m-w closed this May 13, 2024
If we have a recent GPS location of a given accuracy, and our proposed
network location is more than twice as far away as the GPS accuracy
(but at least 1 kilometer away from the GPS location), this is
considered unreasonable. Stop sending that network location.

Change-Id: I1893c63c2d716a5e41011cbf39f069491e6b5da6
@t-m-w t-m-w reopened this May 13, 2024
@t-m-w t-m-w marked this pull request as draft May 13, 2024 19:11
@t-m-w t-m-w marked this pull request as ready for review May 13, 2024 19:29
@ale5000-git
Copy link
Member

ale5000-git commented May 13, 2024

@t-m-w
The min/max functions usually return the opposite of what one expect :p
So if you use the "min" function you should pass the MAX_UNREASONABLE_DISTANCE_FROM_GPS.

@t-m-w
Copy link
Contributor Author

t-m-w commented May 13, 2024

* I am unsure if locations should be restricted at the network location provider if they mismatch GPS. I would assume it's the task of the network location provider to provide whatever it considers reasonable as a current network location, no matter what the GPS says. It's the purpose of a fused / hybrid location provider to understand what the current location could be in cases where GPS and network data (some of which might be outdated) is available. I would rather want to put more logic like this into our fused location manager (`org.microg.gms.location.manager`).

Is it the case that not all apps use the fused location provider? I've been trying to test this new branch with Android 14 and I don't see the log lines at all when using SatStat or Organic Maps. Added some extra ones locally and I don't see them either.

It would be a shame for the network location to be this far off (thousands of miles, here) and have some apps never be able to recover from that. Then again, this change's only goal was to fix it once GPS is known, not permanently; it doesn't resolve the recurring annoyance when all you have is network location, so maybe it's not really a worthwhile approach...?

@t-m-w
Copy link
Contributor Author

t-m-w commented May 13, 2024

@t-m-w The min/max functions usually return the opposite of what one expect :p So if you use the "min" function you should pass the MAX_UNREASONABLE_DISTANCE_FROM_GPS.

Oh, yep, I've done it again... Thanks for pointing that out!

@ale5000-git
Copy link
Member

ale5000-git commented May 13, 2024

Even if the improvement doesn't cover everything it is still a good improvement.

An app can:

  1. Use fused location from Google Play Services (this can be controlled directly by microG and works in every case);
  2. Use fused location from Android directly (this cannot be controlled directly and it works only if microG is registered as location provider);
  3. Use network location from Android directly (it works only if microG is registered as location provider; it is possible that we can still skip wrong results but I'm not sure);
  4. Use GPS location from Android directly (this should never reach microG if I'm not wrong).

This is what I have understood.
@mar-v-in
Correct me if I get something wrong.

@t-m-w
Copy link
Contributor Author

t-m-w commented May 14, 2024

Any recommendations for an app that would take advantage of the approach in the current commits so that I can test it?

@Misalf-git
Copy link

FWIW, Organic Maps location jumps from Network to GPS occasionally (I guess).

@mar-v-in
Copy link
Member

mar-v-in commented May 14, 2024

Apps that have their own location fusing logic (and I assume Organic Maps is one of them) are not affected by this change (and in fact can't really be fixed by us).

The underlying problem is how to understand the accuracy information in a location reported. It is often simplified that the real location is in a circle with a radius of accuracy meters centered around the location given. This is not correct, the real location is only within this circle in 2 out of 3 cases (68th percentile). The remaining cases, it is outside this circle, and this is where the bigger problem is: With GPS, it is never more than a few hundred meters from the given location (no matter the accuracy given), du to how GPS works. With network-based location, as the database might have wrong data, it can be on the other side of the planet.

Now location fusing is what combines network based location and gps based location to a single location, incorporating any metadata from each of the location sources. The basic implementation in AOSP has the simple logic of taking either GPS or network location, if one of them is 11s or more newer than the other, that one will be taken, if they're less than 11s from each other, the one with reported higher accuracy is taken. If apps take this implementation (by requesting fused location from AOSP based systems), they will likely see those jumps, same if they have their own, similarly naive implementations.

We do need a much more sophisticated implementation if we want to compete with Google's service - which is what apps expect. Ignoring locations from the network provider that are implausible given the location information we have from GPS is a good start, but ultimately, we also want to logically merge the information where possible.

@t-m-w
Copy link
Contributor Author

t-m-w commented May 30, 2024

I understand the idea that apps should be smart about how they fuse GPS and network location, if they choose to do it on their own, but in reality that doesn't seem to be the case, and such apps appear to work fine with reasonable network location providers but appear to be broken with microG. This is what makes me eager for a fix (or at least a workaround) in the network location provider, if the source of the provider's data is already known to be inaccurate in a large number of cases and with no solution on the horizon.

At this rate, Mozilla Location Services will be shut down soon (some day in June I think - I don't remember the date off-hand), so we might be able to get away with postponing this without it making much of a difference.

@mar-v-in
Copy link
Member

Can you provide examples of apps that behave peculiar bad with microG location (e.g. a lot of jumping between locations)? So we can better understand in how far this behavior differs in Google and why.

If we just filter out locations from network location so that apps that do fuse can do this better, apps that don't fuse but only request network location won't get updates, which might be worse than giving them inaccurate updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants