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

Report a new location only after receiving a non-null update #77

Merged
merged 4 commits into from
Mar 18, 2016

Conversation

mvglasow
Copy link

Prevents stale locations from being re-sent when backends supply null locations, fixes #75

Signed-off-by: mvglasow <michael -at- vonglasow.com>
@mar-v-in
Copy link
Member

This does not allow to use forced locations (primarily used for debugging currently, but might also be useful for automated testing in the future).

Why don't you just verify that reportLocation is only called after the location was updated (i.e. location to be reported has newer timestamp than last report)?

@mar-v-in
Copy link
Member

Additionally, report(null) is now different from report(old location) which should not be the case (no report, null report and old location report should all be treated the same as they all do mean that there is no new data available)

mvglasow added 2 commits March 18, 2016 09:09
Signed-off-by: mvglasow <michael -at- vonglasow.com>
Signed-off-by: mvglasow <michael -at- vonglasow.com>
@mvglasow
Copy link
Author

3b995b8 modifies BackendHelper to compare timestamps and report locations only if the new location has a more recent timestamp than the previous one.

For forced locations, eead0bd adds code to make them behave in a similar way as a location update from a backend: a forced location which is not more recent than the last forced location is silently discarded. When a new location is forced, reportLocation() gets called, resulting in a single location update. Differences to a backend: forcing a null location will clear the last forced location (which is the same behavior we've had up to now), and forced locations are always asynchronous (there is no equivalent to update() for forced locations).

@@ -78,12 +78,15 @@ public void bind() {
}

public void update() {
Boolean hasUpdates = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be boolean instead of Boolean

Signed-off-by: mvglasow <michael -at- vonglasow.com>
@mvglasow
Copy link
Author

Fixed in 1093f40

mar-v-in added a commit that referenced this pull request Mar 18, 2016
Report a new location only after receiving a non-null update
@mar-v-in mar-v-in merged commit 7b31a55 into microg:master Mar 18, 2016
@n76
Copy link
Contributor

n76 commented Mar 23, 2016

Both of my backends are now causing a crash. From the trace back (below) it seems that I don't have the time set properly on the location report, but I am getting this even if I perform a myLocReport.setTime(System.currentTimeMillis()) immediately before calling report(myLocReport).

I've taken a look at your Apple and Mozilla backends to see what you are doing differently and it appears that the setTime(System.currentTimeMillis()) call is what you use.

Any ideas?

E/AndroidRuntime(22629): Process: org.fitchfamily.android.wifi_backend, PID: 22629
E/AndroidRuntime(22629): java.lang.RuntimeException: Error receiving broadcast Intent { act=android.net.wifi.SCAN_RESULTS flg=0x4000010 } in org.fitchfamily.android.wifi_backend.wifi.WifiReceiver@39f248db
E/AndroidRuntime(22629):    at android.app.LoadedApk$ReceiverDispatcher$Args.run(LoadedApk.java:876)
E/AndroidRuntime(22629):    at android.os.Handler.handleCallback(Handler.java:739)
E/AndroidRuntime(22629):    at android.os.Handler.dispatchMessage(Handler.java:95)
E/AndroidRuntime(22629):    at android.os.Looper.loop(Looper.java:135)
E/AndroidRuntime(22629):    at android.app.ActivityThread.main(ActivityThread.java:5254)
E/AndroidRuntime(22629):    at java.lang.reflect.Method.invoke(Native Method)
E/AndroidRuntime(22629):    at java.lang.reflect.Method.invoke(Method.java:372)
E/AndroidRuntime(22629):    at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:903)
E/AndroidRuntime(22629):    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:698)
E/AndroidRuntime(22629): Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'long android.location.Location.getTime()' on a null object reference
E/AndroidRuntime(22629):    at android.os.Parcel.readException(Parcel.java:1552)
E/AndroidRuntime(22629):    at android.os.Parcel.readException(Parcel.java:1499)
E/AndroidRuntime(22629):    at org.microg.nlp.api.LocationCallback$Stub$Proxy.report(LocationCallback.java:91)
E/AndroidRuntime(22629):    at org.microg.nlp.api.LocationBackendService.report(LocationBackendService.java:52)
E/AndroidRuntime(22629):    at org.fitchfamily.android.wifi_backend.backend.BackendService.process(BackendService.java:186)
E/AndroidRuntime(22629):    at org.fitchfamily.android.wifi_backend.wifi.WifiReceiver.onReceive(WifiReceiver.java:92)
E/AndroidRuntime(22629):    at android.app.LoadedApk$ReceiverDispatcher$Args.run(LoadedApk.java:866)
E/AndroidRuntime(22629):    ... 8 more

@mar-v-in
Copy link
Member

@n76: Use v1.6.4

@n76
Copy link
Contributor

n76 commented Mar 23, 2016

I'm testing against GmsCore v0.2.2, listed as the latest in your downloads and just showing up in your F-Droid repository (I'm using F-Droid to keep up to date with your releases).

FWIW, I am seeing the same crashes and similar stack traces on your Apple and Mozilla backends.

@mar-v-in
Copy link
Member

Please try to download the app again (or update the f-droid repo) there was a broken build online for a few minutes.

@n76
Copy link
Contributor

n76 commented Mar 24, 2016

Apparently that was the problem. . . Not crashing now. Thanks!

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.

Stale location reported when backend calls report(null)
3 participants