Notify capture provider whenever language changed #1000
Conversation
|
WalkthroughThis update enhances the language setting functionality in the application. It ensures that changes in recording language are propagated to the Changes
Uplevel your code reviews with Entelligence.AI ProEntelligence.AI ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, Entelligence.AI Pro is free for open source projects. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (5)
- app/lib/pages/home/page.dart (1)
- app/lib/providers/capture_provider.dart (3)
- app/lib/providers/speech_profile_provider.dart (1)
- app/lib/services/sockets.dart (4)
- app/lib/services/sockets/transcription_connection.dart (2)
Review comments generated (12)
- Review: 10
- LGTM: 2
Tips
Chat with Entelligence.AI Bot (@Entelligence.AI)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@Entelligence.AIin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@Entelligence.AI: ignoreanywhere in the PR description to pause further reviews from the bot.
| setRecordingLanguage: (language) { | ||
| provider.setRecordingLanguage(language); | ||
|
|
||
| // Notify capture provider | ||
| if (context.mounted) { | ||
| context.read<CaptureProvider>().onRecordProfileSettingChanged(); | ||
| } |
There was a problem hiding this comment.
The changes look good, but it's important to note that the context.mounted check is not necessary here. The mounted property in Flutter is used to check if the BuildContext is still in the widget tree, which is useful before calling setState. However, in this case, you're using a provider and not directly setting state, so the check isn't needed.
- // Notify capture provider
- if (context.mounted) {
- context.read<CaptureProvider>().onRecordProfileSettingChanged();
- }
+ // Notify capture provider
+ context.read<CaptureProvider>().onRecordProfileSettingChanged();| String language = SharedPreferencesUtil().recordingsLanguage; | ||
| _socket = await ServiceManager.instance() | ||
| .socket | ||
| .memory(codec: codec, sampleRate: sampleRate, language: language, force: force); |
There was a problem hiding this comment.
The SharedPreferencesUtil().recordingsLanguage is directly accessed without any null check. If the shared preferences instance or the recordingsLanguage property is null, this will cause a null pointer exception. Consider adding a null check before using it.
- String language = SharedPreferencesUtil().recordingsLanguage;
+ String? language = SharedPreferencesUtil()?.recordingsLanguage ?? 'default_language';| @@ -184,7 +187,7 @@ class CaptureProvider extends ChangeNotifier | |||
| debugPrint('resetState'); | |||
| _cleanupCurrentState(); | |||
| await _recheckCodecChange(); | |||
| await _ensureSocketConnection(force: true); | |||
| await _ensureSocketConnection(); | |||
There was a problem hiding this comment.
The _ensureSocketConnection() method was previously called with a force: true parameter. This has been removed in the new code. If the force parameter is necessary for certain scenarios, its removal could lead to issues. Please ensure that this change is intentional and won't affect the functionality.
| Future<void> _ensureSocketConnection() async { | ||
| var codec = SharedPreferencesUtil().deviceCodec; | ||
| if (codec != _socket?.codec || _socket?.state != SocketServiceState.connected) { | ||
| await _initiateWebsocket(force: force); | ||
| var language = SharedPreferencesUtil().recordingsLanguage; | ||
| if (language != _socket?.language || codec != _socket?.codec || _socket?.state != SocketServiceState.connected) { | ||
| await _initiateWebsocket(audioCodec: codec, force: true); |
There was a problem hiding this comment.
Similar to the previous comment, the SharedPreferencesUtil().deviceCodec and SharedPreferencesUtil().recordingsLanguage are directly accessed without any null checks. If the shared preferences instance or the deviceCodec or recordingsLanguage properties are null, this will cause a null pointer exception. Consider adding null checks before using them.
- var codec = SharedPreferencesUtil().deviceCodec;
- var language = SharedPreferencesUtil().recordingsLanguage;
+ var codec = SharedPreferencesUtil()?.deviceCodec ?? 'default_codec';
+ var language = SharedPreferencesUtil()?.recordingsLanguage ?? 'default_language';| Future<TranscriptSegmentSocketService?> memory( | ||
| {required BleAudioCodec codec, required int sampleRate, bool force = false}); | ||
| {required BleAudioCodec codec, required int sampleRate, String language = defaultLanguage, bool force = false}); | ||
|
|
||
| Future<TranscriptSegmentSocketService?> speechProfile( | ||
| {required BleAudioCodec codec, required int sampleRate, bool force = false}); | ||
| {required BleAudioCodec codec, required int sampleRate, String language = defaultLanguage, bool force = false}); |
There was a problem hiding this comment.
The memory and speechProfile methods now have a new optional parameter language with a default value of defaultLanguage. This is a good change as it allows the language to be specified when creating socket services. However, please ensure that all calls to these methods are updated to pass the language parameter if needed.
| @@ -35,6 +37,7 @@ class SocketServicePool extends ISocketService { | |||
| Future<TranscriptSegmentSocketService?> socket({ | |||
| required BleAudioCodec codec, | |||
| required int sampleRate, | |||
| required String language, | |||
There was a problem hiding this comment.
The socket method now requires a language parameter. This is a good change as it ensures that the language is always specified when creating a socket service. However, please ensure that all calls to this method are updated to pass the language parameter.
| @@ -53,7 +56,7 @@ class SocketServicePool extends ISocketService { | |||
| // new socket | |||
| await _socket?.stop(); | |||
|
|
|||
| _socket = MemoryTranscriptSegmentSocketService.create(sampleRate, codec); | |||
| _socket = MemoryTranscriptSegmentSocketService.create(sampleRate, codec, language); | |||
There was a problem hiding this comment.
The MemoryTranscriptSegmentSocketService.create method now takes an additional language parameter. This is a good change as it allows the language to be specified when creating a MemoryTranscriptSegmentSocketService. However, please ensure that the create method in the MemoryTranscriptSegmentSocketService class has been updated to accept and handle this new parameter.
| {required BleAudioCodec codec, | ||
| required int sampleRate, | ||
| String language = defaultLanguage, | ||
| bool force = false}) async { | ||
| debugPrint("socket memory > $codec $sampleRate $force"); | ||
| return await socket(codec: codec, sampleRate: sampleRate, force: force); | ||
| return await socket(codec: codec, sampleRate: sampleRate, language: language, force: force); | ||
| } | ||
|
|
||
| @override | ||
| Future<TranscriptSegmentSocketService?> speechProfile( | ||
| {required BleAudioCodec codec, required int sampleRate, bool force = false}) async { | ||
| {required BleAudioCodec codec, | ||
| required int sampleRate, | ||
| String language = defaultLanguage, | ||
| bool force = false}) async { | ||
| debugPrint("socket speech profile > $codec $sampleRate $force"); | ||
| return await socket(codec: codec, sampleRate: sampleRate, force: force); | ||
| return await socket(codec: codec, sampleRate: sampleRate, language: language, force: force); |
There was a problem hiding this comment.
The memory and speechProfile methods now pass the language parameter to the socket method. This is a good change as it ensures that the language is always specified when creating a socket service. However, please ensure that all calls to these methods are updated to pass the language parameter if needed.
| SpeechProfileTranscriptSegmentSocketService.create(super.sampleRate, super.codec, super.language) | ||
| : super.create(includeSpeechProfile: false); | ||
| } | ||
|
|
||
| class MemoryTranscriptSegmentSocketService extends TranscriptSegmentSocketService { | ||
| MemoryTranscriptSegmentSocketService.create(super.sampleRate, super.codec) : super.create(includeSpeechProfile: true); | ||
| MemoryTranscriptSegmentSocketService.create(super.sampleRate, super.codec, super.language) | ||
| : super.create(includeSpeechProfile: true); |
There was a problem hiding this comment.
The constructors for SpeechProfileTranscriptSegmentSocketService and MemoryTranscriptSegmentSocketService are using super.sampleRate, super.codec, and super.language. However, these properties are not defined in the superclass constructor. Instead, they should be passed as parameters to the constructor of these classes.
- SpeechProfileTranscriptSegmentSocketService.create(super.sampleRate, super.codec, super.language)
+ SpeechProfileTranscriptSegmentSocketService.create(int sampleRate, BleAudioCodec codec, String language)
- MemoryTranscriptSegmentSocketService.create(super.sampleRate, super.codec, super.language)
+ MemoryTranscriptSegmentSocketService.create(int sampleRate, BleAudioCodec codec, String language)| String language; | ||
| bool includeSpeechProfile; | ||
|
|
||
| TranscriptSegmentSocketService.create( | ||
| this.sampleRate, | ||
| this.codec, { | ||
| this.codec, | ||
| this.language, { | ||
| this.includeSpeechProfile = false, | ||
| }) { | ||
| final recordingsLanguage = SharedPreferencesUtil().recordingsLanguage; | ||
| var params = '?language=$recordingsLanguage&sample_rate=$sampleRate&codec=$codec&uid=${SharedPreferencesUtil().uid}' | ||
| var params = '?language=$language&sample_rate=$sampleRate&codec=$codec&uid=${SharedPreferencesUtil().uid}' |
There was a problem hiding this comment.
The SharedPreferencesUtil().uid and SharedPreferencesUtil().transcriptionModel are directly used while constructing the URL. This could lead to potential null pointer exceptions if these shared preferences are not set. It would be better to check for null values before using them.
- var params = '?language=$language&sample_rate=$sampleRate&codec=$codec&uid=${SharedPreferencesUtil().uid}'
- '&include_speech_profile=$includeSpeechProfile&stt_service=${SharedPreferencesUtil().transcriptionModel}';
+ var uid = SharedPreferencesUtil().uid ?? '';
+ var transcriptionModel = SharedPreferencesUtil().transcriptionModel ?? '';
+ var params = '?language=$language&sample_rate=$sampleRate&codec=$codec&uid=$uid'
+ '&include_speech_profile=$includeSpeechProfile&stt_service=$transcriptionModel';
Summary by Entelligence.AI
SocketServicePoolclass to include alanguageparameter, allowing for more precise control over socket service creation.speech_profile_provider.dart, improving code cleanliness and maintainability.