Make changes to allow health connect to work on Android 9 and higher #4790

dshokouhi · 2024-11-08T00:10:07Z

Summary

Makes requestSensorUpdate and checkPermissions suspend functions to allow health connect to work on Android 9 and higher devices instead of just Android 14+. Some other methods were also converted to make the changes here easier to follow (I hope)

Screenshots

Link to pull request in Documentation repository

Documentation: home-assistant/companion.home-assistant#1135

Any other notes

jpelgrom · 2024-11-08T05:46:55Z

Broadcast receivers are short lived and as I understand it may be killed even earlier if you launch (run work asynchronously) without indicating it to the system. Have you tested/encountered this with the app running in the background? Some nice discussions and a Kotlin extension function for it here: https://stackoverflow.com/questions/22741202/how-to-use-goasync-for-broadcastreceiver

dshokouhi · 2024-11-08T05:54:12Z

Have you tested/encountered this with the app running in the background?

So have not enabled all sensors but so far working well about 7 hours in

health connect

dshokouhi · 2024-11-08T06:03:11Z

device tracker also updating regularly during this time

dshokouhi · 2024-11-09T00:05:20Z

24 hours later all enabled sensors including location tracking still working as expected

jpelgrom

You're using MainScope().launch { } in a couple of places, but also a regular CoroutineScope in another one or two. Would it be worth it to introduce a helper function for it, or a custom scope like sensorWorkerScope (which is defined in the base class as = MainScope()) so it is easier to track and update later?

...va/io/homeassistant/companion/android/onboarding/integration/MobileAppIntegrationFragment.kt

dshokouhi · 2024-11-09T18:29:59Z

You're using MainScope().launch { } in a couple of places, but also a regular CoroutineScope in another one or two.

The reason for that is because we had it split up. Specifically the android auto connection sensors requires the CoroutineScope or it fails to update.

Would it be worth it to introduce a helper function for it, or a custom scope like sensorWorkerScope (which is defined in the base class as = MainScope()) so it is easier to track and update later?

if we were to do this it would need to be teh CoroutineScope

jpelgrom · 2024-11-09T18:35:57Z

Specifically the android auto connection sensors requires the CoroutineScope or it fails to update

That's odd because you're still using the main/UI thread, only difference is the supervisor behavior which shouldn't matter.

dshokouhi · 2024-11-09T18:36:55Z

Specifically the android auto connection sensors requires the CoroutineScope or it fails to update

That's odd because you're still using the main/UI thread, only difference is the supervisor behavior which shouldn't matter.

was failing due to the thread not calling Looper.prepare() dont have teh exact traceback now but that was the root cause

...on/src/main/java/io/homeassistant/companion/android/common/sensors/BluetoothSensorManager.kt

app/src/full/java/io/homeassistant/companion/android/sensors/AndroidAutoSensorManager.kt

app/src/main/java/io/homeassistant/companion/android/sensors/CarSensorManager.kt

app/src/full/java/io/homeassistant/companion/android/sensors/HealthConnectSensorManager.kt

jpelgrom · 2024-11-09T21:25:06Z

Testing this on Android 13, requesting Health Connect permissions seems to be broken (immediately denied) and tapping on settings in the snackbar crashes the app:

2024-11-09 22:23:42.768 23522-23522 AndroidRuntime          io....stant.companion.android.debug  E  FATAL EXCEPTION: main
                                                                                                    Process: io.homeassistant.companion.android.debug, PID: 23522
                                                                                                    android.content.ActivityNotFoundException: No Activity found to handle Intent { act=androidx.health.ACTION_REQUEST_PERMISSIONS pkg=io.homeassistant.companion.android.debug (has extras) }
                                                                                                    	at android.app.Instrumentation.checkStartActivityResult(Instrumentation.java:2200)
                                                                                                    	at android.app.Instrumentation.execStartActivity(Instrumentation.java:1839)
                                                                                                    	at android.app.Activity.startActivityForResult(Activity.java:5507)
                                                                                                    	at androidx.activity.ComponentActivity.startActivityForResult(ComponentActivity.kt:704)
                                                                                                    	at androidx.core.app.ActivityCompat.startActivityForResult(ActivityCompat.java:244)
                                                                                                    	at androidx.activity.ComponentActivity$activityResultRegistry$1.onLaunch(ComponentActivity.kt:225)
                                                                                                    	at androidx.activity.result.ActivityResultRegistry$register$3.launch(ActivityResultRegistry.kt:199)
                                                                                                    	at androidx.activity.compose.ActivityResultLauncherHolder.launch(ActivityResultRegistry.kt:154)
                                                                                                    	at androidx.activity.compose.ManagedActivityResultLauncher.launch(ActivityResultRegistry.kt:143)
                                                                                                    	at androidx.activity.result.ActivityResultLauncher.launch(ActivityResultLauncher.kt:37)
                                                                                                    	at io.homeassistant.companion.android.settings.sensor.views.SensorDetailViewKt$SensorDetailView$2$1$1.invokeSuspend(SensorDetailView.kt:119)
                                                                                                    	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
                                                                                                    	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:101)
                                                                                                    	at androidx.compose.ui.platform.AndroidUiDispatcher.performTrampolineDispatch(AndroidUiDispatcher.android.kt:81)
                                                                                                    	at androidx.compose.ui.platform.AndroidUiDispatcher.access$performTrampolineDispatch(AndroidUiDispatcher.android.kt:41)
                                                                                                    	at androidx.compose.ui.platform.AndroidUiDispatcher$dispatchCallback$1.run(AndroidUiDispatcher.android.kt:57)
                                                                                                    	at android.os.Handler.handleCallback(Handler.java:942)
                                                                                                    	at android.os.Handler.dispatchMessage(Handler.java:99)
                                                                                                    	at android.os.Looper.loopOnce(Looper.java:201)
                                                                                                    	at android.os.Looper.loop(Looper.java:288)
                                                                                                    	at android.app.ActivityThread.main(ActivityThread.java:7918)
                                                                                                    	at java.lang.reflect.Method.invoke(Native Method)
                                                                                                    	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
                                                                                                    	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:936)
                                                                                                    	Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [androidx.compose.ui.platform.MotionDurationScaleImpl@1008cfc, androidx.compose.runtime.BroadcastFrameClock@f18ff85, StandaloneCoroutine{Cancelling}@15d81da, AndroidUiDispatcher@afce60b]

dshokouhi · 2024-11-09T22:33:15Z

Testing this on Android 13, requesting Health Connect permissions seems to be broken (immediately denied) and tapping on settings in the snackbar crashes the app:

thanks I was able to hunt down an older device will use that for testing and update this PR

…alth permissions

dshokouhi · 2024-11-10T00:20:58Z

Fixed, also the snackbar has been fixed too. Just an FYI these permissions are also subject to not being prompted multiple times if they are continuously denied.

Verified on my old Pixel 3 XL running android 12

jpelgrom · 2024-11-12T20:05:22Z

also the snackbar has been fixed too

I'm not completely sure what you mean. For me, if I deny the permission twice tapping Settings on the snackbar will completely stop working (nothing happens/the system auto-denies).

Shouldn't the settings button link to an intent with ACTION_MANAGE_HEALTH_PERMISSIONS? That seems like it would always work.

dshokouhi · 2024-11-12T20:07:25Z

I'm not completely sure what you mean. For me, if I deny the permission twice tapping Settings on the snackbar will completely stop working (nothing happens/the system auto-denies).

that should behave like the other sensors right?

Shouldn't the settings button link to an intent with ACTION_MANAGE_HEALTH_PERMISSIONS? That seems like it would always work.

thats only for android 14+, we can launch the HC app though

https://developer.android.com/reference/kotlin/androidx/health/connect/client/HealthConnectClient#ACTION_HEALTH_CONNECT_SETTINGS()

jpelgrom · 2024-11-12T20:08:31Z

that should behave like the other sensors right?

Currently: no it doesn't, anything normal opens system settings for permissions which always works.

Ideally: yes it should behave like others and always work.

thats only for android 14+, we can launch the HC app though

https://developer.android.com/reference/kotlin/androidx/health/connect/client/HealthConnectClient#ACTION_HEALTH_CONNECT_SETTINGS()

Oops / that seems like a good option :) (need to put it in the SM to avoid the dependency in minimal)

app/src/main/java/io/homeassistant/companion/android/settings/sensor/views/SensorDetailView.kt

jpelgrom

Looks good and seems to work as before during testing, but this should be in beta for a while to make sure there is no unexpected impact.

dshokouhi · 2024-11-12T21:05:36Z

Looks good and seems to work as before during testing, but this should be in beta for a while to make sure there is no unexpected impact.

Agreed this needs beta time

Make changes to allow health connect to work on Android9 and higher

96f84f7

home-assistant bot added the cla-signed label Nov 8, 2024

dshokouhi marked this pull request as draft November 8, 2024 00:10

Fix minimal build

3f579dc

dshokouhi marked this pull request as ready for review November 8, 2024 01:07

Fix wear OS permission check update

cc84dfa

dshokouhi mentioned this pull request Nov 9, 2024

Add docs for health connect support on lower android versions home-assistant/companion.home-assistant#1135

Merged

jpelgrom reviewed Nov 9, 2024

View reviewed changes

...va/io/homeassistant/companion/android/onboarding/integration/MobileAppIntegrationFragment.kt Outdated Show resolved Hide resolved

Review comments

32e0f0b

jpelgrom reviewed Nov 9, 2024

View reviewed changes

...on/src/main/java/io/homeassistant/companion/android/common/sensors/BluetoothSensorManager.kt Outdated Show resolved Hide resolved

Revert bluetooth sensor manager changes for sensorWorkerScope

4256755

jpelgrom reviewed Nov 9, 2024

View reviewed changes

app/src/full/java/io/homeassistant/companion/android/sensors/AndroidAutoSensorManager.kt Show resolved Hide resolved

app/src/main/java/io/homeassistant/companion/android/sensors/CarSensorManager.kt Outdated Show resolved Hide resolved

jpelgrom reviewed Nov 9, 2024

View reviewed changes

app/src/full/java/io/homeassistant/companion/android/sensors/HealthConnectSensorManager.kt Outdated Show resolved Hide resolved

Review comments

57d0789

Fix permission granting in lower android versions and snackbar for he…

a6ec697

…alth permissions

dshokouhi mentioned this pull request Nov 10, 2024

Android Health Connect API #2537

Closed

Use action intent for snackbar

676acf0

Add missing import

a292231

jpelgrom reviewed Nov 12, 2024

View reviewed changes

app/src/main/java/io/homeassistant/companion/android/settings/sensor/views/SensorDetailView.kt Outdated Show resolved Hide resolved

Play it safe and clean up

0f9c60b

jpelgrom approved these changes Nov 12, 2024

View reviewed changes

dshokouhi merged commit 5916092 into home-assistant:master Nov 12, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make changes to allow health connect to work on Android 9 and higher #4790

Make changes to allow health connect to work on Android 9 and higher #4790

dshokouhi commented Nov 8, 2024 •

edited

Loading

jpelgrom commented Nov 8, 2024

dshokouhi commented Nov 8, 2024

dshokouhi commented Nov 8, 2024

dshokouhi commented Nov 9, 2024

jpelgrom left a comment

dshokouhi commented Nov 9, 2024

jpelgrom commented Nov 9, 2024

dshokouhi commented Nov 9, 2024

jpelgrom commented Nov 9, 2024 •

edited

Loading

dshokouhi commented Nov 9, 2024

dshokouhi commented Nov 10, 2024

jpelgrom commented Nov 12, 2024

dshokouhi commented Nov 12, 2024

jpelgrom commented Nov 12, 2024 •

edited

Loading

jpelgrom left a comment

dshokouhi commented Nov 12, 2024

Make changes to allow health connect to work on Android 9 and higher #4790

Make changes to allow health connect to work on Android 9 and higher #4790

Conversation

dshokouhi commented Nov 8, 2024 • edited Loading

Summary

Screenshots

Link to pull request in Documentation repository

Any other notes

jpelgrom commented Nov 8, 2024

dshokouhi commented Nov 8, 2024

dshokouhi commented Nov 8, 2024

dshokouhi commented Nov 9, 2024

jpelgrom left a comment

Choose a reason for hiding this comment

dshokouhi commented Nov 9, 2024

jpelgrom commented Nov 9, 2024

dshokouhi commented Nov 9, 2024

jpelgrom commented Nov 9, 2024 • edited Loading

dshokouhi commented Nov 9, 2024

dshokouhi commented Nov 10, 2024

jpelgrom commented Nov 12, 2024

dshokouhi commented Nov 12, 2024

jpelgrom commented Nov 12, 2024 • edited Loading

jpelgrom left a comment

Choose a reason for hiding this comment

dshokouhi commented Nov 12, 2024

dshokouhi commented Nov 8, 2024 •

edited

Loading

jpelgrom commented Nov 9, 2024 •

edited

Loading

jpelgrom commented Nov 12, 2024 •

edited

Loading