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.

Sign up for GitHub

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

Jump to bottom

Android: fused location provider #3447

Merged
merged 4 commits into from
Jun 10, 2024
Merged

Android: fused location provider #3447

merged 4 commits into from
Jun 10, 2024

Conversation

wonder-sk
Copy link
Contributor

@wonder-sk wonder-sk commented May 19, 2024 •
edited
Loading

Initial version of the support for fused location provider:

  • fused provider is opt-in only: nothing should change for the existing users if they do not switch to the "fused" provider. The app remembers whether user wants fused provider or not. The usual position provider is called just "Internal", the fused one is called "Internal (fused)"
  • if the fused location provider is not available (due to Google Play services missing, outdated, disabled or invalid), we detect that and fall back to the internal gps-only provider (based on Qt Positioning)
  • in a future release, we may want to switch to fused location as the default
  • the implementation also adds "Internal (gps)" provider, which is based on Android's LocationManager just like the implementation in Qt Positioning - it is currently left disabled to avoid too many options.
Default (fused available) Default (fused not available) Using fused
Screenshot_20240604_145711 Screenshot_20240604_145022 Screenshot_20240604_145720

@wonder-sk wonder-sk added android 🤖 Relates to Android app version GPS labels May 19, 2024
tomasMizera
tomasMizera reviewed May 20, 2024
View reviewed changes
Copy link
Collaborator

@tomasMizera tomasMizera left a comment

Choose a reason for hiding this comment

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

Brilliant 💎

app/android/src/uk/co/lutraconsulting/MMAndroidPosition.java Show resolved Hide resolved
app/android/src/uk/co/lutraconsulting/MMAndroidPosition.java Show resolved Hide resolved
app/android/src/uk/co/lutraconsulting/MMAndroidPosition.java Outdated Show resolved Hide resolved
app/android/src/uk/co/lutraconsulting/MMAndroidPosition.java Show resolved Hide resolved
@tomasMizera tomasMizera added FROZEN 🥶 do not merge before upcoming release and removed FROZEN 🥶 do not merge before upcoming release labels Jun 3, 2024
@wonder-sk wonder-sk force-pushed the android-fused-location branch from 003023a to 785cd31 Compare June 4, 2024 13:53
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 4, 2024

Pull Request Test Coverage Report for Build 9368402942

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 72 unchanged lines in 4 files lost coverage.
  • Overall coverage increased (+0.04%) to 59.953%
Files with Coverage Reduction New Missed Lines %
input/app/featuresmodel.cpp 1 67.8%
input/core/merginuserinfo.cpp 3 73.76%
input/app/position/providers/positionprovidersmodel.cpp 31 64.81%
input/app/position/positionkit.cpp 37 80.19%
Totals Coverage Status
Change from base Build 9351292521: 0.04%
Covered Lines: 7674
Relevant Lines: 12800
💛 - Coveralls

2 similar comments
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 4, 2024

Pull Request Test Coverage Report for Build 9368402942

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 72 unchanged lines in 4 files lost coverage.
  • Overall coverage increased (+0.04%) to 59.953%
Files with Coverage Reduction New Missed Lines %
input/app/featuresmodel.cpp 1 67.8%
input/core/merginuserinfo.cpp 3 73.76%
input/app/position/providers/positionprovidersmodel.cpp 31 64.81%
input/app/position/positionkit.cpp 37 80.19%
Totals Coverage Status
Change from base Build 9351292521: 0.04%
Covered Lines: 7674
Relevant Lines: 12800
💛 - Coveralls

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 4, 2024

Pull Request Test Coverage Report for Build 9368402942

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 72 unchanged lines in 4 files lost coverage.
  • Overall coverage increased (+0.04%) to 59.953%
Files with Coverage Reduction New Missed Lines %
input/app/featuresmodel.cpp 1 67.8%
input/core/merginuserinfo.cpp 3 73.76%
input/app/position/providers/positionprovidersmodel.cpp 31 64.81%
input/app/position/positionkit.cpp 37 80.19%
Totals Coverage Status
Change from base Build 9351292521: 0.04%
Covered Lines: 7674
Relevant Lines: 12800
💛 - Coveralls

tomasMizera
tomasMizera reviewed Jun 5, 2024
View reviewed changes
Copy link
Collaborator

@tomasMizera tomasMizera left a comment

Choose a reason for hiding this comment

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

Wonderful PR 🥇

app/position/providers/androidpositionprovider.cpp Show resolved Hide resolved
app/position/providers/androidpositionprovider.cpp Show resolved Hide resolved
app/position/providers/androidpositionprovider.cpp Show resolved Hide resolved
app/position/providers/androidpositionprovider.cpp Outdated Show resolved Hide resolved
app/position/providers/androidpositionprovider.cpp Outdated Show resolved Hide resolved
app/position/providers/androidpositionprovider.cpp
Comment on lines +250 to +253
if ( errMsg == "MISSING_PERMISSIONS" )
setState( tr( "No location permissions" ), State::NoConnection );
else if ( errMsg == "FUSED_NOT_AVAILABLE" )
setState( tr( "Fused location not available" ), State::NoConnection );
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shall we have these strings MISSING_PERMISSIONS and FUSED_NOT_AVAILABLE defined as const strings? Alternatively they could be in the AbstractPositionProvider

app/position/positionkit.cpp Show resolved Hide resolved
app/position/providers/androidpositionprovider.cpp Show resolved Hide resolved
app/position/providers/positionprovidersmodel.cpp Show resolved Hide resolved
@wonder-sk
Request location permissions only once needed
adfa992 
(i.e. when project is loaded, not at the startup when position provider is created)
@wonder-sk
Copy link
Contributor Author

@tomasMizera hopefully the last bits should be sorted

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 9, 2024

Pull Request Test Coverage Report for Build 9437629663

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 71 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.005%) to 59.914%
Files with Coverage Reduction New Missed Lines %
input/core/merginuserinfo.cpp 3 73.76%
input/app/position/providers/positionprovidersmodel.cpp 31 64.81%
input/app/position/positionkit.cpp 37 80.19%
Totals Coverage Status
Change from base Build 9351292521: 0.005%
Covered Lines: 7669
Relevant Lines: 12800
💛 - Coveralls

2 similar comments
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 9, 2024

Pull Request Test Coverage Report for Build 9437629663

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 71 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.005%) to 59.914%
Files with Coverage Reduction New Missed Lines %
input/core/merginuserinfo.cpp 3 73.76%
input/app/position/providers/positionprovidersmodel.cpp 31 64.81%
input/app/position/positionkit.cpp 37 80.19%
Totals Coverage Status
Change from base Build 9351292521: 0.005%
Covered Lines: 7669
Relevant Lines: 12800
💛 - Coveralls

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 9, 2024

Pull Request Test Coverage Report for Build 9437629663

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 71 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.005%) to 59.914%
Files with Coverage Reduction New Missed Lines %
input/core/merginuserinfo.cpp 3 73.76%
input/app/position/providers/positionprovidersmodel.cpp 31 64.81%
input/app/position/positionkit.cpp 37 80.19%
Totals Coverage Status
Change from base Build 9351292521: 0.005%
Covered Lines: 7669
Relevant Lines: 12800
💛 - Coveralls

tomasMizera
tomasMizera approved these changes Jun 10, 2024
View reviewed changes
Copy link
Collaborator

@tomasMizera tomasMizera left a comment

Choose a reason for hiding this comment

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

Looking goooood

@tomasMizera
Copy link
Collaborator

CI failures are unrelated

@wonder-sk wonder-sk merged commit c1b96cd into master Jun 10, 2024
10 of 12 checks passed
@tomasMizera tomasMizera added this to the 2024.2.1 milestone Jun 13, 2024
@jozef-budac
Copy link

Verified, I can see the fused location provided according to the screenshot. I also verified that I can switch to it and that the application remembers the user's preference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomasMizera tomasMizera tomasMizera approved these changes

Assignees
No one assigned
Labels
android 🤖 Relates to Android app version GPS
Projects
None yet
Milestone
2024.3.0
Development

Successfully merging this pull request may close these issues.
3 participants
@wonder-sk @tomasMizera @jozef-budac