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

Grant SCHEDULE_EXACT_ALARM permission in instrumentation test #551

Merged
merged 12 commits into from
Jan 17, 2024

Conversation

SM2A
Copy link

@SM2A SM2A commented Jan 17, 2024

I wasn't able to run android test on CI
Please approve this pull request CI checks so I can make sure every thing is fine

@hannesa2
Copy link
Owner

When you push now, it should build as well.
Btw, please remove this pointless
image

@SM2A
Copy link
Author

SM2A commented Jan 17, 2024

Yes yes I will make new clean branch
I was just trying to run tests on CI
gradlew cAT task works fine on my actual device (Google pixel 6a - android 14 - API 34)

@hannesa2
Copy link
Owner

gradlew cAT task works fine on my actual device (Google pixel 6a - android 14 - API 34)

Sorry, this is not the benchmark. The CI should work.

@SM2A
Copy link
Author

SM2A commented Jan 17, 2024

gradlew cAT task works fine on my actual device (Google pixel 6a - android 14 - API 34)

Sorry, this is not the benchmark. The CI should work.

I wasn't trying to say that it works on my device, I wanted you to know I am trying to make it working on CI
Sorry for that

@SM2A
Copy link
Author

SM2A commented Jan 17, 2024

I think errors about using ALARM are fixed but now time out causing tests to fail. Is the server up and running?

@hannesa2
Copy link
Owner

Is the server up and running?

yes #552

@SM2A
Copy link
Author

SM2A commented Jan 17, 2024

After a few hours of searching I found what the problem was.
Please tell if the current code is acceptable so that I remove redundant commits in a new branch
And sorry for taking your time

@hannesa2 hannesa2 merged commit 1d475d5 into hannesa2:api34-3.x Jan 17, 2024
3 checks passed
context.registerReceiver(receiver, filter, Context.RECEIVER_NOT_EXPORTED)
} else
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU)
context.registerReceiver(receiver, filter, Context.RECEIVER_EXPORTED)
Copy link
Owner

Choose a reason for hiding this comment

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

Why must it be exported ?

Copy link
Author

Choose a reason for hiding this comment

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

Not really sure about what I'm going to say but I think because the instrumentation test app and the library are on different places. It took me while to find out that the broadcasts send by the callbackToActivity function are not received by the MqttAndroidClient broadcast receiver. It fixed by this change.
It might work in real app

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Owner

@hannesa2 hannesa2 Jan 17, 2024

Choose a reason for hiding this comment

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

I'm open for any improvement

Copy link
Author

Choose a reason for hiding this comment

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

I will see what can I do

Copy link
Owner

@hannesa2 hannesa2 left a comment

Choose a reason for hiding this comment

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

I'll fix the noisy commits by myself

@@ -6,6 +6,8 @@
<!-- Permissions the Application Requires -->
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
<uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" />
<uses-permission android:name="android.permission.SCHEDULE_EXACT_ALARM" />
Copy link
Owner

Choose a reason for hiding this comment

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

I guess I merged too fast.
This should be only in library

Copy link
Author

Choose a reason for hiding this comment

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

Yes that's right

@@ -6,6 +6,8 @@
<uses-permission android:name="android.permission.POST_NOTIFICATIONS" />
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
<uses-permission android:name="android.permission.FOREGROUND_SERVICE" />
<uses-permission android:name="android.permission.SCHEDULE_EXACT_ALARM" />
Copy link
Owner

Choose a reason for hiding this comment

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

This should be only in library

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for that

@@ -1,5 +1,8 @@
<manifest xmlns:android="http://schemas.android.com/apk/res/android">

<uses-permission android:name="android.permission.SCHEDULE_EXACT_ALARM" />
Copy link
Owner

Choose a reason for hiding this comment

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

👍 Yes, here it is

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