Skip to content

Bugfix/issue 1812 Android 13 Support #1826

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

Merged
merged 7 commits into from
Sep 6, 2022
Merged

Bugfix/issue 1812 Android 13 Support #1826

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8dd082d
Update target and compile sdk to api 33 for sdl_android and hello sdl…
JulianKast Aug 25, 2022
70754e7
Add POST_NOTIFICATION permission to hello_SDL
JulianKast Aug 25, 2022
7ba0d67
Add comments for denying permission
JulianKast Aug 29, 2022
399f373
Formatting fixes
JulianKast Aug 30, 2022
bf7e660
Review updates
JulianKast Aug 31, 2022
cb1c48e
Flip permission methods and rename them
JulianKast Sep 4, 2022
881bbc2
Check local array to see if we need bluetooth connect permission
JulianKast Sep 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions android/hello_sdl_android/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
apply plugin: 'com.android.application'

android {
compileSdkVersion 31
compileSdkVersion 33
defaultConfig {
applicationId "com.sdl.hellosdlandroid"
minSdkVersion 16
targetSdkVersion 31
targetSdkVersion 33
versionCode 1
versionName "1.0"
testInstrumentationRunner 'androidx.test.runner.AndroidJUnitRunner'
Expand Down
2 changes: 2 additions & 0 deletions android/hello_sdl_android/src/main/AndroidManifest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
<uses-permission android:name="android.permission.BLUETOOTH" />
<uses-permission android:name="android.permission.BLUETOOTH_CONNECT"
tools:targetApi="31"/>
<uses-permission android:name="android.permission.POST_NOTIFICATIONS"
tools:targetApi="33"/>
<uses-permission android:name="android.permission.INTERNET" />
<!-- Required to check if WiFi is enabled -->
<uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" />
Expand Down
63 changes: 47 additions & 16 deletions android/hello_sdl_android/src/main/java/com/sdl/hellosdlandroid/MainActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
package com.sdl.hellosdlandroid;

import android.Manifest;
import android.content.Intent;
import android.content.pm.PackageManager;
import android.os.Build;
import android.os.Bundle;
import android.view.Menu;
import android.view.MenuItem;

import androidx.annotation.NonNull;
import androidx.appcompat.app.AppCompatActivity;
import androidx.core.app.ActivityCompat;
import androidx.core.content.ContextCompat;

import static android.Manifest.permission.BLUETOOTH_CONNECT;
import java.util.ArrayList;

public class MainActivity extends AppCompatActivity {

Expand All @@ -23,12 +22,15 @@ protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.activity_main);


if (BuildConfig.TRANSPORT.equals("MULTI") || BuildConfig.TRANSPORT.equals("MULTI_HB")) {
if (android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.S && !checkPermission()) {
requestPermission();
return;
String[] permissionsNeeded = permissionsNeeded();
if (permissionsNeeded.length > 0) {
requestPermission(permissionsNeeded(), REQUEST_CODE);
Copy link
Member

@joeygrover joeygrover Aug 31, 2022

Choose a reason for hiding this comment

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

Suggested change
requestPermission(permissionsNeeded(), REQUEST_CODE);
requestPermission(permissionsNeeded, REQUEST_CODE);

if (checkBTPermission()) {
return;
}
Copy link
Member

@joeygrover joeygrover Aug 31, 2022

Choose a reason for hiding this comment

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

Is there a reason this is still needed? Seems like we could get rid of it.

Suggested change
if (checkBTPermission()) {
return;
}

Copy link
Contributor Author

@JulianKast JulianKast Aug 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This is needed because we need to not call SdlReceiver.queryForConnectedService(this); below only if we need the BLUETOOTH_CONNECT permission.

We could change it below at line 35 to

            if (!checkBTPermission()) {
                //If we are connected to a module we want to start our SdlService
                SdlReceiver.queryForConnectedService(this);
            }

Copy link
Member

@joeygrover joeygrover Aug 31, 2022

Choose a reason for hiding this comment

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

With having an array with strings that contain our permission needs already it seems like we could check those with something like:

for (String permission : permissionsNeeded) {
    if(Manifest.permission.BLUETOOTH_CONNECT.equals(permission)){
        return;
    }
}

I just don't know the efficiency of checking for the permission again, and it seems redundant. At the very least I would say your suggestions should be used as it's bit more concise.

Copy link
Member

@joeygrover joeygrover Aug 31, 2022

Choose a reason for hiding this comment

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

Another option would be to flip the methods for checking permission into returning if the permission is granted instead of currently returning if it is not granted, because that in itself is a big confusing as well.

Copy link
Contributor Author

@JulianKast JulianKast Sep 4, 2022

Choose a reason for hiding this comment

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

I agree that it was confusing especially with the naming of those methods, I flipped them and renamed to hasPNPermission and hasBTPermission. I wanted to not flip them and name them needPNPermision and needBTPermission since its checking the API level and then the permission, But need really isn't a proper java naming convention that I could find, so I added some JavDocs to them.

}

//If we are connected to a module we want to start our SdlService
SdlReceiver.queryForConnectedService(this);
} else if (BuildConfig.TRANSPORT.equals("TCP")){
Expand All @@ -37,24 +39,53 @@ protected void onCreate(Bundle savedInstanceState) {
}
}

private boolean checkPermission() {
return PackageManager.PERMISSION_GRANTED == ContextCompat.checkSelfPermission(getApplicationContext(), BLUETOOTH_CONNECT);
private boolean checkBTPermission() {
return Build.VERSION.SDK_INT >= Build.VERSION_CODES.S && !checkPermission(Manifest.permission.BLUETOOTH_CONNECT);
}

private boolean checkPNPermission() {
return Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU && !checkPermission(Manifest.permission.POST_NOTIFICATIONS);
}

private void requestPermission() {
ActivityCompat.requestPermissions(this, new String[]{BLUETOOTH_CONNECT}, REQUEST_CODE);
private boolean checkPermission(String permission) {
return PackageManager.PERMISSION_GRANTED == ContextCompat.checkSelfPermission(getApplicationContext(), permission);
}

private void requestPermission(String[] permissions, int REQUEST_CODE) {
ActivityCompat.requestPermissions(this, permissions, REQUEST_CODE);
}

private @NonNull String[] permissionsNeeded() {
ArrayList<String> result = new ArrayList<>();
if (checkBTPermission()) {
result.add(Manifest.permission.BLUETOOTH_CONNECT);
}
if (checkPNPermission()) {
result.add(Manifest.permission.POST_NOTIFICATIONS);
}
return (result.toArray(new String[result.size()]));
}

@Override
public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) {
switch (requestCode) {
case REQUEST_CODE:
if (grantResults.length > 0) {

boolean btConnectGranted = grantResults[0] == PackageManager.PERMISSION_GRANTED;

if (btConnectGranted) {
SdlReceiver.queryForConnectedService(this);
for (int i = 0; i < grantResults.length; i++) {
if (permissions[i].equals(Manifest.permission.BLUETOOTH_CONNECT)) {
boolean btConnectGranted =
grantResults[i] == PackageManager.PERMISSION_GRANTED;
if (btConnectGranted) {
SdlReceiver.queryForConnectedService(this);
}
} else if (permissions[i].equals(Manifest.permission.POST_NOTIFICATIONS)) {
boolean postNotificationGranted =
grantResults[i] == PackageManager.PERMISSION_GRANTED;
if (!postNotificationGranted) {
// User denied permission, Notifications for SDL will not appear
// on Android 13 devices.
}
}
}
}
break;
Expand Down
4 changes: 2 additions & 2 deletions android/sdl_android/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
apply plugin: 'com.android.library'

android {
compileSdkVersion 31
compileSdkVersion 33
defaultConfig {
minSdkVersion 16
targetSdkVersion 31
targetSdkVersion 33
versionCode 23
versionName new File(projectDir.path, ('/../../VERSION')).text.trim()
buildConfigField "String", "VERSION_NAME", '\"' + versionName + '\"'
Expand Down