Audio version for Gemini Live To-Do#168
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the sample catalog by adding the Gemini Hybrid sample and renaming Imagen samples to "Nanobanana". The Gemini Live Todo sample is significantly refactored to support a dual-mode (Visual/Audio) experience for AI glasses using the Projected API and Glimmer, alongside updates to the Gemini model and backend. Feedback focuses on several architectural violations of the style guide, including passing Activity references into the TodoScreenViewModel, which risks memory leaks, and direct session access from the UI that breaks Unidirectional Data Flow (UDF). Additionally, several composables are missing required Modifier parameters, and the README needs updating to reflect the change to the Google AI backend.
| } | ||
|
|
||
| @SuppressLint("MissingPermission") | ||
| fun toggleLiveSession(activity: Activity) { |
There was a problem hiding this comment.
The toggleLiveSession function takes an Activity as a parameter. This violates the recommendation that ViewModels should never take an activity as a parameter to avoid memory leaks (Style Guide Line 59). Permission checks and activity-related logic should be handled in the UI layer.
References
- ViewModel functions should never take an activity as a parameter to avoid memory leaks. (link)
| todoRepository.toggleTodoStatus(MIC_TODO_ID) | ||
| } | ||
|
|
||
| fun initializeGeminiLive(activity: Activity) { |
There was a problem hiding this comment.
The initializeGeminiLive function takes an Activity as a parameter, violating Style Guide Line 59. ViewModels should be agnostic of the Android lifecycle. Permission requests and activity-specific initialization should be handled in the UI layer.
References
- ViewModel functions should never take an activity as a parameter to avoid memory leaks. (link)
| internal var session: LiveSession? = null | ||
| private var hostActivityRef: WeakReference<Activity>? = null |
There was a problem hiding this comment.
The session property should be private to adhere to encapsulation and Unidirectional Data Flow (UDF) principles. Additionally, hostActivityRef is a reference to an Activity, which ViewModels should avoid to prevent memory leaks (Style Guide Line 58). Since hostActivityRef is unused in the current implementation, it should be removed. To allow the UI to send voice feedback, expose a dedicated method instead of the session object.
private var session: LiveSession? = null
fun sendVoiceFeedback(text: String) {
viewModelScope.launch {
session?.send(content { text(text) })
}
}References
- ViewModels should be agnostic of the Android lifecycle and avoid references to Activities. (link)
| fun AudioExperience(viewModel: TodoScreenViewModel) { | ||
| val activity = LocalActivity.current as Activity | ||
| val uiState by viewModel.uiState.collectAsStateWithLifecycle() | ||
| val isMicOn = (uiState as? TodoScreenUiState.Success)?.isMicOn ?: false | ||
| val scope = rememberCoroutineScope() | ||
|
|
||
| Box( | ||
| modifier = Modifier | ||
| .fillMaxSize() |
There was a problem hiding this comment.
Every composable function should take a Modifier as its first optional parameter to allow for external styling and layout adjustments, as per Style Guide Line 50. This modifier should then be applied to the root layout of the composable.
| fun AudioExperience(viewModel: TodoScreenViewModel) { | |
| val activity = LocalActivity.current as Activity | |
| val uiState by viewModel.uiState.collectAsStateWithLifecycle() | |
| val isMicOn = (uiState as? TodoScreenUiState.Success)?.isMicOn ?: false | |
| val scope = rememberCoroutineScope() | |
| Box( | |
| modifier = Modifier | |
| .fillMaxSize() | |
| fun AudioExperience(viewModel: TodoScreenViewModel, modifier: Modifier = Modifier) { | |
| val activity = LocalActivity.current as Activity | |
| val uiState by viewModel.uiState.collectAsStateWithLifecycle() | |
| val isMicOn = (uiState as? TodoScreenUiState.Success)?.isMicOn ?: false | |
| val scope = rememberCoroutineScope() | |
| Box( | |
| modifier = modifier | |
| .fillMaxSize() |
References
- Every composable function (except top level screen composable) should take a Modifier as a parameter with a default value. It should be positionned as the first optional parameter. (link)
| scope.launch { | ||
| // Give Gemini Live a moment to open the channel | ||
| kotlinx.coroutines.delay(500) | ||
| viewModel.session?.send(content { text("Turning Microphone On") }) | ||
| } |
There was a problem hiding this comment.
Directly accessing viewModel.session?.send from the UI violates Unidirectional Data Flow (UDF) principles (Style Guide Line 25). The UI should call a method on the ViewModel, which then handles the session interaction. This also allows the ViewModel to manage the session state internally.
viewModel.sendVoiceFeedback("Turning Microphone On")References
- Follow Unidirectional Data Flow (UDF). ViewModels expose UI state using the observer pattern and receive actions via method calls. (link)
| val generativeModel = Firebase.ai(backend = GenerativeBackend.vertexAI()).liveModel( | ||
| "gemini-live-2.5-flash-preview-native-audio-09-2025", | ||
| // See https://firebase.google.com/docs/ai-logic/live-api for an overview of available models | ||
| val generativeModel = Firebase.ai(backend = GenerativeBackend.googleAI()).liveModel( |
There was a problem hiding this comment.
The generativeModel is initialized with GenerativeBackend.googleAI(), but the README.md (line 22) still references GenerativeBackend.vertexAI(). Please update the README to ensure consistency between the documentation and the implementation, as per Style Guide Line 73.
References
- Verify that these changes are properly reflected in the README.md of this sample. (link)
| private fun GlimmerScreenContent( | ||
| uiState: TodoScreenUiState, | ||
| onToggleItem: (Int) -> Unit, | ||
| viewModel: TodoScreenViewModel, | ||
| activity: Activity?, | ||
| onExit: () -> Unit | ||
| ) { |
There was a problem hiding this comment.
This composable function is missing a Modifier parameter. According to Style Guide Line 50, every composable function should take a Modifier as its first optional parameter to support external layout configurations.
References
- Every composable function (except top level screen composable) should take a Modifier as a parameter with a default value. It should be positionned as the first optional parameter. (link)
JolandaVerhoef
left a comment
There was a problem hiding this comment.
Consider addressing the Gemini Code Assist review comments before merging.
Adding audio experience for AI glasses