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

Update Dependencies (Fix jitpack.io build) #377

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

angelix
Copy link

@angelix angelix commented Nov 4, 2024

Build log:
https://jitpack.io/com/github/angelix/UsbSerial/fdefe94f65/build.log

Build Dependency

dependencies {
      implementation 'com.github.angelix:UsbSerial:0f16336a4e'
}

@angelix angelix force-pushed the ave-update-dependencies branch 3 times, most recently from 23dce44 to 0d424dc Compare November 4, 2024 19:32
@@ -211,7 +211,7 @@ private void setFilter() {
*/
private void requestUserPermission() {
Log.d(TAG, String.format("requestUserPermission(%X:%X)", device.getVendorId(), device.getProductId() ) );
PendingIntent mPendingIntent = PendingIntent.getBroadcast(this, 0, new Intent(ACTION_USB_PERMISSION), 0);
PendingIntent mPendingIntent = PendingIntent.getBroadcast(this, 0, new Intent(ACTION_USB_PERMISSION), PendingIntent.FLAG_IMMUTABLE);

Choose a reason for hiding this comment

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

I got no ideas the details here. But it seems 0 would be signifying on flags, and PendingIntent.FLAG_IMMUTABLE is 0x04000000.

So I assume you found that for some reason that flag is necessary?

Copy link
Author

Choose a reason for hiding this comment

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

This change is required if you target Android 12.
It's on the example codebase, won't have any implication on the library itself.

https://developer.android.com/about/versions/12/behavior-changes-12#pending-intent-mutability

Copy link

@ChaseGuru ChaseGuru left a comment

Choose a reason for hiding this comment

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

Overall, this seems like a great modernization PR

@@ -1,5 +1,6 @@
VERSION_NAME=6.1.0
VERSION_CODE=1
ANDROID_BUILD_MIN_SDK_VERSION=12
ANDROID_BUILD_MIN_SDK_VERSION=24
ANDROID_BUILD_TARGET_SDK_VERSION=27

Choose a reason for hiding this comment

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

Maybe would be nice if this could be higher, like android 34, or 33? Though it may not make a difference. I don't believe compiling our flutter app was needing android sdk 27 - so I assume the reason flutter dependencies can end up requiring older SDK versions is if flutter isn't relying on precompiled versions of flutter package's android code? In this case it's using a hosted pre-compiled version from maven/jitpack so it we don't need the compile sdk

Copy link
Author

Choose a reason for hiding this comment

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

Changing target SDK version means that the library must behave as expected in the target environment.
I haven't checked the code for any breaking change targeting 34.
I am not confident on touching this version number if there is no real benefit to it.

@angelix angelix force-pushed the ave-update-dependencies branch from 0d424dc to 80170bb Compare November 19, 2024 12:03
@angelix angelix force-pushed the ave-update-dependencies branch from 80170bb to 0f16336 Compare November 19, 2024 12:09
@angelix angelix changed the title Update build dependencies Update Dependencies (Fix jitpack.io build) Nov 19, 2024
@angelix
Copy link
Author

angelix commented Nov 19, 2024

In commit 0f16336 i fixed the release artifacts naming.

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