app, cmd/gogio: add android foreground permission and service - #67

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.

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.

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

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.

edit: fixed

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

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.

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
+}

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.

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?

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).

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