fixed connection issues I found #174
Conversation
…and improve logging
…ng; refactor CallbackRouter for wildcard topic support
There was a problem hiding this comment.
Pull Request Overview
This PR addresses connection issues across multiple modules, improving address handling, connection validation, and callback routing.
- Update GridAPPSD initialization to support a tuple/list address format
- Enhance connection checks and error handling in the GOSS layer and field bus interface
- Improve callback routing with wildcard support and logging
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| gridappsd-python-lib/gridappsd/gridappsd.py | Refactors init to handle a None address or a (hostname, port) tuple/list |
| gridappsd-python-lib/gridappsd/goss.py | Adds validation for stomp address/port, uses a connection tuple, and revises CallbackRouter methods |
| gridappsd-field-bus-lib/gridappsd_field_bus/field_interface/gridappsd_field_bus.py | Improves logging and connection status checks in the bus connection logic |
| gridappsd-field-bus-lib/gridappsd_field_bus/field_interface/agents/agents.py | Updates connection logic and logging for upstream/downstream message buses |
| super(GridAPPSD, self).__init__(stomp_address=address[0], stomp_port=address[1], **kwargs) | ||
| if address is None: | ||
| super().__init__(**kwargs) | ||
| elif isinstance(address, tuple) or isinstance(address, list) and len(address) == 2: |
There was a problem hiding this comment.
The condition combining isinstance checks may be ambiguous due to operator precedence. Consider adding explicit parentheses to clarify the intended logic, e.g., 'elif isinstance(address, tuple) or (isinstance(address, list) and len(address) == 2):'.
| elif isinstance(address, tuple) or isinstance(address, list) and len(address) == 2: | |
| elif isinstance(address, tuple) or (isinstance(address, list) and len(address) == 2): |
| :param topic_pattern: Topic string, allowing `*` or `**` as wildcards. | ||
| :param callback: Function to call when a matching message is received. | ||
| """ | ||
| regex_pattern = re.escape(topic_pattern).replace(r'\*', '[^/]+').replace(r'\*\*', '.*') |
There was a problem hiding this comment.
Replacing the single wildcard before the double wildcard may prevent the '**' conversion from ever being applied. It is recommended to replace the double wildcard pattern first.
| regex_pattern = re.escape(topic_pattern).replace(r'\*', '[^/]+').replace(r'\*\*', '.*') | |
| regex_pattern = re.escape(topic_pattern).replace(r'\*\*', '.*').replace(r'\*', '[^/]+') |
| else: | ||
| _log.error("GridAPPSD object is not initialized. Cannot connect to message bus.") | ||
|
|
There was a problem hiding this comment.
The logic in the connect method is confusing: the else branch logs an error when gridappsd_obj is already set. Consider revising the condition to ensure that an existing, valid GridAPPSD object is used instead of logging an error.
| else: | |
| _log.error("GridAPPSD object is not initialized. Cannot connect to message bus.") | |
| elif not self.gridappsd_obj.connected: | |
| _log.debug("GridAPPSD object is not connected. Reconnecting to message bus.") | |
| self.gridappsd_obj.connect() | |
| else: | |
| _log.debug("GridAPPSD object is already connected to the message bus.") |
1 participant
No description provided.