app, cmd/gogio: add android foreground permission and service by mixmasala · Pull Request #67 · gioui/gio

mixmasala · 2021-11-18T10:58:03Z

This adds the permission android.permission.FOREGROUND_SERVICE and adds
GioForegroundService which creates the tray Notification necessary to
implement the Foreground Service. The package foreground includes the method
StartForeground, which on android, notifies the system that the program
will perform background work and that it shouldn't be killed. It returns
a channel that should be closed when the background work is complete.

See https://developer.android.com/guide/components/foreground-services
and https://developer.android.com/training/notify-user/build-notification

mixmasala · 2021-11-18T11:07:41Z

I've tested this with my application; but feel that there could be better integration with the notification library gioui.org/x/notify, so that the foreground service can share the same notification channels, in particular so that the text of the background notification can be updated. I'm not sure that this change should depend on that package though, but I think that the Service class needs to be declared in the application manifest. Any help resolving/improving the above details is very much appreciated.

eliasnaur · 2021-11-18T16:27:59Z

Nice work, thank you.

I've tested this with my application; but feel that there could be better integration with the notification library gioui.org/x/notify, so that the foreground service can share the same notification channels, in particular so that the text of the background notification can be updated. I'm not sure that this change should depend on that package though, but I think that the Service class needs to be declared in the application manifest. Any help resolving/improving the above details is very much appreciated.

Why share notifications with niotify? This change is for foreground work, niotify is for notifications. However, see my review comments for a suggestion to customize foreground notification content text.

app/GioForegroundService.java

app/permission/foreground/android.go

app/permission/foreground/main.go

app/permission/foreground/android.go

eliasnaur · 2021-11-18T16:29:24Z

Oh, and remember to sign-off your change and squash commits.

mixmasala · 2021-11-18T17:25:33Z

Nice work, thank you.

I've tested this with my application; but feel that there could be better integration with the notification library gioui.org/x/notify, so that the foreground service can share the same notification channels, in particular so that the text of the background notification can be updated. I'm not sure that this change should depend on that package though, but I think that the Service class needs to be declared in the application manifest. Any help resolving/improving the above details is very much appreciated.

Why share notifications with niotify? This change is for foreground work, niotify is for notifications. However, see my review comments for a suggestion to customize foreground notification content text.

well, the foreground service requires a notification - the code here just sets a static string which isn't very appealing. Android applications using foreground services tend to have some kind of content displayed in the service notification and it would be useful to have a clean way to interact with the notification - possibly the StartForeground method should return or accept this as an argument - and this is where it starts to overlap with the functionality provided by niotify.

mixmasala · 2021-11-19T12:46:37Z

edit: fixed

mixmasala · 2021-11-26T11:50:08Z

I've refactored this PR to address issues raised above and changed the API to include setting the notification ID, title, and text.

app/os_android.go

app/permission/foreground/main.go

cmd/gogio/androidbuild.go

mixmasala · 2021-11-29T16:25:44Z

edit: stopped using startForegroundService because I saw some crash with resuming the application from background with same error as: transistorsoft/nativescript-background-geolocation-lt#121 (comment) . Instead startService is used, which fixes this crash.

app/GioForegroundService.java

app/os_android.go

app/permission/foreground/main.go

cmd/gogio/permission.go

cmd/gogio/androidbuild.go

eliasnaur

Leaving a few review comments for your follow-ups.

app/GioForegroundService.java

cmd/gogio/androidbuild.go

app/os_android.go

app/GioForegroundService.java

app/Gio.java

mixmasala · 2022-11-10T16:31:48Z

For some reason github wont let me comment on the change requests directly any more.

I moved code out of gioui.org/app/permissions, but I was not able to find a solution to check that gioui.org/app/permissions/foreground was imported when built. For example, I thought that BuildInfo might have package-level resolution, but it does not:

+func hasPermission(permission string) bool {
+       permissionPath := filepath.Join("gioui.org/app/permission/", permission)
+       if info, ok := debug.ReadBuildInfo(); ok {
+               for _, module := range info.Deps {
+                       fmt.Println("Imports: ", module.Path)
+                       // XXX: we do not have a way to get the list of included go packages at runtime
+                       if module.Path == permissionPath {
+                               return true
+                       }
+               }
+       }
+       return false
+}

eliasnaur · 2022-11-12T17:49:41Z

For some reason github wont let me comment on the change requests directly any more.

I moved code out of gioui.org/app/permissions, but I was not able to find a solution to check that gioui.org/app/permissions/foreground was imported when built. For example, I thought that BuildInfo might have package-level resolution, but it does not:
+func hasPermission(permission string) bool {
+       permissionPath := filepath.Join("gioui.org/app/permission/", permission)
+       if info, ok := debug.ReadBuildInfo(); ok {
+               for _, module := range info.Deps {
+                       fmt.Println("Imports: ", module.Path)
+                       // XXX: we do not have a way to get the list of included go packages at runtime
+                       if module.Path == permissionPath {
+                               return true
+                       }
+               }
+       }
+       return false
+}

I don't think you need to check for the import. It should be enough to detect the Java exception/error when trying to open a foreground notification.

mixmasala · 2022-12-29T18:35:17Z

I don't think you need to check for the import. It should be enough to detect the Java exception/error when trying to open a foreground notification.

Good Idea. I tried to catch java.lang.SecurityException, but the exception is not raised to the caller of android.startForegroundService, as it is thrown in the method onStartCommand which is called by the android environment after the service is started. Do you have any ideas how to handle this?

eliasnaur · 2023-01-01T16:35:53Z

I don't think you need to check for the import. It should be enough to detect the Java exception/error when trying to open a foreground notification.

Good Idea. I tried to catch java.lang.SecurityException, but the exception is not raised to the caller of android.startForegroundService, as it is thrown in the method onStartCommand which is called by the android environment after the service is started. Do you have any ideas how to handle this?

One idea is to make onStartCommand report back to Gio whether the foreground notification succeeded or not. I think you'll need to wrap a chan error or similar with cgo.NewHandle to pass it to the Intent argument to onStartCommand. Then, in app.StartForeground wait for the channel to return an error (and free the handle).

eliasnaur

I somehow failed to send this review out after writing it. I apologize.

eliasnaur · 2023-01-07T00:55:57Z

app/Gio.java

+
+	static Intent startForegroundService(Context ctx, String title, String text) throws ClassNotFoundException {
+		Intent intent = new Intent();
+		intent.setClass(ctx, ctx.getClassLoader().loadClass("org/gioui/GioForegroundService"));


There must be more to the error than what you pasted. A stack trace or similar that can tell us why the class cannot be loaded.

eliasnaur · 2023-01-07T00:58:15Z

app/os_android.go

+				foregroundService.intent = C.jni_NewGlobalRef(env, foregroundService.intent)
+
+			} else {
+				panic(err)


You should propagate this error to the caller. In particular when the XXX issue above is resolved so that startForegroundService can fail for configuration reasons.

eliasnaur · 2023-01-07T01:00:56Z

app/os_android.go

+// Start starts the foreground service
+func Start(title, text string) (stop func(), err error) {


The documentation needs expanding. To make the documentation visible for everyone, rename this function startForeground and add a documented StartForeground in package app that calls it.

eliasnaur · 2023-01-07T01:01:21Z

app/os_nandroid.go

+package app
+
+// Start is a no-op on platforms other than android
+func Start(title, text string) (stop func(), err error) {


As described above, rename this function to startForeground.

This adds the permission android.permission.FOREGROUND_SERVICE and adds GioForegroundService which creates the tray Notification necessary to implement the Foreground Service. This adds the method Start to package app, which on android, notifies the system that the program will perform background work and that it shouldn't be killed. The foreground service is stopped using the cancel function returned by Start(). If multiple calls to Start are made, the foreground service will not be stopped until the final cancel function has been called. See https://developer.android.com/guide/components/foreground-services and https://developer.android.com/training/notify-user/build-notification Signed-off-by: Masala <masala@riseup.net>

dosgo · 2024-09-24T00:52:04Z

Is there any progress on this? Write Android applications in pure Go by adding services and BroadcastReceiver support.

mixmasala marked this pull request as draft November 18, 2021 11:09

mixmasala marked this pull request as ready for review November 18, 2021 11:09

eliasnaur reviewed Nov 18, 2021

View reviewed changes

mixmasala marked this pull request as draft November 19, 2021 12:43

mixmasala force-pushed the add_foreground_service branch from 596a0f2 to 87a7c2f Compare November 26, 2021 11:48

mixmasala marked this pull request as ready for review November 26, 2021 11:49

mixmasala force-pushed the add_foreground_service branch from 87a7c2f to d295b22 Compare November 26, 2021 11:51

eliasnaur reviewed Nov 26, 2021

View reviewed changes

mixmasala force-pushed the add_foreground_service branch 4 times, most recently from c65a7be to 917b1da Compare November 29, 2021 16:22

eliasnaur reviewed Dec 2, 2021

View reviewed changes

mixmasala force-pushed the add_foreground_service branch 6 times, most recently from 206d76d to 37f30b7 Compare December 9, 2021 23:32

eliasnaur reviewed Dec 10, 2021

View reviewed changes

mixmasala force-pushed the add_foreground_service branch 2 times, most recently from f0f3c2a to 84b41ef Compare December 10, 2021 09:16

mixmasala force-pushed the add_foreground_service branch from 84b41ef to 44a59e2 Compare January 2, 2022 11:28

mixmasala force-pushed the add_foreground_service branch from e2f1cfa to c517124 Compare November 10, 2022 16:52

mixmasala force-pushed the add_foreground_service branch 2 times, most recently from 7fd3b23 to 079448b Compare January 2, 2023 15:52

eliasnaur reviewed Jan 19, 2023

View reviewed changes

whereswaldon force-pushed the main branch from 8ad48ab to ae3bd2a Compare September 8, 2023 16:49

mixmasala force-pushed the add_foreground_service branch from 079448b to 1c97118 Compare September 17, 2023 18:52

mixmasala force-pushed the add_foreground_service branch 2 times, most recently from 5a91e41 to a1625b7 Compare September 27, 2023 11:19

whereswaldon force-pushed the main branch 4 times, most recently from 0d543a0 to 1686874 Compare October 7, 2023 00:14

whereswaldon force-pushed the main branch from 21bf070 to fc6e51d Compare November 15, 2023 17:24

gioui deleted a comment from ddkwork Apr 11, 2024

whereswaldon force-pushed the main branch from 38d8889 to 691adf4 Compare May 1, 2024 18:00

whereswaldon force-pushed the main branch 2 times, most recently from 67c77c9 to 46cc311 Compare May 30, 2024 08:05

whereswaldon force-pushed the main branch 2 times, most recently from f8029f2 to 026d3f9 Compare June 20, 2024 07:54

whereswaldon force-pushed the main branch 5 times, most recently from 4759601 to f676fd2 Compare June 27, 2024 12:13

		// Start starts the foreground service
		func Start(title, text string) (stop func(), err error) {

Conversation

mixmasala commented Nov 18, 2021

Uh oh!

mixmasala commented Nov 18, 2021

Uh oh!

eliasnaur commented Nov 18, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eliasnaur commented Nov 18, 2021

Uh oh!

mixmasala commented Nov 18, 2021

Uh oh!

mixmasala commented Nov 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mixmasala commented Nov 26, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mixmasala commented Nov 29, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eliasnaur left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mixmasala commented Nov 10, 2022

Uh oh!

eliasnaur commented Nov 12, 2022

Uh oh!

mixmasala commented Dec 29, 2022

Uh oh!

eliasnaur commented Jan 1, 2023

Uh oh!

eliasnaur left a comment

Choose a reason for hiding this comment

Uh oh!

eliasnaur Jan 7, 2023

Choose a reason for hiding this comment

Uh oh!

eliasnaur Jan 7, 2023

Choose a reason for hiding this comment

Uh oh!

eliasnaur Jan 7, 2023

Choose a reason for hiding this comment

Uh oh!

mixmasala commented Nov 19, 2021 •

edited

Loading