feat(voice): add Flutter UI and platform channel for voice profile se…#772
Open
Arunodoy18 wants to merge 1 commit intoAOSSIE-Org:masterfrom
Open
feat(voice): add Flutter UI and platform channel for voice profile se…#772Arunodoy18 wants to merge 1 commit intoAOSSIE-Org:masterfrom
Arunodoy18 wants to merge 1 commit intoAOSSIE-Org:masterfrom
Conversation
Contributor
|
🎉 Welcome @Arunodoy18!
We appreciate your contribution! 🚀 |
📝 WalkthroughWalkthroughIntroduces a cross-platform voice control feature with Flutter UI components and platform channels. Adds Android and iOS MethodChannel handlers for voice profile and preview state management, paired with Dart controller, service, and screen implementations for voice profile selection. Changes
Sequence DiagramsequenceDiagram
participant User as User (UI)
participant Screen as VoiceProfileScreen
participant Controller as VoiceProfileController
participant Service as VoiceControlService
participant Channel as MethodChannel
participant Native as Native Platform<br/>(Android/iOS)
User->>Screen: Select voice profile<br/>or toggle preview
Screen->>Controller: onVoiceProfileChanged(voice)<br/>or onPreviewToggled(value)
Controller->>Controller: Update observable<br/>(selectedVoice/<br/>isPreviewEnabled)
Controller->>Service: setVoiceProfile(voice)<br/>or setPreviewEnabled(enabled)
Service->>Channel: invokeMethod('setVoiceProfile')<br/>or ('setPreviewEnabled')
Channel->>Native: Route to platform handler<br/>(MainActivity/AppDelegate)
Native->>Native: applyVoiceProfile(profile)<br/>or setVoicePreviewEnabled(enabled)
Native-->>Channel: Return success
Channel-->>Service: Complete Future
Note over Controller,Native: Initial state sent<br/>on controller onInit()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
lib/routes/app_pages.dart (1)
153-156: Missingbinding:for the voice profile route — pairs with theGet.putlifecycle issue inVoiceProfileScreen.Without a
Binding, GetX does not automatically create or disposeVoiceProfileControlleras part of route navigation. Add aVoiceProfileBinding(see the diff proposed invoice_profile_screen.dart) to properly wire controller lifecycle to route entry/exit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/routes/app_pages.dart` around lines 153 - 156, The GetPage for AppRoutes.voiceProfileScreen is missing a binding so the VoiceProfileController isn't created/disposed with navigation; update the GetPage declaration for AppRoutes.voiceProfileScreen to include binding: VoiceProfileBinding (so GetX will instantiate VoiceProfileController per route) and ensure VoiceProfileBinding registers the controller (e.g., via Get.put/Get.lazyPut inside VoiceProfileBinding), matching the controller lifecycle expected by VoiceProfileScreen.ios/Runner/AppDelegate.swift (1)
27-67: StorevoiceChannelas a property to make the lifecycle explicit.
voiceChannelis a localletthat goes out of scope afterdidFinishLaunchingWithOptionsreturns. While Flutter's binary messenger internally retains the handler block, this is an implementation detail. The conventional pattern for iOS platform channels stores the channel as a class-level property (e.g.,private var voiceChannel: FlutterMethodChannel?) so it can be explicitly torn down and so the pattern is unambiguously safe when the channel later needs to invoke methods from native→Flutter.♻️ Suggested refactor
`@objc` class AppDelegate: FlutterAppDelegate { private let voiceControlChannelName = "voice_control_channel" + private var voiceChannel: FlutterMethodChannel? // ...inside didFinishLaunchingWithOptions: - let voiceChannel = FlutterMethodChannel( + voiceChannel = FlutterMethodChannel( name: voiceControlChannelName, binaryMessenger: controller.binaryMessenger )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/Runner/AppDelegate.swift` around lines 27 - 67, Make voiceChannel a class-level property (e.g., private var voiceChannel: FlutterMethodChannel?) instead of a local let in didFinishLaunchingWithOptions so its lifecycle is explicit; initialize it there using voiceControlChannelName and controller.binaryMessenger, assign the handler that calls applyVoiceProfile and setVoicePreviewEnabled as shown, and ensure you clear/tear down voiceChannel (set to nil) when appropriate (e.g., on deinit or when the Flutter engine is invalidated) to avoid relying on messenger retention details.android/app/src/main/kotlin/com/resonate/resonate/MainActivity.kt (2)
15-49: Consider storing theMethodChannelas a member property and cleaning it up incleanUpFlutterEngine.Without a stored reference, you can't call
setMethodCallHandler(null)on teardown. For a single-engineFlutterActivitythis is low-impact, but it deviates from the Android embedding best-practice and will matter if this activity is ever used with multiple engine instances.♻️ Suggested refactor
class MainActivity : FlutterActivity() { private val channelName = "voice_control_channel" + private var voiceChannel: MethodChannel? = null override fun configureFlutterEngine(flutterEngine: FlutterEngine) { super.configureFlutterEngine(flutterEngine) - MethodChannel( + voiceChannel = MethodChannel( flutterEngine.dartExecutor.binaryMessenger, channelName ).setMethodCallHandler { call, result -> // ... } } + override fun cleanUpFlutterEngine(flutterEngine: FlutterEngine) { + voiceChannel?.setMethodCallHandler(null) + super.cleanUpFlutterEngine(flutterEngine) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/kotlin/com/resonate/resonate/MainActivity.kt` around lines 15 - 49, Store the MethodChannel instance as a MainActivity member (e.g., a private lateinit var methodChannel) instead of creating it inline, assign it where you now call MethodChannel(...).setMethodCallHandler and use that member in your call handling for methods like "setVoiceProfile" (applyVoiceProfile) and "setPreviewEnabled" (setVoicePreviewEnabled); then override/implement cleanUpFlutterEngine to call methodChannel.setMethodCallHandler(null) to release the handler on teardown. Ensure the member is initialized with flutterEngine.dartExecutor.binaryMessenger and that cleanUpFlutterEngine checks for initialization/null before clearing.
10-10: Channel name lacks reverse-domain namespacing.The Flutter docs recommend a reverse-domain prefix (e.g.,
"com.resonate.resonate/voice_control") to avoid collisions with third-party plugins that may register a channel with the same generic name.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/kotlin/com/resonate/resonate/MainActivity.kt` at line 10, The channel name constant channelName in MainActivity.kt uses a generic string and should be changed to a reverse-domain namespaced identifier (e.g., "com.resonate.resonate/voice_control"); update the private val channelName declaration accordingly and then update any Flutter/Dart code that references the old "voice_control_channel" string to use the new namespaced value so both sides match (ensure MainActivity's channel registration and the Dart MethodChannel/BasicMessageChannel use the identical new name).lib/views/screens/voice_profile_screen.dart (1)
6-10:Get.putin aStatelessWidgetfield bypasses GetX route lifecycle management.When navigating back, GetX automatically calls
onClose()and deletes controllers that were registered via aBinding. BecauseVoiceProfileControlleris registered withGet.putinside the widget's field rather than through aBinding, GetX's routing system does not track it and the controller is not cleaned up when the route is popped — it stays in memory for the app's lifetime.The idiomatic fix is a dedicated
Bindingclass (also registered inapp_pages.dart) andGet.findin the screen:♻️ Suggested refactor
Create
lib/bindings/voice_profile_binding.dart:import 'package:get/get.dart'; import '../controllers/voice_profile_controller.dart'; class VoiceProfileBinding extends Bindings { `@override` void dependencies() { Get.lazyPut<VoiceProfileController>(() => VoiceProfileController()); } }In
voice_profile_screen.dart:-class VoiceProfileScreen extends StatelessWidget { - VoiceProfileScreen({super.key}); - - final VoiceProfileController controller = - Get.put(VoiceProfileController()); +class VoiceProfileScreen extends StatelessWidget { + const VoiceProfileScreen({super.key}); + + `@override` + Widget build(BuildContext context) { + final controller = Get.find<VoiceProfileController>();In
app_pages.dart:- GetPage( - name: AppRoutes.voiceProfileScreen, - page: () => VoiceProfileScreen(), - ), + GetPage( + name: AppRoutes.voiceProfileScreen, + page: () => const VoiceProfileScreen(), + binding: VoiceProfileBinding(), + ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/views/screens/voice_profile_screen.dart` around lines 6 - 10, Replace the direct Get.put usage in the VoiceProfileScreen field with lifecycle-managed binding: create a VoiceProfileBinding class that implements Bindings and registers VoiceProfileController via Get.lazyPut (or Get.put) in its dependencies(), register that Binding with the VoiceProfileScreen route in your routing setup (app_pages.dart), and update VoiceProfileScreen to retrieve the controller via Get.find<VoiceProfileController>() (not Get.put) so GetX will track and call onClose() when the route is popped.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ios/Runner/AppDelegate.swift`:
- Around line 46-47: The calls using optional self
(self?.applyVoiceProfile(selectedVoice) and
self?.setVoicePreviewEnabled(isPreviewEnabled)) must not silently noop while
still returning success; update both handlers (the applyVoiceProfile and
setVoicePreviewEnabled call sites in AppDelegate.swift) to unwrap self with a
guard (or if let) and return a Flutter error via result(...) when self is nil,
so Flutter receives failure instead of unconditional success; ensure you use the
same pattern for both the applyVoiceProfile(selectedVoice) block and the
setVoicePreviewEnabled(isPreviewEnabled) block (and the analogous occurrences
mentioned around the second block) so the method returns early with a
descriptive FlutterError when self cannot be obtained.
In `@lib/controllers/voice_profile_controller.dart`:
- Around line 29-39: Both handlers currently mutate reactive state before
awaiting the platform/channel call which can desync UI if the call fails; move
the assignments so the observable updates only after the await completes
successfully. Specifically, in onVoiceProfileChanged, call await
_sendVoiceProfile() first and only set selectedVoice.value = voice after that
succeeds (catch PlatformException/any error and avoid changing selectedVoice or
revert it). Likewise, in onPreviewToggled, call await _sendPreviewState() before
setting isPreviewEnabled.value = value and handle errors to keep UI and native
audio engine consistent. Ensure both methods catch and handle exceptions from
_sendVoiceProfile and _sendPreviewState to prevent stale UI state.
- Around line 20-26: onInit currently fires _sendVoiceProfile() and
_sendPreviewState() without handling their returned Futures, so any
PlatformException is unobserved; update onInit to call each future and attach
proper error handling (e.g., _sendVoiceProfile().catchError(...) and
_sendPreviewState().catchError(...)) to log the PlatformException and apply a
safe fallback state; do not rely on making onInit async (GetX won't await it) —
explicitly handle errors on the Futures returned by _sendVoiceProfile and
_sendPreviewState so the native layer isn't left in an unknown state.
In `@lib/services/voice_control_service.dart`:
- Around line 11-23: The setVoiceProfile and setPreviewEnabled methods call
_channel.invokeMethod without handling PlatformException, which allows errors to
propagate and cause silent unhandled Futures and UI/native state desync; wrap
each invokeMethod call in a try/catch that catches PlatformException, log/report
the error via your logger, and surface failure to callers (e.g., return a bool
or rethrow a wrapped exception) so callers like VoiceProfileController
(_sendVoiceProfile, _sendPreviewState, onVoiceProfileChanged, onPreviewToggled)
can revert reactive state or show an error; ensure the methods' signatures and
return values are adjusted so callers can act on failure.
---
Nitpick comments:
In `@android/app/src/main/kotlin/com/resonate/resonate/MainActivity.kt`:
- Around line 15-49: Store the MethodChannel instance as a MainActivity member
(e.g., a private lateinit var methodChannel) instead of creating it inline,
assign it where you now call MethodChannel(...).setMethodCallHandler and use
that member in your call handling for methods like "setVoiceProfile"
(applyVoiceProfile) and "setPreviewEnabled" (setVoicePreviewEnabled); then
override/implement cleanUpFlutterEngine to call
methodChannel.setMethodCallHandler(null) to release the handler on teardown.
Ensure the member is initialized with flutterEngine.dartExecutor.binaryMessenger
and that cleanUpFlutterEngine checks for initialization/null before clearing.
- Line 10: The channel name constant channelName in MainActivity.kt uses a
generic string and should be changed to a reverse-domain namespaced identifier
(e.g., "com.resonate.resonate/voice_control"); update the private val
channelName declaration accordingly and then update any Flutter/Dart code that
references the old "voice_control_channel" string to use the new namespaced
value so both sides match (ensure MainActivity's channel registration and the
Dart MethodChannel/BasicMessageChannel use the identical new name).
In `@ios/Runner/AppDelegate.swift`:
- Around line 27-67: Make voiceChannel a class-level property (e.g., private var
voiceChannel: FlutterMethodChannel?) instead of a local let in
didFinishLaunchingWithOptions so its lifecycle is explicit; initialize it there
using voiceControlChannelName and controller.binaryMessenger, assign the handler
that calls applyVoiceProfile and setVoicePreviewEnabled as shown, and ensure you
clear/tear down voiceChannel (set to nil) when appropriate (e.g., on deinit or
when the Flutter engine is invalidated) to avoid relying on messenger retention
details.
In `@lib/routes/app_pages.dart`:
- Around line 153-156: The GetPage for AppRoutes.voiceProfileScreen is missing a
binding so the VoiceProfileController isn't created/disposed with navigation;
update the GetPage declaration for AppRoutes.voiceProfileScreen to include
binding: VoiceProfileBinding (so GetX will instantiate VoiceProfileController
per route) and ensure VoiceProfileBinding registers the controller (e.g., via
Get.put/Get.lazyPut inside VoiceProfileBinding), matching the controller
lifecycle expected by VoiceProfileScreen.
In `@lib/views/screens/voice_profile_screen.dart`:
- Around line 6-10: Replace the direct Get.put usage in the VoiceProfileScreen
field with lifecycle-managed binding: create a VoiceProfileBinding class that
implements Bindings and registers VoiceProfileController via Get.lazyPut (or
Get.put) in its dependencies(), register that Binding with the
VoiceProfileScreen route in your routing setup (app_pages.dart), and update
VoiceProfileScreen to retrieve the controller via
Get.find<VoiceProfileController>() (not Get.put) so GetX will track and call
onClose() when the route is popped.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements Issue #684.
Closes #684
Summary by CodeRabbit
New Features