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

Doesn't work with H10 with Android 11 #246

Open
2 of 10 tasks
KennethEvans opened this issue Mar 4, 2022 · 16 comments
Open
2 of 10 tasks

Doesn't work with H10 with Android 11 #246

KennethEvans opened this issue Mar 4, 2022 · 16 comments
Labels
bug Something isn't working

Comments

@KennethEvans
Copy link

Platform on which you observed the bug:

  • Android
  • iOS
  • Other
  • Platform is not relevant for this bug

Device on which you observed the bug:

  • Polar OH1
  • Polar Verity Sense
  • Polar H10
  • Polar H9
  • Other
  • Device is not relevant for this bug

I got a new Galaxy S22, which runs Android 11. My app, which ran ok on Android 10, is crashing. There are so many messages some are lost in Logcat. At least one problem seems to be

Need android.permission.BLUETOOTH_SCAN permission

The Logcat contents (that weren't truncated) are attached. Note that the Logcat continued filling long after the UI was gone.

The H10 was actually not available during this run. I don't think that matters, though. It ran ok before in that situation, just never connects.

Logcat-2022-03-03.txt

@KennethEvans KennethEvans added the bug Something isn't working label Mar 4, 2022
@JOikarinen
Copy link
Contributor

I see. Are you sure your Galaxy S22 is on Android 11, the error indicates it is running Android 12?

Android 12 introduced the new permissions for bluetooth, like BLUETOOTH_SCAN. If your phone is running Android 12, is your application requesting the permission BLUETOOTH_SCAN from the user?

This is the code snippet from the example app how the permission queries are initialised for example application purposes:

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) {
            requestPermissions(arrayOf(Manifest.permission.BLUETOOTH_SCAN, Manifest.permission.BLUETOOTH_CONNECT), PERMISSION_REQUEST_CODE)
        } else {
            requestPermissions(arrayOf(Manifest.permission.ACCESS_FINE_LOCATION), PERMISSION_REQUEST_CODE)
         }
     } else {
        requestPermissions(arrayOf(Manifest.permission.ACCESS_COARSE_LOCATION), PERMISSION_REQUEST_CODE)
    }
}        

@KennethEvans
Copy link
Author

Thanks for the quick reply. Yes, it's Android 12. Sorry.

No I'm not requesting the permission. I am not scanning. IMHO it should be inherited from the SDK.

It was late last night when I discovered this. I am in the process of fixing all my apps to run on 12. The main problems are with the even more limited acess to the file system. I will get to my own Bluetooth ones today. They should already be set for Android 12, including SCAN permission. If they work, I will add the permissions I used. I still think it should be done in the SDK, but I can do it.

@KennethEvans
Copy link
Author

Ok, I got it to work, but not exactly as it should.

I used the following permissions in AndroidManifest:

    <uses-permission android:name="android.permission.ACCESS_COARSE_LOCATION" />
    <uses-permission android:name="android.permission.ACCESS_FINE_LOCATION" />
    <!-- For below Android 12 -->
    <uses-permission
        android:name="android.permission.BLUETOOTH"
        android:maxSdkVersion="30" />
    <uses-permission
        android:name="android.permission.BLUETOOTH_ADMIN"
        android:maxSdkVersion="30" />
    <!-- For Android 12 and above -->
    <uses-permission android:name="android.permission.BLUETOOTH_CONNECT"
        android:minSdkVersion="31" />
    <uses-permission android:name="android.permission.BLUETOOTH_SCAN"
        android:minSdkVersion="31" />

It still crashed, saying it needed BLUETOOTH_SCAN.

I then manually set the permissions. There were two choices: Location and Nearby devices. I had previously set Location, because it asks for it. I didn't allow Nearby devices as I wasn't asked. I didn't do it manually because I didn't know what it was. It never asked for BLUETOOTH_CONNECT or BLUETOOTH_SCAN. Apparently these are presented to the user as the group Nearby devices.

I do ask for permissions that are not granted at the appropriate time in other of my apps. I don't think I am asking for the above permissions from my code in this app (because there would be no appropriate place to do so). What did happen must come from your code. It isn't asking for Nearby devices (or whatever that includes) though.

There is another issue with Android 12 . I notice that my apps on Android 12 are not calling onDestroy() when the back button is pressed, just onPause(). Consequently the service keeps running (what filled up the logs). I had to implement on BackPressed() and call finish() in there to fix it.

I haven't yet checked that it still works on Android 10, but it probably will.

@KennethEvans
Copy link
Author

And I would like it to not crash if Nearby devices is not granted. That's not a good user experience.

Note that it is not granted the first time after installation.

@JOikarinen
Copy link
Contributor

JOikarinen commented Mar 6, 2022

The SDK needs the permissions, discussed on README, to access bluetooth features of the Android phone. However, SDK cannot launch or actively start the permission request on behalf of the application. That why it is up to the application developer to implement the actually visible requests for the user. Very basic samples given in example app and demo app for permission request, which just follow the Android permission documentation .

And I would like it to not crash if Nearby devices is not granted.

This is actually a bug -> need to be fixed on SDK. Good finding. 👍

@KennethEvans
Copy link
Author

It's asking for location ok. The guidelines are to ask for the others when scanning is about to start as that is when they are needed.

I have discovered my Bluetooth apps have the same problem (crash if not set manually). I set it up while still working with Android 10, so it wasn't really tested. So it's not just with the SDK. The OS should not be crashing apps if permissions are not given.

I have some other things to work on before checking into it. It is not so critical as some of my other problems related to restricted access to the file system, and there is a work around (manually set the permissions). I'll get back.

@JOikarinen
Copy link
Contributor

Back to you original post. The attached logs shows how the Polar BLE SDK has end up to forever loop because of missing BLUETOOTH_SCAN permission. However, I have hard time to reproduce the issue and fix it. I have purposely left the app without BLUETOOTH_SCAN permission and tried different ways to trigger the similar forever loop you reported. I am pretty sure that this is still bug in the SDK, I have seen this the similar logs before.

Bottom line is that SDK shall not end up to forever loop even some of the required permissions are missing. For now pause the investigation, but I will come back to this later if I get the idea of actual root cause.

@KennethEvans
Copy link
Author

I have looked into this some, but don't have it completely figured out yet. In my app the problem is in the service. I had fixed the UI to properly handle permissions, but forgot the service.

The guidelines are to ask the user for ungranted permissions at the time they are needed and only proceed if not granted. The service is not a good place to present a UI to the user, so this doesn't work so well.

The goal seems to be sure the service doesn't crash or do other bad things when the permissions are not granted. And this must be done if the user revokes the permissions while it is running!

So it seems the job of the UI is to handle asking for permissions when needed. In the case of the SDK, since I don't know when it needs them, it seems I should not start the SDK if they are not granted. I am not sure how to handle it when they are revoked in the middle, however. It would seem to involve some notification process from the service to the UI. I am not aware of this in the SDK.

I believe whatever is done must insure the app does not crash but rather gracefully handles these user decisions. Crashing is just not acceptable.

Those are my thoughts as of the moment. It's not a trivial problem.

@KennethEvans
Copy link
Author

Thinking about the forever loop, I'm not sure it is a bug. The new policy in Android 12 is not calling onDestroy when the user touches the back button, but rather to call onPause and leave it not destroyed. This means the service continues to run "forever" as it is not stopped until onDestroy.

This doesn't seem to be highly publicized (nor well thought out), but it seems to be true for all apps. I fixed it by implementing onBackPressed and calling finish. It doesn't continue to run forever now, as it it did when I started this issue.

@KennethEvans
Copy link
Author

I have spent more time than I would have liked understanding permissions. There are a lot of changes with Android 12. I believe I have my app working however. I only tested on Android 10 and 12. I will soon lose the Android 10 device though, since they give me money for trading it in.

This is what is in AndroidManifest

    <!-- For below Android 12 -->
    <uses-permission
        android:name="android.permission.ACCESS_COARSE_LOCATION"
        android:maxSdkVersion="30" />
    <uses-permission
        android:name="android.permission.ACCESS_FINE_LOCATION"
        android:maxSdkVersion="30" />
    <uses-permission
        android:name="android.permission.BLUETOOTH"
        android:maxSdkVersion="30" />
    <uses-permission
        android:name="android.permission.BLUETOOTH_ADMIN"
        android:maxSdkVersion="30" />
    <!-- For Android 12 and above -->
    <uses-permission
        android:name="android.permission.BLUETOOTH_CONNECT"
        android:minSdkVersion="31" />
    <uses-permission
        android:name="android.permission.BLUETOOTH_SCAN"
        android:minSdkVersion="31"
        android:usesPermissionFlags="neverForLocation" />

It is slightly different from what I posted before.

This is what I used in onCreate():

        if (Build.VERSION.SDK_INT >= 31) {
            // Android 12 (S)
            this.requestPermissions(new String[]{
                            Manifest.permission.BLUETOOTH_SCAN,
                            Manifest.permission.BLUETOOTH_CONNECT},
                    REQ_ACCESS_LOCATION);
        } else if (Build.VERSION.SDK_INT >= 23) {
            // Android 6 (M)
            this.requestPermissions(new String[]{
                            Manifest.permission.ACCESS_FINE_LOCATION},
                    REQ_ACCESS_LOCATION);
        }

This is quite a bit different from the one you posted for the example app.

  1. You don't have to request permissions below 23. They are always granted.
  2. I am not sure why it needs FINE location for below 31, but it doesn't return any ECG data if only COARSE is requested.
  3. Note that no location is required for 31, owing to the android:usesPermissionFlags="neverForLocation".

I am not seeing an infinite loop. I think that is owing to the behavior in Android 12 of only pausing the app, not destroying it when the back button is pressed. I don't think it is a bug in the SDK. For completeness, this is what I did:

    @Override
    public void onBackPressed() {
        // This seems to be necessary with Android 12
        // Otherwise onDestroy is not called
        Log.d(TAG, this.getClass().getSimpleName() + ": onBackPressed");
        finish();
        super.onBackPressed();
    }

I tried turning off permissions while the app was running. Granted, it is somewhat contentious to do this, but according to the guidelines, the app should handle it (Note that on Android 12, the only permission group is Nearby devices (no Location).) I tried turning it off while the app was running. It seems to close the app, calling onPause(), but not onDestroy(). The app does not restart. The next time it asks for the permission. I am ok with what it does.

@KennethEvans
Copy link
Author

I spoke too soon. I forgot to actually try it with Nearby devices not granted. When I do that, it crashes.

It does appear to be in an infinite loop and eventually crashes from an OutOfMemoryError. I think this is what happend the first time, when I made the original post. It was probably also what happened when I revoked the permission while it was running, but I have not explored that further.

E/AndroidRuntime: FATAL EXCEPTION: main
    Process: net.kenevans.polar.polarecg, PID: 4546
    java.lang.OutOfMemoryError: OutOfMemoryError thrown while trying to throw OutOfMemoryError; no stack trace available

There are two files attached. The first is from running ADB on the command line. It includes two runs:

03-11 20:53: This was the first time I tried this. The H10 was not available.
03-11 21:07: The same with the H10 attached.

These don't show the OutOfMemoryError. (Errors are not Exceptions, both are Throwables.)

KE.Net-ECG-Crash-2022-03-11.2.txt

The second is from the Run window in Android Studio for the second run. It has overflowed the Run window buffer, which is why I attached the the first one also. You probably only have to look at this one, though.

KE.Net-ECG-Crash-2022-03-11.3.txt

This is happening in the SDK. In simple terms, it seems to be getting an error and continuing to run without notifying the client or doing anything else about it.

I think I have done the right things in my code about handling permissions. It seems to be ok if the permissions are granted. It is not acceptable for it to crash like this if the permissions are not granted, though.

@JOikarinen
Copy link
Contributor

  1. You don't have to request permissions below 23. They are always granted.

👍 run time permissions were introduced in Android 6.0

  1. I am not sure why it needs FINE location for below 31, but it doesn't return any ECG data if only COARSE is requested.

The location permission has been Android platform requirement for the Bluetooth until API 32. It has been really painful to explain why location permission is needed even if user just wants to use bluetooth.

This actually my full understanding of the needed Permissions.

Below Android 6.0:

  • run time permissions: NA
  • manifest permissions:
    • BLUETOOTH
    • BLUETOOTH_ADMIN

Android 6.0 - Android 9.0

  • run time permissions: ACCESS_COARSE_LOCATION
  • and manifest permissions:
    • BLUETOOTH
    • BLUETOOTH_ADMIN

Android 10.0 - Android 11.0

  • run time permissions: ACCESS_FINE_LOCATION
  • and manifest permissions:
    • BLUETOOTH
    • BLUETOOTH_ADMIN

Android 12.0 and above

  • run time permissions: BLUETOOTH_SCAN and BLUETOOTH_CONNECT
  • manifest only permissions: -

You runtime permission request should work just fine. In theory you could add even one more if else, just in case you want to refine permission requests for sub 29 devices :

        } else if (Build.VERSION.SDK_INT >= 29) {
            this.requestPermissions(new String[]{
                            Manifest.permission.ACCESS_FINE_LOCATION},
                    REQ_ACCESS_LOCATION);
        } else if (Build.VERSION.SDK_INT >= 23) {
            this.requestPermissions(new String[]{
                            Manifest.permission.ACCESS_COARSE_LOCATION},
                    REQ_ACCESS_LOCATION);
        }

3. Note that no location is required for 31, owing to the android:usesPermissionFlags="neverForLocation".

Correct. That is the big and good change in Android 31. Attribute android:usesPermissionFlags="neverForLocation" is additional promise by your app that you are not using Bluetooth for deciding the users location.

@KennethEvans
Copy link
Author

I stand corrected. What you say is consistent with this article, which is probably the current definitive one.
https://developer.android.com/guide/topics/connectivity/bluetooth/permissions

I have also verified with my own app that it needs FINE on Android 10 to scan properly. As an aside, that app now works with no permissions granted. It does not crash. It continues to do things that don't require Bluetooth and just does nothing for those that do. This is at the expense of a lot of

        if (Build.VERSION.SDK_INT >= 31 &&
                checkSelfPermission(Manifest.permission.BLUETOOTH_CONNECT) !=
                        PackageManager.PERMISSION_GRANTED) {
            Log.d(TAG, "close: " +
                    "BLUETOOTH_CONNECT not granted");
            return;
        }

all over the code. Some of these were found by Lint, but not all of them.

There is one other issue I don't understand. Currently with the android:minSdkVersion="31" in AndroidManifest, you would think the new permissions would not show on Android 10. However, they do:

This code snippet will get all the requested permissions:

    /**
     * Gets a String with the requested permissions. The ones not granted will
     * be preceded by an X.
     *
     * @param ctx The Context.
     * @return A String with the info.
     */
    public static String getRequestedPermissionsInfo(Context ctx) {
        StringBuilder sb = new StringBuilder();
        sb.append("Requested Permissions").append("\n");
        try {
            PackageInfo pi = ctx.getPackageManager().
                    getPackageInfo(ctx.getPackageName(),
                            PackageManager.GET_PERMISSIONS);
            String[] permissions = pi.requestedPermissions;
            // Note: permissions seems to be  null rather than a
            // zero-length  array if there are no permissions
            if (permissions != null) {
                boolean granted;
                String shortName;
                for (int i = 0; i < permissions.length; i++) {
                    granted = (pi.requestedPermissionsFlags[i] &
                            PackageInfo
                                    .REQUESTED_PERMISSION_GRANTED) != 0;
                    shortName = permissions[i]
                            .substring("android.Permission.".length());
                    sb.append("  ").append(granted ?
                            "" : "X ").append(shortName).append(
                            "\n");
                }
            } else {
                sb.append("    None").append("\n");
            }

            return sb.toString();
        } catch (PackageManager.NameNotFoundException ex) {
            sb.append("   Error finding permissions for ")
                    .append(ctx.getPackageName()).append("\n");
            return sb.toString();
        }
    }

On Android 10 (for an app that is granted all the available permissions), it returns:

    Requested Permissions
      ACCESS_COARSE_LOCATION
      ACCESS_FINE_LOCATION
      BLUETOOTH
      BLUETOOTH_ADMIN
      X BLUETOOTH_CONNECT
      X BLUETOOTH_SCAN

I don't understand this. (It returns what you would expect on Android 12, so the android:maxSdkVersion="30" works as expected.)

@JOikarinen
Copy link
Contributor

I stand corrected. What you say is consistent with this article, which is probably the current definitive one.
https://developer.android.com/guide/topics/connectivity/bluetooth/permissions

Exactly. You are totally correct.

Currently with the android:minSdkVersion="31" in AndroidManifest, you would think the new permissions would not show on Android 10. However, they do:

I cannot confirm that. New permissions BLUETOOTH_CONNECT and BLUETOOTH_SCAN shall only work when your app target is set to 31 and your app is running on Android12 device or above. That's my understanding and also what I see when testing on different phones.

@KennethEvans
Copy link
Author

I cannot confirm that. New permissions BLUETOOTH_CONNECT and BLUETOOTH_SCAN shall only work when your app target is set to 31 and your app is running on Android12 device or above. That's my understanding and also what I see when testing on different phones.

Did you run the code snippet? On my Android 10 device, they are there. However there is no UI to access them as far as I know, and they are not granted. So effectively it is as you say. On my Android 12 device there are only SCAN and CONNECT, and they can be granted or denied. That is what I would expect.

@KennethEvans
Copy link
Author

It is important. The reason is that you have to do something like:

                    if (Build.VERSION.SDK_INT >= 31 &&
                            ActivityCompat.checkSelfPermission(BCMBleService.this,
                                    Manifest.permission.BLUETOOTH_CONNECT) !=
                                    PackageManager.PERMISSION_GRANTED) {
                        Log.d(TAG, "onConnectionStateChange: " +
                                "BLUETOOTH_CONNECT not granted");
                        do something...
                    } else {
                        do something else...
                    }

Without the build check it does "something" not "something else" on Android versions less than 12, because the permissions are there but not granted on those devices (or at least on my own Android 10 device, the only one I've checked).

You should do the build check anyway, but the Lint suggested code does not include that, and it works without it, just incorrectly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants