-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds support for SpeechStarted #249
Conversation
WalkthroughThe update introduces support for the Changes
Related issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (6)
- Deepgram/Clients/LiveTranscriptionClient.cs (3 hunks)
- Deepgram/Interfaces/ILiveTranscriptionClient.cs (1 hunks)
- Deepgram/Models/LiveTranscriptionOptions.cs (1 hunks)
- Deepgram/Models/LiveTranscriptionResult.cs (1 hunks)
- Deepgram/Models/LiveTranscriptionSpeechStarted.cs (1 hunks)
- Deepgram/Models/SpeechStartedEventArgs.cs (1 hunks)
Files skipped from review due to trivial changes (1)
- Deepgram/Models/LiveTranscriptionSpeechStarted.cs
Additional comments: 7
Deepgram/Models/SpeechStartedEventArgs.cs (1)
- 16-16: Consider making the
SpeechStarted
property read-only to ensure the immutability of the event args. This can be achieved by removing theset;
accessor if the property is only intended to be set during object construction.Deepgram/Models/LiveTranscriptionResult.cs (1)
- 54-55: The addition of the
Type
property is consistent with the context of the class and follows best practices for JSON property annotations.Deepgram/Interfaces/ILiveTranscriptionClient.cs (1)
- 31-34: The addition of the
SpeechStarted
event is well-documented and follows the naming conventions of existing events in the interface.Deepgram/Models/LiveTranscriptionOptions.cs (1)
- 251-252: The addition of the
VadEvents
property is consistent with the context of the class and follows best practices for JSON property annotations.Deepgram/Clients/LiveTranscriptionClient.cs (3)
- 14-14: The addition of the
using Newtonsoft.Json.Linq;
directive is necessary for the use ofJObject
in the message processing logic.- 59-62: The declaration of the
SpeechStarted
event is consistent with other events in the class, and the documentation clearly explains its purpose.- 273-298: The message processing logic for handling "SpeechStarted" type messages is implemented correctly. It uses
JObject
to parse the WebSocket message, checks for a "SpeechStarted" type, deserializes the message, and raises theSpeechStarted
event appropriately.
This looks good to me, but will need to take a look at this. Currently working on v4 stuff and we dont have an example that I can really quickly run this against or have a unit test for a quick check. |
I needed to use speech started event so I made an update with the event added
Summary by CodeRabbit
SpeechStarted
event will be triggered if VAD events are enabled.VadEvents
to enable or disable VAD events during live transcription.Type
property, improving result interpretation.SpeechStarted
event andVadEvents
option.