Skip to content

Conversation

@sindrehan
Copy link
Member

  • Separate the legacy protocol out into it's own package (blueye.legacyprotocol). This is a breaking change, hence the version bump to v3.
  • Add a feature flag in the blueye.protocol package to install the legacy protocol, fixes Put numpy dependency behind feature flag #53
  • Use uv instead of poetry for project management
  • Use ruff instead of flake8 for linting
  • Remove setup.py (and the functionality to generate it). Fixes Remove setup.py generator #48
  • Use implicit namespacing (the pkg_resources method has been deprecated for some time)

@sindrehan sindrehan force-pushed the rework-project-structure branch from 1f59dc3 to 31e5e2a Compare June 6, 2025 14:57
@sindrehan sindrehan requested a review from Copilot June 6, 2025 16:07
@sindrehan sindrehan marked this pull request as ready for review June 6, 2025 16:08
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

Reorganize the project by extracting legacy UDP/TCP protocol code into its own package, replacing Poetry/Flake8 with uv/Ruff, and removing setup.py in favor of PEP 621 metadata.

  • Extract legacy protocol modules into blueye.legacyprotocol with its own pyproject.toml, README, and LICENSE
  • Update generators and imports to point to the new legacyprotocol path
  • Migrate CI workflows from Poetry/flake8 to uv/Ruff and remove setup.py

Reviewed Changes

Copilot reviewed 45 out of 45 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
legacyprotocol/pyproject.toml Added project metadata and dependencies for new package
legacyprotocol/blueye/legacyprotocol/udp_protocol_parser.py Refactored parser, updated imports, and formatting
legacyprotocol/blueye/legacyprotocol/udp_protocol_dict.py Bumped _generator_hash in autogenerated protocol dictionary
legacyprotocol/blueye/legacyprotocol/udp_client.py Switched to local exception import and streamlined socket init
legacyprotocol/blueye/legacyprotocol/tcp_client.py Updated exception imports to use local package
legacyprotocol/blueye/legacyprotocol/init.py Defined public API exports
legacyprotocol/README.md Added package overview and deprecation notice
legacyprotocol/LICENSE Included LGPLv3 license text
generators/generate_udp_protocol.py Changed hard-coded paths to script-relative paths
generators/generate_tcp_protocol.py Updated module path and cleaned code formatting
blueye/protocol/init.py Removed legacy protocol exports from the main package
blueye/init.py Dropped deprecated pkg_resources namespace declaration
README.md Updated to uv/Ruff and removed setup.py instructions
.github/workflows/update-protocol.yml Switched CI from Poetry to uv in update workflow
.github/workflows/tests.yml Switched CI from Poetry to uv and flake8 to Ruff in tests
.github/workflows/publish-release.yml Switched CI from Poetry to uv for release publishing
.flake8 Removed old Flake8 configuration
Comments suppressed due to low confidence (4)

legacyprotocol/blueye/legacyprotocol/udp_protocol_parser.py:83

  • Using peek() only works on gzip file objects; for regular files open() doesn’t expose peek(). Consider reading the first two bytes with read(2) then seeking back or always wrapping in a buffered stream that supports peek.
version, packet_type = bin_file.peek(2)[:2]

legacyprotocol/blueye/legacyprotocol/udp_protocol_parser.py:52

  • Typo in error message: 'Endianess' should be spelled 'Endianness'.
raise ValueError("Endianess not supported")

legacyprotocol/blueye/legacyprotocol/udp_protocol_parser.py:1

  • Key methods like np_array_from_file and unpack_data lack associated tests. Consider adding unit tests to cover gzipped and non-gzipped parsing, edge cases, and version mismatches.
#!/usr/bin/env python3

legacyprotocol/pyproject.toml:24

  • The dev dependencies still include Flake8 even though the CI and docs now reference Ruff—remove Flake8 to avoid redundant linting tools.
"flake8~=7.1",

with open(protocol_definitions_path, "r") as f:
data = json.loads(f.read(), object_pairs_hook=OrderedDict)

protocol_md5 = md5(protocol_definitions_path)
Copy link

Copilot AI Jun 6, 2025

Choose a reason for hiding this comment

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

You compute protocol_md5 and generator_script_md5 but never inject them into data_file. Update the template to embed these checksums in the generated header so _json_hash and _generator_hash stay in sync.

Copilot uses AI. Check for mistakes.
@sindrehan sindrehan force-pushed the rework-project-structure branch 2 times, most recently from 88cf799 to 9175f78 Compare June 6, 2025 16:33
@sindrehan sindrehan force-pushed the rework-project-structure branch from 9175f78 to faa2516 Compare June 6, 2025 17:00
@sindrehan sindrehan merged commit a690163 into master Jun 6, 2025
12 checks passed
@sindrehan sindrehan deleted the rework-project-structure branch June 6, 2025 17:06
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.

Put numpy dependency behind feature flag Remove setup.py generator

2 participants