Feature/android 12 fixes by RHenigan · Pull Request #1771 · smartdevicelink/sdl_java_suite

RHenigan · 2021-11-30T18:38:56Z

Fixes #1732, #1751 and #1794

This PR is ready for review.

Risk

This PR makes major API changes.

Testing Plan

I have verified that I have not introduced new warnings in this PR (or explain why below)
I have run the unit tests with this PR
I have tested this PR against Core and verified behavior (if applicable, if not applicable, explain why below).
I have tested Android, Java SE, and Java EE

Unit Tests

N/A

Core Tests

Extensive smoke testing was done that is reflected in the Release Testing Template.
Targeting Android 11 should result in the same behavior as before
Target Android 12 tests include:
Connecting to SYNC over USB with BT permissions denied

Router Service Should Start as expected and a notification should be presented to grant permissions

Connecting to SYNC over BT with BT permissions denied

The Router Service should not be started

Connecting to SYNC over USB with BT permissions granted

Router Service Should Start as expected and a notification should NOT be presented to grant permissions

Connecting to SYNC over BT with BT permissions granted

Router Service Should Start as expected

Connecting to SYNC over USB with BT permissions denied, granting permissions once connected, then connecting BT.

Once BT connects, Router service should be connected over BT and USB

Connecting to SYNC with multiple apps targeting android 12 (one app with BT permissions granted, one with BT permissions denied).

Router Service should be started for app with permissions, verify the app without permissions still connects to the router service.

Core version / branch / commit hash / module tested against: SYNC3
HMI name / version / branch / commit hash / module tested against: SYNC3

Summary

This PR addresses the issues that have been introduced by targeting Android 12 as outlined in this proposal (0345).

This includes:

CLA

I have signed the CLA

…/sdl_java_suite into feature/android_12_fixes

codecov · 2021-11-30T18:47:05Z

Codecov Report

Merging #1771 (03a0682) into develop (b51cda0) will decrease coverage by 0.66%.
The diff coverage is 2.27%.

@@              Coverage Diff              @@
##             develop    #1771      +/-   ##
=============================================
- Coverage      54.39%   53.72%   -0.67%     
- Complexity      5516     5537      +21     
=============================================
  Files            562      562              
  Lines          25543    26070     +527     
  Branches        3328     3481     +153     
=============================================
+ Hits           13893    14006     +113     
- Misses         10391    10796     +405     
- Partials        1259     1268       +9

Impacted Files	Coverage Δ
...link/transport/USBAccessoryAttachmentActivity.java	`0.00% <0.00%> (ø)`
...in/java/com/smartdevicelink/util/AndroidTools.java	`19.17% <0.00%> (-1.72%)`	⬇️
.../com/smartdevicelink/protocol/SdlProtocolBase.java	`15.71% <0.00%> (-0.04%)`	⬇️
.../smartdevicelink/transport/TransportConstants.java	`0.00% <ø> (ø)`
...om/smartdevicelink/transport/SdlRouterService.java	`10.71% <2.19%> (-0.39%)`	⬇️
...martdevicelink/transport/SdlBroadcastReceiver.java	`3.13% <4.00%> (+0.09%)`	⬆️
...ink/managers/screen/BaseTextAndGraphicManager.java	`64.58% <0.00%> (+0.83%)`	⬆️
...elink/managers/lifecycle/BaseLifecycleManager.java	`16.19% <0.00%> (+2.20%)`	⬆️
...artdevicelink/transport/utl/SdlDeviceListener.java	`10.41% <0.00%> (+2.48%)`	⬆️
...smartdevicelink/protocol/SecurityQueryPayload.java	`78.84% <0.00%> (+5.76%)`	⬆️
... and 1 more

joeygrover

With the addition of in line comments, you also need to update the bluetoothAvailable() method in SdlRouterService to check for bluetooth permissions. Currently the RS will check for BT availability when deciding if it should stay open and if the RS doesn't have BT permissions then it shouldn't.

android/hello_sdl_android/src/main/java/com/sdl/hellosdlandroid/MainActivity.java

android/hello_sdl_android/src/main/java/com/sdl/hellosdlandroid/SdlReceiver.java

android/sdl_android/src/main/java/com/smartdevicelink/util/AndroidTools.java

android/sdl_android/src/main/res/values/strings.xml

android/sdl_android/src/main/java/com/smartdevicelink/transport/SdlRouterService.java

joeygrover · 2022-02-23T16:29:12Z

android/sdl_android/src/main/res/values/strings.xml

    <string name="lockscreen_device_image_description">Device Icon</string>
    <string name="default_lockscreen_warning_message">Swipe down to dismiss, acknowledging that you are not the driver.</string>
    <string name="spp_out_of_resource">Too many apps are using Bluetooth</string>
+    <string name="allow_bluetooth_permissions">Please grant this app bluetooth permissions</string>


Suggested change

<string name="allow_bluetooth_permissions">Please grant this app bluetooth permissions</string>

<string name="allow_bluetooth_permissions">Please grant this app the Nearby Devices Permission to use bluetooth</string>

The permission name from Android isn't specific to bluetooth so we should make sure to match what it is.

joeygrover · 2022-02-23T18:41:52Z

android/sdl_android/src/main/java/com/smartdevicelink/transport/SdlRouterService.java

+                    } else {
+                        waitingForBTRuntimePermissions = false;
+                        initBluetoothSerialService();
+                    }


Suggested change

} else {

waitingForBTRuntimePermissions = false;

initBluetoothSerialService();

}

} else {

waitingForBTRuntimePermissions = false;

initBluetoothSerialService();

final NotificationManager notificationManager = (NotificationManager) getSystemService(Context.NOTIFICATION_SERVICE);

if(notificationManager != null) {

notificationManager.cancel("SDL", TransportConstants.SDL_ERROR_NOTIFICATION_CHANNEL_ID_INT);

}

}

We want to cancel the notification if the permission has been granted

joeygrover · 2022-02-23T19:37:56Z

android/hello_sdl_android/src/main/java/com/sdl/hellosdlandroid/MainActivity.java

+            if (android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) {
+                if (!checkPermission()) {
+                    requestPermission();
+                }
+            } else if (BuildConfig.TRANSPORT.equals("TCP")){
+                //If we are connected to a module we want to start our SdlService
+                SdlReceiver.queryForConnectedService(this);
+            }
+        } else {


Suggested change

if (android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) {

if (!checkPermission()) {

requestPermission();

}

} else if (BuildConfig.TRANSPORT.equals("TCP")){

//If we are connected to a module we want to start our SdlService

SdlReceiver.queryForConnectedService(this);

}

} else {

if (android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.S

&& !checkPermission()) {

requestPermission();

return;

}

//If we are connected to a module we want to start our SdlService

SdlReceiver.queryForConnectedService(this);

} else if (BuildConfig.TRANSPORT.equals("TCP")) {

This logic needed to be cleaned up. Basically check for OS version and if permission is granted, if not request them. If OS is less than 12 or permissions are granted, query for the RS. The previous else if was in the wrong position for TCP.

joeygrover · 2022-02-23T19:46:56Z

.../sdl_android/src/main/java/com/smartdevicelink/transport/USBAccessoryAttachmentActivity.java

                        return;
                    }
                    serviceIntent.setAction(TransportConstants.BIND_REQUEST_TYPE_ALT_TRANSPORT);
+                    serviceIntent.putExtra(TransportConstants.CONNECTION_TYPE_EXTRA, TransportConstants.ACTION_USB_ACCESSORY_ATTACHED);


Suggested change

serviceIntent.putExtra(TransportConstants.CONNECTION_TYPE_EXTRA, TransportConstants.ACTION_USB_ACCESSORY_ATTACHED);

serviceIntent.putExtra(TransportConstants.CONNECTION_TYPE_EXTRA, TransportConstants.AOA_USB);

Should avoid using an action string for an extra in an intent.

joeygrover · 2022-02-23T19:48:35Z

android/sdl_android/src/main/java/com/smartdevicelink/transport/SdlRouterService.java

        }
+        boolean isConnectedOverUSB = false;
+        if (intent != null && intent.hasExtra(TransportConstants.CONNECTION_TYPE_EXTRA)) {
+            isConnectedOverUSB = TransportConstants.ACTION_USB_ACCESSORY_ATTACHED.equalsIgnoreCase(intent.getStringExtra(TransportConstants.CONNECTION_TYPE_EXTRA));


Suggested change

isConnectedOverUSB = TransportConstants.ACTION_USB_ACCESSORY_ATTACHED.equalsIgnoreCase(intent.getStringExtra(TransportConstants.CONNECTION_TYPE_EXTRA));

isConnectedOverUSB = TransportConstants.AOA_USB.equalsIgnoreCase(intent.getStringExtra(TransportConstants.CONNECTION_TYPE_EXTRA));

bilal-alsharifi and others added 24 commits July 28, 2021 14:54

Update sdk to 31

1656b45

Update sdk to 31

26ccb8b

Add the new required android:exported="true" attr

664e305

Add FLAG_IMMUTABLE to PendingIntent

01ca770

Update BT Permissions and Notification Behavior

8bb0272

Merge branch 'develop' into feature/android_12_fixes

121a51f

Add targetApi

4cb096b

RS Notificaiton only if has connected before

250dffb

Get BT permissions as runtime permissions

49b8180

Merge branch 'develop' into feature/android_12_fixes

d1a77c9

Only set immutable flag if android 12 or newer

327d8d6

Enable or disable rs based on permission

a07bd7c

Toggle Service enabled based on BT permissions

17d39c2

BroadcastReceiver changes

316c673

Merge branch 'develop' into feature/android_12_fixes

90f8a4a

Stop service is disabled

01fae83

Fix broadcastReceiverBehavior

8c58dc4

Merge branch 'feature/android_12_fixes' of github.com:smartdevicelink…

9eb0069

…/sdl_java_suite into feature/android_12_fixes

Allow USB to start Router Service if no BT perm

7bd28d7

Fix initCheck in RS

e5bb2cf

Add PendingIntents to start services from RS

71dc5f6

Add Check for TransportType to RS

18133ce

Update RS to pick idedal RS

f866291

Remove secondPass Logic

a76ac65

RHenigan changed the base branch from master to develop November 30, 2021 18:39

bilal-alsharifi and others added 4 commits November 30, 2021 14:38

Add notification to remind user to allow bluetooth permissions

71f80f4

Merge branch 'develop' into feature/android_12_fixes

997b231

Proposal Revision Changes

0dfdb87

Add Bt check for t permissions notificaiton

578a182

RHenigan added 3 commits February 14, 2022 15:15

Remove BT Scan Permission Check

475e170

Router Service Version increase

6fd76ae

Clean up and comments

3b2f02b

joeygrover reviewed Feb 18, 2022

View reviewed changes

Dev Review Fixes

48f7cb0

joeygrover reviewed Feb 23, 2022

View reviewed changes

RHenigan added 3 commits February 23, 2022 15:33

Re-Review changes

bbbfb8c

Fix edge case with permissions being revoked

2600817

Remove accidental ignore

03a0682

RHenigan marked this pull request as ready for review February 24, 2022 21:48

joeygrover approved these changes Feb 24, 2022

View reviewed changes

joeygrover merged commit dab6bc6 into develop Feb 24, 2022

joeygrover deleted the feature/android_12_fixes branch February 24, 2022 22:09

	<string name="allow_bluetooth_permissions">Please grant this app bluetooth permissions</string>
	<string name="allow_bluetooth_permissions">Please grant this app the Nearby Devices Permission to use bluetooth</string>

	serviceIntent.putExtra(TransportConstants.CONNECTION_TYPE_EXTRA, TransportConstants.ACTION_USB_ACCESSORY_ATTACHED);
	serviceIntent.putExtra(TransportConstants.CONNECTION_TYPE_EXTRA, TransportConstants.AOA_USB);

	isConnectedOverUSB = TransportConstants.ACTION_USB_ACCESSORY_ATTACHED.equalsIgnoreCase(intent.getStringExtra(TransportConstants.CONNECTION_TYPE_EXTRA));
	isConnectedOverUSB = TransportConstants.AOA_USB.equalsIgnoreCase(intent.getStringExtra(TransportConstants.CONNECTION_TYPE_EXTRA));

Conversation

RHenigan commented Nov 30, 2021 • edited by joeygrover Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Risk

Testing Plan

Unit Tests

Core Tests

Summary

CLA

Uh oh!

codecov bot commented Nov 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

joeygrover left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

RHenigan commented Nov 30, 2021 •

edited by joeygrover

Loading

codecov bot commented Nov 30, 2021 •

edited

Loading