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

Dhruv2295/code refactor #128

Merged
merged 4 commits into from Dec 14, 2020
Merged

Conversation

dhruv2295
Copy link
Contributor

Migration of dependencies to androidX.
Basic code cleanup to remove redundant initialisers.

@dhruv2295 dhruv2295 closed this Dec 13, 2020
@dhruv2295 dhruv2295 reopened this Dec 13, 2020
Copy link
Collaborator

@thias15 thias15 left a comment

Choose a reason for hiding this comment

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

Upgrade to API 30 causes app crash when logging. The permission handling for storage seems to have changed.

@@ -240,11 +239,7 @@ protected void onCreate(final Bundle savedInstanceState) {
new ViewTreeObserver.OnGlobalLayoutListener() {
@Override
public void onGlobalLayout() {
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about old Android versions?

Copy link
Contributor Author

@dhruv2295 dhruv2295 Dec 14, 2020

Choose a reason for hiding this comment

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

App's minimum supported version is 21. This check was checking for 18.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, makes sense. Android 5.0 (API level 21) = Lollipop.

updateVehicleState();

runOnUiThread(
new Runnable() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why use lambda previously but not here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no global converter in android studio to convert all these anonymous functions to lambdas at once.
So it's more of an iterative process, where I keep replacing them as and when I find them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've converted this and pushed another commit

LOGGER.e("Error receiving USB data");
}
private final UsbSerialInterface.UsbReadCallback callback =
data -> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happens on error, since try/catch is removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was an android studio recommendation that this catch block would never be called. I'll add it back. Better safer.

@thias15 thias15 merged commit d4eebd5 into isl-org:master Dec 14, 2020
Copy link
Collaborator

@thias15 thias15 left a comment

Choose a reason for hiding this comment

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

Merged

@dhruv2295 dhruv2295 deleted the dhruv2295/code_refactor branch December 30, 2020 16:50
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.

None yet

2 participants