-
Notifications
You must be signed in to change notification settings - Fork 2
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: forward some events to GUI clients #62
Conversation
allow GUIs to react to wakeword etc without needing a connection to the main bus GUI clients are not supposed to connect to the main bus, only to the GUI bus, but in practice the QT gui has been connecting to both due to missing events
WalkthroughThe changes in the Changes
Poem
Warning Rate limit exceeded@JarbasAl has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 36 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (3)ovos_gui/namespace.py (3)
The added imports support the file system operations and page management functionality. Also applies to: 47-47, 58-59
The implementation effectively forwards essential events to GUI clients, properly categorized into:
This aligns with the PR objective of enabling GUI clients to respond to events without requiring a direct connection to the main bus.
Fix the The Apply this fix: - @staticmethod
- def forward_to_gui(self, message: Message):
+ def forward_to_gui(self, message: Message):
"""
Forward a core Message to the GUI
@param message: Core message to forward
"""
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
ovos_gui/namespace.py
(2 hunks)
🔇 Additional comments (1)
ovos_gui/namespace.py (1)
43-44
: Import statements are appropriate and necessary
The added import statements for join
, dirname
, exists
, Message
, MessageBusClient
, and GuiPage
are correct and essential for the new functionality introduced.
Also applies to: 47-47, 58-59
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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 (1)
ovos_gui/namespace.py (1)
461-505
: LGTM: Well-structured message forwarding implementation.The implementation effectively addresses the PR's objective of enabling GUI clients to respond to events without requiring direct connection to the main bus. The code:
- Clearly categorizes messages by service type
- Uses a clean loop pattern to register handlers
- Includes all necessary events for GUI responsiveness
Consider adding a configuration option to selectively enable/disable forwarding of specific event categories, which could help optimize performance by reducing unnecessary message traffic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
ovos_gui/namespace.py
(2 hunks)
🔇 Additional comments (2)
ovos_gui/namespace.py (2)
43-44
: LGTM: Import changes are well-organized and appropriate.
The changes to the imports are clean and follow best practices:
- Using
exists
instead ofisfile
is more semantically correct for path checking - Imports are properly organized following PEP 8 guidelines
Also applies to: 47-59
506-519
: Fix incorrect @staticmethod decorator usage.
The forward_to_gui
method is marked as @staticmethod
but is called as an instance method in _define_messages_to_forward
. This will cause runtime errors.
allow GUIs to react to wakeword etc without needing a connection to the main bus
GUI clients are not supposed to connect to the main bus, only to the GUI bus, but in practice the QT gui has been connecting to both due to missing events
Summary by CodeRabbit
New Features
Bug Fixes
Chores
FakeBus
across multiple test files to reflect module reorganization.