Skip to content

Comments

Merge of rileys code and latest changes#6

Open
riley206-pnnl wants to merge 17 commits intoeclipse-volttron:developfrom
riley206-pnnl:merge_of_rileys_code_and_latest_changes
Open

Merge of rileys code and latest changes#6
riley206-pnnl wants to merge 17 commits intoeclipse-volttron:developfrom
riley206-pnnl:merge_of_rileys_code_and_latest_changes

Conversation

@riley206-pnnl
Copy link
Contributor

This pull request introduces several enhancements and new features to the BACnet protocol proxy library, focusing on developer experience, backward compatibility, and improved tooling for BACnet device discovery and configuration. The most significant changes include the addition of new utility scripts for device scanning and batch configuration, improvements to documentation and packaging, and minor bug fixes.

New BACnet utility scripts:

  • Added scripts/bacnet_scan.py, a modernized BACnet network scanner using BACpypes3 and asyncio, supporting both legacy and new argument styles, INI file compatibility, device discovery, and CSV export.
  • Added scripts/grab_multiple_configs.py, a batch utility for scraping configuration from multiple BACnet devices using a CSV input, compatible with both legacy and current workflows.
  • Added a sample BACpypes.ini configuration file for easier setup and compatibility with the new scripts.

Documentation and packaging improvements:

  • Significantly expanded and updated README.md with installation instructions, dependency listing, usage notes, and project badges for Python versions and CI status.
  • Updated pyproject.toml to clarify Python version specification for mypy, add a placeholder for development dependencies, and ensure accurate dependency management. [1] [2]

Bug fixes and code quality:

  • Fixed a bug in bacnet.py where errors during property writes now return the error object instead of just logging it, improving error handling for clients.

Copy link

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 modernizes the BACnet protocol proxy library by migrating from BACpypes v2 to BACpypes3, introducing new utility scripts for device discovery and configuration management, and improving documentation and project configuration.

Key Changes:

  • Migrated three utility scripts (bacnet_scan.py, grab_bacnet_config.py, grab_multiple_configs.py) to use BACpypes3 with asyncio while maintaining backward compatibility with legacy BACpypes.ini configuration files
  • Fixed error handling in bacnet.py to return error objects instead of only logging them
  • Expanded README with installation instructions, dependency information, and project badges

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 25 comments.

Show a summary per file
File Description
src/protocol_proxy/protocol/bacnet/bacnet_proxy.py Removed unused apdu_timeout parameter from who_is_endpoint method
src/protocol_proxy/protocol/bacnet/bacnet.py Modified write_property to return error object on exception instead of just logging
scripts/grab_multiple_configs.py New batch configuration scraper supporting CSV input for multiple BACnet devices
scripts/grab_bacnet_config.py New device configuration scraper using BACpypes3 with INI file backward compatibility
scripts/bacnet_scan.py New network scanner using BACpypes3 with device discovery and CSV export capabilities
scripts/BACpypes.ini Sample configuration file for backward compatibility with legacy workflows
pyproject.toml Fixed mypy python_version to use string format and added dev dependencies section
README.md Expanded with installation instructions, dependencies, badges, and project information

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@riley206-pnnl
Copy link
Contributor Author

Most of those are gone now since I removed the scripts.

@riley206-pnnl
Copy link
Contributor Author

this should be ready to merge!

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