Update the device availability snippets#908
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the collectDeviceLifecycle function to reactively monitor projected device connection states using flatMapLatest and isProjectedDeviceConnected. The changes include adding necessary API requirements and experimental annotations. Feedback focuses on improving the formatting of the @OptIn annotation and ensuring that CancellationException is not swallowed in the try-catch block, which is critical for proper coroutine cancellation behavior.
| @OptIn( | ||
| ExperimentalXrDeviceLifecycleApi::class, ExperimentalProjectedApi::class, | ||
| ExperimentalCoroutinesApi::class, | ||
|
|
||
| ) |
There was a problem hiding this comment.
The @OptIn annotation formatting can be improved by removing the unnecessary empty line and consistently placing each experimental API on its own line for better readability.
| @OptIn( | |
| ExperimentalXrDeviceLifecycleApi::class, ExperimentalProjectedApi::class, | |
| ExperimentalCoroutinesApi::class, | |
| ) | |
| @OptIn( | |
| ExperimentalXrDeviceLifecycleApi::class, | |
| ExperimentalProjectedApi::class, | |
| ExperimentalCoroutinesApi::class | |
| ) |
| } catch (e: Exception) { | ||
| flowOf(Lifecycle.State.DESTROYED) | ||
| } |
There was a problem hiding this comment.
Catching a broad Exception can inadvertently intercept CancellationException, which is essential for the proper functioning of Kotlin coroutines (e.g., when flatMapLatest cancels the previous block). It is recommended to explicitly rethrow CancellationException to maintain proper structured concurrency behavior. Additionally, if you know the specific exceptions thrown by the API (such as IllegalStateException), it is better to catch those specifically.
| } catch (e: Exception) { | |
| flowOf(Lifecycle.State.DESTROYED) | |
| } | |
| } catch (e: Exception) { | |
| if (e is kotlinx.coroutines.CancellationException) throw e | |
| flowOf(Lifecycle.State.DESTROYED) | |
| } |
There was a problem hiding this comment.
Done! catching an IllegalStateException now.
Update the device availability snippets to check for projected device connection first before getting the lifecycle.