-
-
Notifications
You must be signed in to change notification settings - Fork 4
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
bring http client closes to websocket client #64
Conversation
bring the http client api closes to the websocket one
WalkthroughThis pull request introduces significant changes to the Hivemind Bus Client library, focusing on rebranding, documentation updates, and enhanced client functionality. The modifications include renaming the package from "Hivemind Websocket Client" to "Hivemind Bus Client", updating README documentation, and improving the HTTP client with new Mycroft message handling capabilities. The changes aim to provide clearer usage instructions and more flexible message handling for developers. Changes
Sequence DiagramsequenceDiagram
participant Client as HiveMindHTTPClient
participant Server as HiveMind Server
Client->>Server: Initiate Connection
Client->>Server: Wait for Handshake
Server-->>Client: Handshake Complete
Client->>Server: Register Mycroft Handlers
Client->>Server: Emit Mycroft Message
Server-->>Client: Process Message
Possibly related issues
Possibly related PRs
Suggested labels
Poem
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
🧹 Nitpick comments (7)
hivemind_bus_client/http_client.py (5)
81-87
: Consider improving handshake timeout handling.The recursive
wait_for_handshake
implementation could potentially lead to infinite recursion if the handshake consistently fails. Consider adding a maximum retry count or implementing an exponential backoff strategy.def wait_for_handshake(self, timeout=5): self.connected.wait(timeout=timeout) self.handshake_event.wait(timeout=timeout) if not self.handshake_event.is_set(): + max_retries = 3 # Add max retries + if not hasattr(self, '_handshake_attempts'): + self._handshake_attempts = 0 + self._handshake_attempts += 1 + if self._handshake_attempts >= max_retries: + raise ConnectionError("Failed to establish handshake after maximum retries") self.protocol.start_handshake() self.wait_for_handshake() + self._handshake_attempts = 0 # Reset counter on successful handshake time.sleep(1) # let server process our "hello" response
265-268
: Add type hints for better code maintainability.Consider adding type hints to improve code maintainability and IDE support.
- def on_mycroft(self, event_name: str, func: Callable): + def on_mycroft(self, event_name: str, func: Callable[[MycroftMessage], None]) -> None: if event_name not in self._agent_handlers: self._agent_handlers[event_name] = [] self._agent_handlers[event_name].append(func)
225-230
: Enhance error handling with more context.The error handling could be more informative by including the event name and message details in the error log.
if message.msg_type == HiveMessageType.BUS and message.payload.msg_type in self._agent_handlers: for handler in self._agent_handlers[message.payload.msg_type]: try: handler(message.payload) except Exception as e: - LOG.error(f"Error in agent message handler: {handler} - {e}") + LOG.error(f"Error in agent message handler for event '{message.payload.msg_type}': {handler.__name__} - {e}") + LOG.debug(f"Message payload: {message.payload}")
434-437
: Avoid using global variables in example code.The example code uses global variables which is not a recommended practice. Consider using a class to manage state.
+class ChatHandler: + def __init__(self): + self.answer = None + self.answered = threading.Event() + + def handle_speak(self, message: MycroftMessage): + self.answer = message.data['utterance'] + + def utt_handled(self, message: MycroftMessage): + self.answered.set()
398-399
: Consider using a more verbose log level for development.Setting the log level to "ERROR" might hide important debugging information. Consider using "INFO" or "DEBUG" during development.
init_service_logger("HiveMindHTTP") -LOG.set_level("ERROR") +LOG.set_level("INFO") # Use more verbose logging during developmentREADME.md (2)
13-14
: Add links to protocol documentation.Consider adding links to protocol documentation or specifications to help users understand the differences between HTTP and WebSocket protocols.
54-60
: Add error handling examples in documentation.The chat example could benefit from showing how to handle connection errors and timeouts.
try: client.emit(HiveMessage(HiveMessageType.BUS, Message("recognizer_loop:utterance", {"utterances": [utt]}))) if not answered.wait(timeout=30): # Add timeout print("Timeout waiting for response") except ConnectionError as e: print(f"Connection error: {e}")
add
on_mycroft
Summary by CodeRabbit
Documentation
New Features
Bug Fixes