Skip to content

Fix issues around location in nearby activity #983

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

Merged
merged 4 commits into from
Dec 2, 2017

Conversation

maskaravivek
Copy link
Member

@maskaravivek maskaravivek commented Nov 25, 2017

  • Fixes the issue where nearby list was not loaded in the first attempt.

How to reproduce?

  • Initially phone's GPS is off or the Commons app doesn't have location permissions.
  • Either of these things are provided at run time.
  • The nearby list just keeps loading.
  • It loads after killing the app.

The above issue is now fixed.

  • App was continuously listening for location updates and draining a lot of battery

How do you know?

  • The location Setting page in the your phone tags Commons app as high battery usage application.

The above should be hopefully fixed.

  • The app now stops listening for location updates as soon as the nearby list is populated.

  • It again registers the location manager when the refresh button is clicked.

  • App now uses multiple providers to fetch the location.

Why?

GPS_PROVIDER is usually slower. Now the app would be using both NETWORK_PROVIDER for coarse location and GPS_PROVIDER for finer location. Upon receiving a location update, it will determine if the newer location is better than the last location. If it is, the newer location would be used.

Also fixes #982.

@commons-app commons-app deleted a comment Nov 25, 2017
@maskaravivek
Copy link
Member Author

Will be fixing #978 after this is merged.

}

// private void turnGPSOn(Context context){
Copy link
Member

@misaochan misaochan Dec 1, 2017

Choose a reason for hiding this comment

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

Could I ask why this block is commented out?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was trying out something. It doesn't matter now. We have @psh's test to replace ours.

@misaochan
Copy link
Member

Thanks for submitting this, @maskaravivek ! I'm hoping to test it this weekend if I can, and then maybe we could include it in our release (planned for early next week). Some conflicts need to be resolved though, after the recent test PR merge.

Copy link
Member

@misaochan misaochan left a comment

Choose a reason for hiding this comment

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

Tested this on real device running Android 7.0 and works well for me. Happy to merge once conflicts are resolved. :)

@misaochan
Copy link
Member

Tested this on real device running Android 7.0 and works well for me. Happy to merge once conflicts are resolved. :)

@commons-app commons-app deleted a comment Dec 2, 2017
@maskaravivek
Copy link
Member Author

@misaochan I have rebased the branch.

@commons-app commons-app deleted a comment Dec 2, 2017
@misaochan misaochan merged commit f8b389e into commons-app:master Dec 2, 2017
@maskaravivek maskaravivek deleted the nearbyRefactoring branch September 12, 2018 20:24
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.

Optional Permissions in Nearby Places activity?
2 participants