Fix unable to Toast on QS tiles being clicked error

[dshokouhi]

Summary

Fixes the following sentry errors:

java.lang.NullPointerException: Can't toast on a thread that has not called Looper.prepare()
    at com.android.internal.util.Preconditions.checkNotNull(Preconditions.java:167)
    at android.widget.Toast.getLooper(Toast.java:265)
    at android.widget.Toast.<init>(Toast.java:250)
    at android.widget.Toast.makeText(Toast.java:865)
    at android.widget.Toast.makeText(Toast.java:853)
    at android.widget.Toast.makeText(Toast.java:892)
    at io.homeassistant.companion.android.qs.TileExtensions$tileClicked$2.invokeSuspend(TileExtensions.kt:152)
    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
    at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
    at kotlinx.coroutines.internal.LimitedDispatcher.run(LimitedDispatcher.kt:42)
    at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:95)
    at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:570)
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:750)
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:677)
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:664)

and

java.lang.RuntimeException: Can't toast on a thread that has not called Looper.prepare()
    at android.widget.Toast$TN.<init>(Toast.java:474)
    at android.widget.Toast.<init>(Toast.java:142)
    at android.widget.Toast.makeText(Toast.java:333)
    at android.widget.Toast.makeText(Toast.java:323)
    at android.widget.Toast.makeText(Toast.java:370)
    at io.homeassistant.companion.android.qs.TileExtensions$tileClicked$2.invokeSuspend(TileExtensions.kt:152)
    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
    at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
    at kotlinx.coroutines.internal.LimitedDispatcher.run(LimitedDispatcher.kt:42)
    at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:95)
    at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:570)
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:750)
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:677)
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:664)

Screenshots

Link to pull request in Documentation repository

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

Any other notes

[jpelgrom]

It works, but considering coroutines are used wouldn't using withContext(Dispatchers.Main) { ... } be a cleaner fix?

(Also the UX of this isn't great, the toast isn't visible without collapsing the notification drawer)

[dshokouhi]

It works, but considering coroutines are used wouldn't using withContext(Dispatchers.Main) { ... } be a cleaner fix?

I thought that at first but if you notice the first Toast message where the actual error points to is already wrapped in withContext() so I took it as we need to use Looper still. So you're saying we should use 2 withContext calls within each other here?

(Also the UX of this isn't great, the toast isn't visible without collapsing the notification drawer)

I agree, we could temporarily change the tile but then we would need to change it back after some delay. Posting a notification if it fails feels a bit intrusive too.

[jpelgrom]

I thought that at first but if you notice the first Toast message where the actual error points to is already wrapped in withContext() so I took it as we need to use Looper still. So you're saying we should use 2 withContext calls within each other here?

The outer one is withContext(Dispatchers.IO) right? It looks like a classic example of "the app will crash if you do UI stuff on a non-UI thread" to me.

(Also the UX of this isn't great, the toast isn't visible without collapsing the notification drawer)

I agree, we could temporarily change the tile but then we would need to change it back after some delay. Posting a notification if it fails feels a bit intrusive too.

Yes the options for feedback are limited with tiles. Not something for this PR :)

[dshokouhi]

The outer one is withContext(Dispatchers.IO) right?

Correct, so this should be changed to use the main dispatcher there then?

[jpelgrom]

The outer one is withContext(Dispatchers.IO) right?

Correct, so this should be changed to use the main dispatcher there then?

I suggest replacing Handler(Looper.getMainLooper()).post { with withContext(Dispatchers.Main) {, only the Toast needs to be posted from the UI thread. I don't know why withContext(Dispatchers.IO) was originally used but presumably it was necessary.

[dshokouhi]

I don't know why withContext(Dispatchers.IO) was originally used but presumably it was necessary.

It was using runBlocking{} before which was an inappropriate blocking call here. I had switched to withContext() based on a recommendation I saw on Stackoverflow. Its necessary because we need to wait for some output to properly update the tile based on the response.