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

Handle permissions and gps status #17

Open
wants to merge 1 commit into
base: 2.1.x
Choose a base branch
from

Conversation

michalfapso
Copy link

This patch contains two changes:

  • Return error when permissions are not set properly for wifi access.
  • In Android 6 and above, wifi networks scanning return empty SSID list
    when GPS is turned off, so now it also asks to turn on GPS if it is disabled.

All these changes were merged from https://github.com/paraggarg37/WifiWizard/

Specifically from these commits:

Why Should This Be In Core?

I used WifiWizard on my Android 5 without problems, but when I deployed it on other people's phones for testing, it didn't work on some phones. It turned out to be either because of unset permissions or disabled GPS on Android >= 6. So these changes could save some troubles to WifiWizard users :)

Of course, one could use other plugins for checking permissions and GPS status, but I think it is nice to have it all included in WifiWizard itself.

Code parts to check

I was not sure, where to put these checks, so please just check if that makes sense to you:

public boolean execute(...) {
        ...
        if(!displayLocationSettingsRequest()){
            callbackContext.error("Android 6 and above Gps turned off");
            return true;
        }

        if(!checkCurrentPermissions()){
            callbackContext.error("permission not found") ;
            return true;
        }

Thanks a lot for maintaining WifiWizard2.
Michal

Return error when permissions are not set properly for wifi access.

In Android 6 and above, wifi networks scanning return empty SSID list
when GPS is turned off, so now it asks to turn on GPS if it is disabled.

All these changes were merged from:
https://github.com/paraggarg37/WifiWizard/

Specifically from these commits:
- added permissions for marshmallow
paraggarg37/WifiWizard@47590ea#diff-71da7ed529b38209b2b0f4bfc226b3ec
- added support for android 6
paraggarg37/WifiWizard@eb2b9a6#diff-71da7ed529b38209b2b0f4bfc226b3ec
@tripflex
Copy link
Owner

Thanks for submitting this PR, have you tried the version 3.0.0 branch by chance? Permission handling was added in that branch.

I don't mind adding this for support in the old version of WifiWizard (version 2), but have you tested this yourself? If so what have you tested it on?

@michalfapso
Copy link
Author

Thanks, I have tested it with the 3.0.0 branch, but it didn't work the same way. I don't remember now, how it behaved, but I'll try it soon again and let you know.

@tripflex tripflex changed the base branch from master to 2.1.x April 12, 2018 19:25
@tripflex
Copy link
Owner

Ok yeah this should only be for version 2.x so I changed the PR to that instead of master, as i'm about to merge 3.x into master since 2.x will no longer be supported/maintained in favor of 3.x+

@michalfapso
Copy link
Author

Sure, that makes sense.

@gontard
Copy link

gontard commented May 29, 2018

I am interested by this PR too. Any news ?

@michalfapso
Copy link
Author

Sorry guys, I haven't tested the 3.0.0 branch yet.

@gontard, could you please test it? If you have Android >= 6, just create an example app with WifiWizard2 (branch 3.0.0) and try wifi scanning when the app doesn't have permissions to access location and when the GPS is turned off.

@arsenal942
Copy link
Collaborator

@tripflex IS this still relevant?

@tripflex
Copy link
Owner

@arsenal942 only for the 2.1.x branch, this is already handled in the 3.x branch

@tripflex tripflex added needs testing 2.x.x Anything related to the 2.x.x versions labels Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.x.x Anything related to the 2.x.x versions needs testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants