-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat:fallback_plugins #263
Conversation
similar to WakeWords, allows defining a plugin to load if the primary fails this is DIFFERENT from fallback TTS/STT, it isnt loaded at same time but instead when main plugin fails to load for any reason
WalkthroughThe pull request introduces significant changes across multiple files in the Changes
Possibly related PRs
Suggested labels
Poem
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional comments not posted (2)
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: 0
Outside diff range and nitpick comments (2)
ovos_plugin_manager/vad.py (1)
Line range hint
85-111
: Fallback mechanism looks good, consider a minor refactoring.The changes to the
create
method are approved:
- The change from static method to class method allows the method to call itself recursively to handle the fallback case, which is a good use of the class method.
- The fallback mechanism enhances the robustness of the plugin loading process by providing an alternative in case the primary plugin fails to load.
- The check to ensure the fallback plugin is different from the primary plugin avoids unnecessary attempts to reload the same module.
Consider this minor refactoring for consistency:
-clazz = OVOSVADFactory.get_class(vad_config) +clazz = cls.get_class(vad_config)ovos_plugin_manager/wakewords.py (1)
Line range hint
114-125
: Ensure robust handling of module names after removingMAPPINGS
.The removal of the
MAPPINGS
dictionary simplifies the code but shifts the responsibility of ensuring valid module names to thehotword_config
structure. This change may impact the robustness of the hotword loading process if invalid module names are provided inhotword_config
.Consider adding validation for module names in
hotword_config
to maintain the reliability of the hotword loading process. For example:def validate_module_name(module_name): # Add validation logic here # Raise an exception or log an error if the module name is invalid pass # In get_class method ww_module = hotword_config[hotword]["module"] validate_module_name(ww_module)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- ovos_plugin_manager/g2p.py (3 hunks)
- ovos_plugin_manager/language.py (5 hunks)
- ovos_plugin_manager/microphone.py (2 hunks)
- ovos_plugin_manager/vad.py (4 hunks)
- ovos_plugin_manager/wakewords.py (2 hunks)
Additional comments not posted (7)
ovos_plugin_manager/microphone.py (1)
52-53
: LGTM! The changes enhance flexibility and robustness.The changes made to the
create
method are approved for the following reasons:
Changing the method from static to class method:
- Allows the method to access class-level properties and methods, enhancing its flexibility.
- Enables the method to utilize class-specific behavior.
Implementing the fallback mechanism:
- Improves the resilience of the microphone loading process by providing a fallback option.
- Enhances the overall functionality of the microphone management system by attempting to load an alternative microphone plugin if the primary one fails.
The fallback mechanism is a valuable addition that helps maintain functionality even in the event of a failure with the primary plugin.
Also applies to: 65-65, 73-73, 78-81
ovos_plugin_manager/vad.py (1)
84-84
: LGTM!The change from static method to class method is approved. This allows the method to access class-level properties if needed in the future, providing more flexibility.
ovos_plugin_manager/g2p.py (2)
94-94
: LGTM!The removal of the
MAPPINGS
dictionary simplifies the logic for determining the G2P module to load. The code now directly checks if theg2p_module
is equal to 'dummy', which is a more straightforward approach.
Line range hint
99-124
: LGTM!The changes to the
create
method enhance its flexibility and improve the robustness of the plugin loading process:
- Changing the method from a static method to a class method allows it to access class-level properties and methods more effectively.
- The introduction of a fallback mechanism provides an alternative option if the primary plugin fails to load, improving the overall reliability of the system.
The code changes are well-structured and follow a clear logic flow.
ovos_plugin_manager/wakewords.py (1)
Line range hint
144-155
: LGTM!The changes to the
create_hotword
method look good. The fallback mechanism enhances the robustness of the plugin loading process by providing an alternative when the primary plugin fails. The code correctly checks if the fallback plugin exists inww_configs
and is different from the primary plugin before attempting to load it.ovos_plugin_manager/language.py (2)
Line range hint
108-138
: LGTM!The changes in the
create
method ofOVOSLangDetectionFactory
are well-structured and improve the handling of fallback plugins. The key improvements include:
- Changing the method from static to a class method, allowing for more flexible access to class-level properties and methods.
- Implementing a fallback mechanism to load an alternative plugin if the primary one fails, enhancing the robustness of the plugin loading process.
- Simplifying the logic by removing the checks for mappings and directly retrieving the fallback module from the configuration.
The code changes are approved.
Line range hint
164-194
: LGTM!The changes in the
create
method ofOVOSLangTranslationFactory
are well-structured and improve the handling of fallback plugins, similar to the changes inOVOSLangDetectionFactory
. The key improvements include:
- Changing the method from static to a class method, allowing for more flexible access to class-level properties and methods.
- Implementing a fallback mechanism to load an alternative plugin if the primary one fails, enhancing the robustness of the plugin loading process.
- Simplifying the logic by removing the checks for mappings and directly retrieving the fallback module from the configuration.
The code changes are approved.
similar to WakeWords, allows defining a plugin to load if the primary fails this is DIFFERENT from fallback TTS/STT, it isnt loaded at same time but instead when main plugin fails to load for any reason companion to OpenVoiceOS/ovos-plugin-manager#263
similar to WakeWords, allows defining a plugin to load if the primary fails this is DIFFERENT from fallback TTS/STT, it isnt loaded at same time but instead when main plugin fails to load for any reason companion to OpenVoiceOS/ovos-plugin-manager#263
similar to WakeWords, allows defining a plugin to load if the primary fails this is DIFFERENT from fallback TTS/STT, it isnt loaded at same time but instead when main plugin fails to load for any reason companion to OpenVoiceOS/ovos-plugin-manager#263
* plugins * debug * feat:fallback_plugins OpenVoiceOS/ovos-config#153 OpenVoiceOS/ovos-plugin-manager#263 * Update requirements.txt
similar to WakeWords, allows defining a plugin to load if the primary fails
this is DIFFERENT from fallback TTS/STT, it isnt loaded at same time but instead when main plugin fails to load for any reason
companion to OpenVoiceOS/ovos-config#153
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Refactor