android / location-samples

Multiple samples showing the best practices in location APIs on Android.
Apache License 2.0
2.7k stars 2.77k forks source link

Geofencing sample: build upgrades #317

Closed jdkoren closed 1 year ago

jdkoren commented 1 year ago

Change-Id: I6eebf318bfe8a04c721175a7c8ba9a0179c08c61

javadude commented 1 year ago

Also - IIRC there were several things in the code that needed to change, possibly for updated other dependencies. Not sure what they were, but I know a full upgrade was pretty painful. Might have been wrt updating permissions to the registerActivityResult form?

jdkoren commented 1 year ago

Also - IIRC there were several things in the code that needed to change, possibly for updated other dependencies. Not sure what they were, but I know a full upgrade was pretty painful. Might have been wrt updating permissions to the registerActivityResult form?

Yes, there are definitely a lot of things which need fixing. Doing all of that at once would make for a very large PR, so I just tried to do the minimum that would build with the updated Gradle and AGP versions.

javadude commented 1 year ago

Was just testing it out and got

Caused by: java.lang.IllegalArgumentException: com.google.android.gms.location.sample.geofencing: Targeting S+ (version 31 and above) requires that one of FLAG_IMMUTABLE or FLAG_MUTABLE be specified when creating a PendingIntent.
Strongly consider using FLAG_IMMUTABLE, only use FLAG_MUTABLE if some functionality depends on the PendingIntent being mutable, e.g. if it needs to be used with inline replies or bubbles.

when trying to add a geofence. Looks familiar. I started to go down the route of making changes (adding PendingIntent.FLAG_IMMUTABLE and updating the min SDK to 23 and updating dependencies) and it keeps saying

W/MainActivity: Unknown error: the Geofence service is not available now

Did it run for you? I'm running on a Pixel 7 with Android 13

jdkoren commented 1 year ago

My goal was just for it to build with Gradle. This sample is pretty stale and I don't expect it to run properly without several more changes.

javadude commented 1 year ago

Ok - I just verified that the existing app main branch builds but has the same error when running. Should we just remove it?