Add Android Digital Assistant integration#385
Conversation
There was a problem hiding this comment.
Thank you! The situation in the current codebase on master is indeed quite bad (UI not showing automatically on wakeword, and when shown using all screen), and this PR improves on a few of those problems. I think we need to give some thought into which interactions we want the user to be able to do when using this overlay.
I rebased and pushed.
I would move the files from org.stypox.dicio.io.assistant to org.stypox.dicio.overlay, this is unrelated to I/O.
I got a crash by following these steps right after installation:
- DO NOT grant any permission
- Set this version of Dicio as the digital assistant from Settings -> Apps -> Default apps
- Trigger "Voice Assist" from emulator controls, or with the system gesture
- The overlay pops up properly, and the microphone button shows "Grant microphone permission"
- Click on "Grant microphone permission", and you get this crash:
Stacktrace
android.content.ActivityNotFoundException: No Activity found to handle Intent { act=androidx.activity.result.contract.action.REQUEST_PERMISSIONS flg=0x10000000 (has extras) }
at android.app.Instrumentation.checkStartActivityResult(Instrumentation.java:2239)
at android.app.Instrumentation.execStartActivity(Instrumentation.java:1878)
at android.app.ContextImpl.startActivity(ContextImpl.java:1132)
at android.app.ContextImpl.startActivity(ContextImpl.java:1103)
at android.content.ContextWrapper.startActivity(ContextWrapper.java:436)
at org.stypox.dicio.io.assistant.AssistantOverlayService$customActivityResultRegistry$1.onLaunch(AssistantOverlayService.kt:110)
at androidx.activity.result.ActivityResultRegistry$register$3.launch(ActivityResultRegistry.kt:191)
at androidx.activity.compose.ActivityResultLauncherHolder.launch(ActivityResultRegistry.kt:150)
at androidx.activity.compose.ManagedActivityResultLauncher.launch(ActivityResultRegistry.kt:139)
at androidx.activity.result.ActivityResultLauncher.launch(ActivityResultLauncher.kt:37)
at org.stypox.dicio.ui.home.SttButtonKt.SttFab$lambda$1$0(SttButton.kt:81)
at org.stypox.dicio.ui.home.SttButtonKt.$r8$lambda$ADX5ggr9w64gHJNQaE3YfZzSTyE(Unknown Source:0)
at org.stypox.dicio.ui.home.SttButtonKt$$ExternalSyntheticLambda8.invoke(D8$$SyntheticClass:0)
at androidx.compose.foundation.ClickableNode.onPointerEvent-H0pRuoY(Clickable.kt:1009)
at androidx.compose.ui.input.pointer.Node.dispatchMainEventPass(HitPathTracker.kt:436)
at androidx.compose.ui.input.pointer.Node.dispatchMainEventPass(HitPathTracker.kt:422)
at androidx.compose.ui.input.pointer.NodeParent.dispatchMainEventPass(HitPathTracker.kt:275)
at androidx.compose.ui.input.pointer.HitPathTracker.dispatchChanges(HitPathTracker.kt:171)
at androidx.compose.ui.input.pointer.PointerInputEventProcessor.process-BIzXfog(PointerInputEventProcessor.kt:118)
at androidx.compose.ui.platform.AndroidComposeView.sendMotionEvent-8iAsVTc(AndroidComposeView.android.kt:2428)
at androidx.compose.ui.platform.AndroidComposeView.handleMotionEvent-8iAsVTc(AndroidComposeView.android.kt:2378)
at androidx.compose.ui.platform.AndroidComposeView.dispatchTouchEvent(AndroidComposeView.android.kt:2249)
at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3120)
at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2801)
at android.view.View.dispatchPointerEvent(View.java:15919)
at android.view.ViewRootImpl$ViewPostImeInputStage.processPointerEvent(ViewRootImpl.java:7021)
at android.view.ViewRootImpl$ViewPostImeInputStage.onProcess(ViewRootImpl.java:6815)
at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:6229)
at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:6286)
at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:6252)
at android.view.ViewRootImpl$AsyncInputStage.forward(ViewRootImpl.java:6417)
at android.view.ViewRootImpl$InputStage.apply(ViewRootImpl.java:6260)
at android.view.ViewRootImpl$AsyncInputStage.apply(ViewRootImpl.java:6474)
at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:6233)
at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:6286)
at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:6252)
at android.view.ViewRootImpl$InputStage.apply(ViewRootImpl.java:6260)
at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:6233)
at android.view.ViewRootImpl.deliverInputEvent(ViewRootImpl.java:9211)
at android.view.ViewRootImpl.doProcessInputEvents(ViewRootImpl.java:9162)
at android.view.ViewRootImpl.enqueueInputEvent(ViewRootImpl.java:9131)
at android.view.ViewRootImpl$WindowInputEventReceiver.onInputEvent(ViewRootImpl.java:9337)
at android.view.InputEventReceiver.dispatchInputEvent(InputEventReceiver.java:267)
at android.os.MessageQueue.nativePollOnce(Native Method)
at android.os.MessageQueue.next(MessageQueue.java:335)
at android.os.Looper.loopOnce(Looper.java:162)
at android.os.Looper.loop(Looper.java:294)
at android.app.ActivityThread.main(ActivityThread.java:8177)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:552)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:971)
Clicking "Grant microphone permission" in MainActivity still works just fine.
There was a problem hiding this comment.
Instead of creating new components from scratch, can't you reuse the other components, and just pass modifiers (or other parameters) to make them smaller?
| class AssistantLauncherActivity : ComponentActivity() { | ||
| override fun onCreate(savedInstanceState: Bundle?) { | ||
| super.onCreate(savedInstanceState) | ||
| AssistantOverlayService.start(this) |
There was a problem hiding this comment.
If MainActivity is already shown in foreground, then the overlay should not be started. And in any case the overlay should be dismissed if it's open when MainActivity transitions from background (or fully closed) to foreground.
| import javax.inject.Inject | ||
|
|
||
| @AndroidEntryPoint | ||
| class AssistantOverlayService : Service(), LifecycleOwner, ViewModelStoreOwner, SavedStateRegistryOwner, ActivityResultRegistryOwner { |
There was a problem hiding this comment.
Why make a service and not an activity?
E.g. in NewPipe we have a transparent-background activity, see https://github.com/TeamNewPipe/NewPipe/blob/dev/app/src/main/java/org/schabi/newpipe/RouterActivity.java . Or is there some difference I am not understanding when it comes to overlays? Ah ok yeah, apparently an overlay is special in the sense that you can still interact with apps below. NewPipe uses an overlay for the popup player: https://github.com/TeamNewPipe/NewPipe/blob/dev/app/src/main/java/org/schabi/newpipe/player/ui/PopupPlayerUi.java .
Is there no way to show an activity as an overlay? I think having simply MainActivity change shape (depending on whether it should be overlaid or not) would simplify a lot of stuff. It would also allow making nice transitions from the small overlay assistant to the fullscreen assistant (e.g. through an "enlarge" button). Also a bunch of stuff in this class is duplicate with MainActivity, which would be avoided (e.g. if MainActivity can't be used directly, at least use a base class with common functions).
Actually, now that I think of it, do we really need the user to still be able to interact with other apps below the overlay? Since the overlay uses half of the screen it's hard to do anything meaningful on the rest of the screen. If I really wanted to do Dicio-assisted stuff on my phone I would probably use the phone's split-screen feature and obtain a better experience probably. This is very much up for debate though: we need to clarify what problems exactly this overlay is trying to solve.
Maybe instead we could show a normal bottom sheet popup (just like SttPopupActivity) that turns into a "bubble" when dismissed, and then quickly reopens from the bubble when the wakeword is triggered or when tapping on the bubble. What do you think?
| interactionLog = interactionLog, | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .height(200.dp) |
There was a problem hiding this comment.
This should adapt properly to different screen sizes: e.g. it could be max(1/3*screen_height, min(screen_height, 200.dp)) (feel free to finetune the values)
| onDismiss: () -> Unit, | ||
| ) { | ||
| // Use a simple MaterialTheme without the Activity-dependent SideEffect | ||
| val colorScheme = if (isSystemInDarkTheme()) darkColorScheme() else lightColorScheme() |
There was a problem hiding this comment.
This seems to use a wrong theme (?). I get purple buttons. Also, please add a boolean parameter to AppTheme to disable Activity side-effects, instead of reinventing the wheel.
| } | ||
| } | ||
|
|
||
| private fun showOverlay() { |
There was a problem hiding this comment.
If possible, it would be good if you could move some of these standalone helper functions in a separate file, so it's easier to follow what is happening in this class.
| CompositionLocalProvider( | ||
| androidx.activity.compose.LocalActivityResultRegistryOwner provides this@AssistantOverlayService | ||
| ) { | ||
| AssistantOverlayContent( |
There was a problem hiding this comment.
I would inline AssistantOverlayContent directly here, it's just a wrapper around AssistantOverlay anyway.
| setViewTreeLifecycleOwner(this@AssistantOverlayService) | ||
| setViewTreeViewModelStoreOwner(this@AssistantOverlayService) | ||
| setViewTreeSavedStateRegistryOwner(this@AssistantOverlayService) | ||
| setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed) | ||
|
|
||
| setContent { | ||
| CompositionLocalProvider( | ||
| androidx.activity.compose.LocalActivityResultRegistryOwner provides this@AssistantOverlayService | ||
| ) { |
There was a problem hiding this comment.
What is all of this Compose setup? I guess it's necessary because you are in a Service? Please add comments and documentation source.
| hideOverlay() | ||
| stopSelf() | ||
| } | ||
| else -> { |
There was a problem hiding this comment.
This branch is duplicate with ACTION_SHOW_OVERLAY. Also, maybe we should just ignore unknown intents (up for debate)?
| android:name=".io.assistant.AssistantOverlayService" | ||
| android:enabled="true" | ||
| android:exported="false" | ||
| android:foregroundServiceType="specialUse" |
There was a problem hiding this comment.
Is this supposed to be specialUse or microphone? Please add a comment or a documentation source.
6b54dbd to
861e86e
Compare
This PR adds setting Dicio as the default Digital Assistant for Android. Invoking Dicio from the hot word or assistant shortcut now brings up a small window to show user input and Dicio output in a scrollable chat window. If the user activates Dicio from a screen-off state, there is a one-minute timeout for the window before it closes automatically.
Fixes #247 fixes #68 fixes #154