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 14 foreground service type required #1860

Merged
merged 44 commits into from
Nov 29, 2023

Conversation

JulianKast
Copy link
Contributor

@JulianKast JulianKast commented Aug 11, 2023

Fixes #1859

This PR is ready for review.

Risk

This PR makes minor 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

Unit Tests

Unit tests were run on the integration branch. CI check failed due to needing the app to target API 34.

Core Tests

Note: Before you install a second and third app, unplug the phone from the computer and switch the build variant before deploying the app to the phone otherwise Android Studio will kill the app.

Apps A, B and C.
Test 1:
Install App A
Accept BT permission
Install App B
Deny BT permission
Connect to TDK via AOA
Select App A to receive intent
Allow App B to connect when prompted
Observe: App B pass hosting the RS to app A and both apps connect.

Test 2:
Install App A
Deny BT permission
Install App B
Deny BT permission
Connect to TDK via AOA
Select App A to receive intent
Allow App B to connect when prompted
Observe App B passed hosting the RS to app A and both apps connect.

Test 3:
Install App A
Deny BT permission
Install App B
Deny BT permission
Install App C
Deny BT permission
Connect to TDK via AOA
Select App A to receive intent
Allow App C to connect when prompted
Allow App B to connect if you're prompted.(not 100%)
Observe App C passed hosting the RS to App B who passed to App A and Apps A and C are connected. App B may not connect.

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

Summary

This pr adds the foreground service type of connectedDevice to our library. This is a requirement for Android 14.
With this addition, we are required to have either the Bluetooth Connect permission or call UsbManager.requestPermission() to have our service enter the foreground. if the connection is AOA and an app selected to host the RouterService does not have either permission, we will try to pass hosting the RouterService off to another app.

I also add the ability to be able to request the usb permission in SdlReceiver.onSdlEnabled in the event that

Changelog

Bug Fixes
  • Fixed the library to allow for it to target Android 14 and be able to enter the foreground

CLA

@JulianKast
Copy link
Contributor Author

Unit test for Android currently fail due to needing to target API 34 which is in a different PR.

Base automatically changed from bugfix/issue_1852_update_gradle to develop October 19, 2023 15:00
Copy link
Member

@joeygrover joeygrover left a comment

Choose a reason for hiding this comment

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

Looks like the other PRs need to be finished and merged into develop, then develop back into this branch to test more properly. During the first review it seems like everything is working as it should, but will need to retest and more robustly post the rest of the fixes being merged.

Comment on lines 86 to 90
try {
pendingIntent.send(context, 0, cachedIntent);
} catch (PendingIntent.CanceledException e) {
e.printStackTrace();
}
Copy link
Member

Choose a reason for hiding this comment

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

There's currently no call to unregister this receiver so it will potentially leak. Changed the caught exception to a generic one as well in case something happens during unregistering.

Suggested change
try {
pendingIntent.send(context, 0, cachedIntent);
} catch (PendingIntent.CanceledException e) {
e.printStackTrace();
}
try {
pendingIntent.send(context, 0, cachedIntent);
context.unregisterReceiver(this);
} catch (Exception e) {
e.printStackTrace();
}

Copy link
Member

@joeygrover joeygrover left a comment

Choose a reason for hiding this comment

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

Looks like one last missed change request from the previous review

Comment on lines 84 to 89
try {
pendingIntentToStartService.send(context, 0, startSdlServiceIntent);
} catch (PendingIntent.CanceledException e) {
e.printStackTrace();
}
}
Copy link
Member

Choose a reason for hiding this comment

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

This was missed from previous review.

Suggested change
try {
pendingIntentToStartService.send(context, 0, startSdlServiceIntent);
} catch (PendingIntent.CanceledException e) {
e.printStackTrace();
}
}
try {
pendingIntentToStartService.send(context, 0, startSdlServiceIntent);
context.unregisterReceiver(this);
} catch (Exception e) {
e.printStackTrace();
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, looks like I made the this change in the wrong spot

@joeygrover joeygrover merged commit 7ba7e31 into develop Nov 29, 2023
4 checks passed
@joeygrover joeygrover deleted the bugfix/issue_1859_foreground_service_type branch November 29, 2023 19:48
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

Successfully merging this pull request may close these issues.

2 participants