Skip to content

Comments

Bus adapter changes#2

Merged
craig8 merged 10 commits intoeclipse-volttron:developfrom
davidraker:bus_adapter_changes
Sep 29, 2025
Merged

Bus adapter changes#2
craig8 merged 10 commits intoeclipse-volttron:developfrom
davidraker:bus_adapter_changes

Conversation

@davidraker
Copy link
Contributor

This pull request introduces several improvements and refactorings across the protocol proxy codebase, focusing on better subprocess logging, more robust type usage, and code simplification. The most significant changes are the introduction of unified logging for subprocess output in both asyncio and gevent managers, adjustments to class inheritance and typing for clarity and correctness, and removal of unused or redundant code related to environment setup.

Subprocess Logging Improvements

  • Added unified methods (log_subprocess_output and log_subprocess_output_line) to both asyncio and gevent managers for capturing and logging output from proxy subprocesses, improving error visibility and debugging. These methods decode log lines, attempt JSON parsing for structured logs, and handle errors gracefully. ([[1]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-7cc95dd544d0240549121729661d1b6426981ec7e0c69b56fbb681eaeb22f298L44-R63), [[2]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-2e6cfb07631679361fbd1b2a7683bea829e7c63941ad6e87f1b1b98416af7af7R136-R156), [[3]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-f3f5fdb921c083c2a7637eb19da8ddfb7e91e629dee75b66392a233b8dca8852L46-R50), [[4]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-f3f5fdb921c083c2a7637eb19da8ddfb7e91e629dee75b66392a233b8dca8852R65-R69))
  • Updated proxy process creation to capture and log stdout and stderr streams in both asyncio and gevent implementations, replacing previous placeholder or commented-out code. ([[1]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-7cc95dd544d0240549121729661d1b6426981ec7e0c69b56fbb681eaeb22f298L44-R63), [[2]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-f3f5fdb921c083c2a7637eb19da8ddfb7e91e629dee75b66392a233b8dca8852L46-R50))

Class and Type Refactoring

  • Switched several classes (IPCConnector, ProtocolHeaders, ProtocolProxy) to use ABCMeta for proper abstract base class semantics, and replaced abc.abstractmethod with direct imports where needed. ([[1]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-48848710903864c51c9baf618d21fa5d744d45f6d493dc1b7d158fb3fbea8d2bL3-R3), [[2]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-48848710903864c51c9baf618d21fa5d744d45f6d493dc1b7d158fb3fbea8d2bL53-R53), [[3]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-5499c571be0e7952b96e4f48f4059032399a671a61a59a3eb8701aa8900d2d8dL1-R16), [[4]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-5499c571be0e7952b96e4f48f4059032399a671a61a59a3eb8701aa8900d2d8dL31-R44), [[5]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-cb234840c21bd35447934a39a12a73e46e32d9f0b172e56f008e558449cdaafbL13-R14))
  • Updated type hints to use Any instead of any in decorators, and added explicit cast usage for peer objects to ensure correct typing in proxy registration logic. ([[1]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-88f3a5c7266a8db92630b70c66cc3bafb9da6b1ce9210283f74ff9bb5da65502R3-R20), [[2]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-99fbed7b4cbb6da3388477fe7398856815aade43958605b0a8390243743f5745R5), [[3]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-99fbed7b4cbb6da3388477fe7398856815aade43958605b0a8390243743f5745L46-R47), [[4]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-ed340393b11be1e7b9da559c373a92d396377ded0894345787d776a09a10e9acR6), [[5]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-ed340393b11be1e7b9da559c373a92d396377ded0894345787d776a09a10e9acL21-R26))

Code Simplification and Cleanup

  • Removed unnecessary code related to environment variable setup for proxy subprocesses, simplifying process command construction and eliminating unused variables. ([src/protocol_proxy/manager/base.pyL48-R50](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-2e6cfb07631679361fbd1b2a7683bea829e7c63941ad6e87f1b1b98416af7af7L48-R50))
  • Fixed typos and improved debug logging messages for clarity, such as correcting "peername" to "peer_name" and "VERISON" to "VERSION". ([[1]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-7a0ff7402f41977000969720ca23cac62ce0a21b756190a1d61328dd101a09d8L111-R111), [[2]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-7a0ff7402f41977000969720ca23cac62ce0a21b756190a1d61328dd101a09d8L142-R142), [[3]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-5499c571be0e7952b96e4f48f4059032399a671a61a59a3eb8701aa8900d2d8dL31-R44))
  • Adjusted logic for proxy process cleanup and error handling, including improved exception messages and correct usage of process objects. ([[1]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-7cc95dd544d0240549121729661d1b6426981ec7e0c69b56fbb681eaeb22f298L71-R88), [[2]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-f3f5fdb921c083c2a7637eb19da8ddfb7e91e629dee75b66392a233b8dca8852L82-R88))

Minor Improvements

  • Updated import statements for consistency, removed unused imports, and made minor adjustments to default values and method signatures for clarity. ([[1]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-7cc95dd544d0240549121729661d1b6426981ec7e0c69b56fbb681eaeb22f298L4-R7), [[2]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-7e96d363ef2d96b3c8b721798a827290f735d8befb8ca4e72fd52886ed8ae670L170-R170), [[3]](https://github.com/eclipse-volttron/lib-protocol-proxy/pull/2/files#diff-99fbed7b4cbb6da3388477fe7398856815aade43958605b0a8390243743f5745L17-R18))

These changes collectively improve maintainability, error handling, and the overall robustness of the protocol proxy framework.

@davidraker davidraker requested a review from Copilot September 27, 2025 21:29
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request introduces comprehensive improvements to the protocol proxy codebase focused on subprocess logging, type safety, and code cleanup. The changes implement unified logging for proxy subprocess output, improve type usage with proper abstract base class semantics, and simplify the codebase by removing unused functionality.

  • Unified subprocess logging implementation across asyncio and gevent managers with JSON log parsing
  • Enhanced type safety through proper ABCMeta usage and explicit type casting
  • Code cleanup including typo fixes, unused code removal, and improved error handling

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/protocol_proxy/proxy/launch.py Adds structured JSON logging configuration and fixes UUID token parsing
src/protocol_proxy/proxy/gevent.py Removes manager_token parameter and adds type casting for peer objects
src/protocol_proxy/proxy/base.py Updates class inheritance to use ABCMeta and removes manager_token parameter
src/protocol_proxy/proxy/asyncio.py Adds type casting and removes manager_token parameter
src/protocol_proxy/manager/gevent.py Implements subprocess output logging with stdout/stderr capture
src/protocol_proxy/manager/base.py Adds abstract logging methods and JSON log parsing functionality
src/protocol_proxy/manager/asyncio.py Implements async subprocess logging and improves process handling
src/protocol_proxy/ipc/headers.py Converts to ABCMeta and fixes typo in repr method
src/protocol_proxy/ipc/gevent.py Updates default value for io_wait_time
src/protocol_proxy/ipc/decorator.py Fixes type annotation from 'any' to 'Any' and adds return statements
src/protocol_proxy/ipc/base.py Updates IPCConnector to use ABCMeta
src/protocol_proxy/ipc/asyncio.py Fixes typos in debug comments

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

davidraker and others added 2 commits September 27, 2025 14:54
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@davidraker davidraker requested a review from craig8 September 27, 2025 22:02
@craig8 craig8 merged commit 95cdff7 into eclipse-volttron:develop Sep 29, 2025
3 checks passed
schandrika pushed a commit that referenced this pull request Feb 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants