gjedeer / mylocation

Share your location on Android - with Email, SMS, Conversations, Tox, Whatsapp etc
https://f-droid.org/repository/browse/?fdid=net.mypapit.mobile.myposition
GNU General Public License v2.0
29 stars 14 forks source link

Add Open Location Code support #23

Closed goblin closed 4 years ago

goblin commented 4 years ago

Closes https://github.com/gjedeer/mylocation/issues/22.

This branch adds the relevant functionality.

However, I could not compile the project in the latest Android Studio as-is - Gradle update was required for some reason. To get the changes that I made to get it to compile on my box, check out my OLC branch. I'm not exactly sure what these changes do or whether you want them that way (most importantly - I don't have the signing keys so I commented them out in app/build.gradle).

goblin commented 4 years ago
Could not read /opt/android/sdk/platform-tools/api/annotations.zip
java.io.IOException: Could not parse XML from android/accounts/annotations.xml

I believe the failing tests are not related to my changes as I didn't touch these files.

They're most likely due to Gradle outdatedness or some other weirdness.

goblin commented 4 years ago

Ah no, perhaps they are my fault:

Dex: Error converting bytecode to dex:
Cause: Dex cannot parse version 52 byte code.
This is caused by library dependencies that have been compiled using Java 8 or above.
If you are using the 'java' gradle plugin in a library submodule add 
targetCompatibility = '1.7'
sourceCompatibility = '1.7'
to that submodule's build.gradle file.
...while parsing com/google/openlocationcode/OpenLocationCode.class
    PARSE ERROR:
    unsupported class file version 52.0
    ...while parsing com/google/openlocationcode/OpenLocationCode.class

I've never heard of Dex or CircleCI and whatnots... Perhaps you know how to fix this?

gjedeer commented 4 years ago

I just tried building it and:

  1. Including the OpenLocationCode.java file in sources builds just fine in Java 1.7 (the error you got was because the jar in Maven Central requires Java 1.8). I don't want to bump Java version because it is a very simple application and there's no reason why we should break compatiblity with older Android devices. Just change the package name in source to net.mypapit.mylocation. Ugly workaround but a woring one.
  2. About annotations.zip. no idea
  3. I've messed up the gradle file massively, but the quick way to build this app locally is:

CIRCLECI=1 ./gradlew assembleDebug

gjedeer commented 4 years ago

In fact I've just done that myself and pushed the changes. Good job, nice addition to the app. I'm going to go ahead and release it when I have a few more minutes.

gjedeer commented 4 years ago

Whoops, the app crashes when resumed:

11-02 13:38:59.443 E/AndroidRuntime(8938): java.lang.RuntimeException: Unable to resume activity {net.mypapit.mobile.myposition/net.mypapit.mobile.myposition.MyLocationActivity}: java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.String net.mypapit.mobile.myposition.OpenLocationCode.getCode()' on a null object reference

11-02 13:38:59.443 E/AndroidRuntime(8938): Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.String net.mypapit.mobile.myposition.OpenLocationCode.getCode()' on a null object reference```

11-02 13:38:59.443 E/AndroidRuntime(8938):  at net.mypapit.mobile.myposition.MyLocationActivity$2.run(MyLocationActivity.java:348)

11-02 13:38:59.443 E/AndroidRuntime(8938):  at net.mypapit.mobile.myposition.MyLocationActivity.registerLocationListener(MyLocationActivity.java:343)

11-02 13:38:59.443 E/AndroidRuntime(8938):  at net.mypapit.mobile.myposition.MyLocationActivity.checkLocationPermission(MyLocationActivity.java:168)

 11-02 13:38:59.443 E/AndroidRuntime(8938):     at net.mypapit.mobile.myposition.MyLocationActivity.onResume(MyLocationActivity.java:142)  
gjedeer commented 4 years ago

Looks like just constructing the object from (lat,lon) instead of caching it may be an easy fix here.

goblin commented 4 years ago

Hmm, quite possibly the lat and lon equal 0.0 at that point (initialized in MyLocationActivity.java:324), so it's probably better to check for null instead of constructing an invalid OLC (I didn't notice any crashing, however).

goblin commented 4 years ago

Thanks for merging! :-)