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

Status widget #431

Merged
merged 9 commits into from
Aug 24, 2023
Merged

Status widget #431

merged 9 commits into from
Aug 24, 2023

Conversation

Anton-V-K
Copy link
Contributor

@Anton-V-K Anton-V-K commented Jan 30, 2022

Simple implementation of the status widget (#429).
The base code was generated with Android Studio 2020.3.1.
Testing:

  • Android 4.1
  • Android 4.4
  • Android 5.0
  • Android 6.0 (Samsung Galaxy S6, 768x1280, Genymotion)
  • Android 7.0 (Pixel 2, 1080x1920, AVD)
  • Android 8.1 (Samsung Galaxy S8, 1440x2960, Genymotion)
  • Android 9.0 (Amazon Fire HD 10, 1920x1200, Genymotion)
  • Android 9.0 (Oukitel Y4800, 1080x2340)
  • Android 10.0 (Samsung Galaxy S10, 1440x3040, Genymotion)
  • Android 11.0 (Realme 8i, 1080x2400)
  • Android 12.0
  • Android 13.0
  • Android 14.0

@Anton-V-K Anton-V-K mentioned this pull request Jan 30, 2022
11 tasks
@tananaev
Copy link
Member

I think some of the comments are not addressed. Can we remove the theme and minimize the number of files/code that we need to add such a simple widget?

@Anton-V-K
Copy link
Contributor Author

Anton-V-K commented Jan 30, 2022

Yes, I kept the theme support - users with Android 12+ may benefit from it. Does it make a problem?
I'm not sure which redundant code you see - each piece is needed for the widget to work properly.
And I've dropped images for the widget's preview.

@tananaev
Copy link
Member

Can you please explain why we need the theme.

@Anton-V-K
Copy link
Contributor Author

I've removed support for style and theme, since they aren't needed for the status widget.

app/src/main/java/org/traccar/client/StatusWidget.kt Outdated Show resolved Hide resolved
app/src/main/res/values/dimens.xml Outdated Show resolved Hide resolved
app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved
app/src/main/res/xml/status_widget_info.xml Outdated Show resolved Hide resolved
app/src/main/res/values/attrs.xml Outdated Show resolved Hide resolved
app/src/main/res/layout/status_widget.xml Outdated Show resolved Hide resolved
@Anton-V-K
Copy link
Contributor Author

Wow! A year has passed! :)
I wonder if anything else is required from my side to make this mini-feature ready for merging?

Copy link
Member

@tananaev tananaev left a comment

Choose a reason for hiding this comment

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

Sorry, looks like I forgot about it back then. Added some comments.

app/src/main/res/layout/status_widget.xml Outdated Show resolved Hide resolved
app/src/main/res/values/dimens.xml Outdated Show resolved Hide resolved
app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved
app/src/main/java/org/traccar/client/StatusWidget.kt Outdated Show resolved Hide resolved
app/src/main/java/org/traccar/client/StatusWidget.kt Outdated Show resolved Hide resolved
app/src/main/java/org/traccar/client/StatusWidget.kt Outdated Show resolved Hide resolved
@Anton-V-K
Copy link
Contributor Author

New changes are ready for another review round :)

Copy link
Member

@tananaev tananaev left a comment

Choose a reason for hiding this comment

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

Looks much better. Added some very minor comments.

app/src/main/java/org/traccar/client/StatusWidget.kt Outdated Show resolved Hide resolved
app/src/main/java/org/traccar/client/StatusWidget.kt Outdated Show resolved Hide resolved
app/src/main/java/org/traccar/client/StatusWidget.kt Outdated Show resolved Hide resolved
app/src/main/res/xml/status_widget_info.xml Outdated Show resolved Hide resolved
app/src/main/res/xml/status_widget_info.xml Outdated Show resolved Hide resolved
app/src/main/java/org/traccar/client/StatusWidget.kt Outdated Show resolved Hide resolved
app/src/main/java/org/traccar/client/StatusWidget.kt Outdated Show resolved Hide resolved
@Anton-V-K
Copy link
Contributor Author

Anton-V-K commented Aug 13, 2023

I've performed more testing of the recent build on several versions of Android (virtualized), and noticed that the widget isn't updated any longer when the service is started/stopped (via the shortcut or from within the app).
The update works for:

  • 6.0 (Samsung Galaxy S6)
  • 7.0 (Pixel 2)
  • 9.0 (Amazon Fire HD 10)

It doesn't happen for:

  • 8.1 (Samsung Galaxy S8)
  • 10 (Samsung Galaxy S10)
  • 11 (Realmi 8i, real device)

The only significant change between the builds is the way how intent's handlers are registered for the widget: explicitly (old build) or in the app's manifest (new build).
I guess, there is some limitation/restriction in newer Androids regarding catching custom intents via app's manifest, but I can't find any official ground for this.

@tananaev
Copy link
Member

Not sure. Maybe there are some limitations or maybe some attributes are missing.

@Anton-V-K
Copy link
Contributor Author

Anton-V-K commented Aug 22, 2023

I've found the reason why the widget doesn't receive custom notifications - since Android 8 (API level 26) the system imposes additional restrictions on manifest-declared receivers (reference):

If your app targets Android 8.0 or higher, you cannot use the manifest to declare a receiver for most implicit broadcasts (broadcasts that don't target your app specifically). You can still use a context-registered receiver when the user is actively using your app.

Possible workarounds:

  1. Old approach with explicitly registered receiver (in StatusWidget.onEnabled)
  2. Context-registered reciever (probably in StatusWidget.onEnabled, requires AndroidX Core library 1.9.0+)
  3. Specify package name for intents when sending broadcasts (in TrackingService.onCreate and TrackingService.onDestroy) - this approach also works as expected. The changes are as follows:
    • sendBroadcast(Intent(ACTION_STARTED).setPackage(packageName)) in TrackingService.onCreate()
    • sendBroadcast(Intent(ACTION_STOPPED).setPackage(packageName)) in TrackingService.onDestroy()

Which way is preferred?
Any other ideas?

@tananaev
Copy link
Member

Does the last option allow us to keep XML registration? Then I like that one.

@Anton-V-K
Copy link
Contributor Author

Yes, the option 3 goes on top of the current implementation with manifest-registered receiver with custom intents filter.
Other options don't require custom intents filter in AndroidManifest.xml for the status widget.

@Anton-V-K
Copy link
Contributor Author

Ooops! There is an easy-to-resolve conflict in TrackingService.onCreate :(
Should I merge and resolve conventionally or should I try to rebase my changes on top of master?

@tananaev
Copy link
Member

Both merge and rebase are fine.

@Anton-V-K
Copy link
Contributor Author

Anton-V-K commented Aug 24, 2023

I've merged master into the widget's branch locally, and now I get more warnings during the build:

> Task :app:compileRegularDebugKotlin
w: file:///.../app/src/main/java/org/traccar/client/AndroidPositionProvider.kt:57:34
    This declaration overrides deprecated member but not marked as deprecated itself.
    This deprecation won't be inherited in future releases. Please add @Deprecated annotation or suppress.
    See https://youtrack.jetbrains.com/issue/KT-47902 for details
w: file:///.../app/src/main/java/org/traccar/client/BatteryOptimizationHelper.kt:58:40
    'resolveActivity(Intent, Int): ResolveInfo?' is deprecated. Deprecated in Java
w: file:///.../app/src/main/java/org/traccar/client/MainFragment.kt:249:21
    'requestPermissions(Array<(out) String!>, Int): Unit' is deprecated. Deprecated in Java
w: file:///.../app/src/main/java/org/traccar/client/MainFragment.kt:268:21
    'requestPermissions(Array<(out) String!>, Int): Unit' is deprecated. Deprecated in Java
w: file:///.../app/src/main/java/org/traccar/client/TrackingService.kt:49:13
    'stopForeground(Boolean): Unit' is deprecated. Deprecated in Java
w: file:///.../app/src/main/java/org/traccar/client/TrackingService.kt:93:9
    'stopForeground(Boolean): Unit' is deprecated. Deprecated in Java

I haven't tried to build master separately, so I'm not sure whether these warnings are accceptable.
It seems these warnings aren't in my code, so may I push without fixing them?

@tananaev
Copy link
Member

If they're the same on master, it should be fine.

…et with minor changes for traccar#429

# Resolved conflicts:
#	app/src/main/java/org/traccar/client/TrackingService.kt
@Anton-V-K
Copy link
Contributor Author

I've merged master into the widget's branch and applied few minor changes to address warnings in my code.

@tananaev tananaev merged commit f7ecc48 into traccar:master Aug 24, 2023
@tananaev
Copy link
Member

Merged, thanks for fixing all the feedback. I know it was a lot.

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