-
-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
Add timeout to Google Cloud configuration #37122
base: next
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for home-assistant-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
📝 WalkthroughWalkthroughThe pull request introduces a new configuration option Changes
Sequence DiagramsequenceDiagram
participant User
participant HomeAssistant
participant GoogleCloudTTS
User->>HomeAssistant: Configure timeout
HomeAssistant->>GoogleCloudTTS: Request TTS with timeout
GoogleCloudTTS-->>HomeAssistant: Generate audio response
The sequence diagram illustrates how the new 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
source/_integrations/google_cloud.markdown
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
source/_integrations/google_cloud.markdown
[grammar] ~112-~112: Did you mean “generating”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ...ation. Increasing this value will allow to generate longer audio files from Google Cloud TT...
(ALLOW_TO)
tts_timeout: | ||
description: "Time in seconds to wait for the TTS response generation. Increasing this value will allow to generate longer audio files from Google Cloud TTS" | ||
required: false | ||
type: int | ||
default: 10 |
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.
🛠️ Refactor suggestion
Enhance the configuration documentation for better clarity and completeness.
The documentation for the new tts_timeout
option needs improvements in several areas:
- Grammar and clarity
- Missing important context
- Configuration details
Apply this diff to improve the documentation:
tts_timeout:
- description: "Time in seconds to wait for the TTS response generation. Increasing this value will allow to generate longer audio files from Google Cloud TTS"
+ description: "Time in seconds to wait for the TTS response generation. The default timeout of 10 seconds is sufficient for most text (up to 600-800 characters). For longer text, increase this value to allow more time for audio file generation. Configurable through the UI."
required: false
type: int
default: 10
+ selector:
+ number:
+ min: 1
+ max: 120
+ unit_of_measurement: seconds
The changes:
- Fix grammar in description
- Add context about character limits
- Add UI configuration mention
- Add selector with reasonable min/max values
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
tts_timeout: | |
description: "Time in seconds to wait for the TTS response generation. Increasing this value will allow to generate longer audio files from Google Cloud TTS" | |
required: false | |
type: int | |
default: 10 | |
tts_timeout: | |
description: "Time in seconds to wait for the TTS response generation. The default timeout of 10 seconds is sufficient for most text (up to 600-800 characters). For longer text, increase this value to allow more time for audio file generation. Configurable through the UI." | |
required: false | |
type: int | |
default: 10 | |
selector: | |
number: | |
min: 1 | |
max: 120 | |
unit_of_measurement: seconds |
🧰 Tools
🪛 LanguageTool
[grammar] ~112-~112: Did you mean “generating”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ...ation. Increasing this value will allow to generate longer audio files from Google Cloud TT...
(ALLOW_TO)
@@ -108,6 +108,16 @@ text_type: | |||
required: false | |||
type: string | |||
default: "text" | |||
tts_timeout: |
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.
Remove the tts_ prefix and update PR title
I'm drafting this PR, as changes are needed. Please undraft it once the changes have been implemented and the PR is ready for review again. |
Proposed change
This is updated documentation for the change proposed here.
The existing implementation has a hard-coded 10s timeout when waiting for the generated speech from Google Cloud TTS.
Because of that, the integration fails (error "400 Request contains an invalid argument") when you try to generate outputs larger than roughly 600-800 characters, since it would take longer than that for Google Cloud TTS to return a response. I've been hitting this issue constantly when trying to announce long responses coming from my LLM, for example.
The proposed change makes both STT and TTS Timeout configurable in the UI along with the existing model options. The default value is set at 10 seconds (same as the existing implementation), but now users can increase it if needed to generate longer output. The tests have been updated, too.
This updates the documentation to add the
timeout
option (used by both STT and TTS).Type of change
current
branch).current
branch).next
branch).next
branch).Additional information
Checklist
current
branch.next
branch.Summary by CodeRabbit