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

(Android) enableGPS does not attempt to enable GPS due to incorrect isGPSEnabled check #266

Open
delaneyb opened this issue Feb 11, 2023 · 1 comment

Comments

@delaneyb
Copy link

delaneyb commented Feb 11, 2023

enableGPS uses the check

if (!this.isGPSEnabled())

to determine whether we need to prompt the user to enable location services on the device. isGPSEnabled however returns a Promise, meaning the non-awaited returned value is always truthy, and therefore enableGPS will never actually actually prompt the user to enable location services, even if the promise returned by isGPSEnabled resolves to false

ble/src/ble/index.android.ts

Lines 1277 to 1300 in 9820235

public enableGPS(): Promise<void> {
if (Trace.isEnabled()) {
CLog(CLogTypes.info, 'Bluetooth.enableGPS');
}
return new Promise((resolve, reject) => {
const activity = andApp.foregroundActivity || andApp.startActivity;
if (!this.isGPSEnabled()) {
const onActivityResultHandler = (data: AndroidActivityResultEventData) => {
andApp.off(AndroidApplication.activityResultEvent, onActivityResultHandler);
if (data.requestCode === 0) {
if (this.isGPSEnabled()) {
resolve();
} else {
reject('GPS not enabled');
}
}
};
andApp.on(AndroidApplication.activityResultEvent, onActivityResultHandler);
activity.startActivityForResult(new android.content.Intent(android.provider.Settings.ACTION_LOCATION_SOURCE_SETTINGS), 0);
} else {
resolve();
}
});
}

A simple solution would be to make the promise constructor callback async and adjust the condition to:

if (!(await this.isGPSEnabled()))

The check after receiving the request result from the system on line 1287 also needs to be corrected

if (this.isGPSEnabled()) {

@farfromrefug
Copy link
Member

@delaneyb thanks for this. Tbh i dont use that part of the plugin. I always use the perms plugin and move the logic in the app. The reason is that the logic is constantly changing and getting more complex as new os versions are released
I even think of removing that part and let the user handles it with the perms plugin.
However if you want to make a PR to fix all this I would gladly merge it

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

No branches or pull requests

2 participants