Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMigrates USB serial backend from pyserial-asyncio-fast to serialx, bumps package version to 0.47.7, raises minimum Python to 3.14.0, updates CHANGELOG, and increments CI cache version values. ChangesSerialx Library Migration & Dependency Update
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #443 +/- ##
==========================================
+ Coverage 81.76% 81.79% +0.02%
==========================================
Files 36 36
Lines 8195 8192 -3
==========================================
Hits 6701 6701
+ Misses 1494 1491 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
pyproject.toml (1)
7-7: Consider aligning development-status classifier with prerelease version.Line 7 sets an alpha version (
0.47.7a0), while Line 11 still declaresProduction/Stable. Consider switching classifier during prerelease cycles to avoid metadata ambiguity.📦 Optional metadata alignment
- "Development Status :: 5 - Production/Stable", + "Development Status :: 3 - Alpha",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pyproject.toml` at line 7, The package metadata declares a prerelease version string ("version = \"0.47.7a0\"") but the classifiers still indicate a production/stable release; update the Development Status classifier to match an alpha prerelease (for example change "Development Status :: 5 - Production/Stable" to "Development Status :: 3 - Alpha" or similar) so the metadata aligns with the prerelease version; locate the classifiers block in pyproject.toml and replace the Production/Stable entry with the appropriate prerelease classifier.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Line 5: Replace the empty PR link placeholder "PR []()" in CHANGELOG.md with a
proper markdown link or remove it: update the text to include the actual PR
number and URL (e.g., PR [`#123`](https://github.com/owner/repo/pull/123)) or
simply remove the empty link so the line becomes a valid sentence; ensure no
remaining "[]()" token remains in the file.
In `@plugwise_usb/connection/manager.py`:
- Line 10: The call site that invokes create_serial_connection (the block
currently translating SerialException/TimeoutError to StickError around the
exception mapping for SerialException in manager.py) must defensively translate
all invalid-URL related errors into StickError; update the exception handling
around create_serial_connection so it catches SerialException, TimeoutError,
TypeError, ValueError and a broad Exception fallback from
create_serial_connection and re-raises a StickError with the original exception
attached (preserving current behavior for SerialException/TimeoutError),
ensuring functions like create_serial_connection and SerialTransport creation
never leak dependency-specific exceptions outside (refer to the exception
mapping currently around create_serial_connection/SerialTransport in
manager.py).
---
Nitpick comments:
In `@pyproject.toml`:
- Line 7: The package metadata declares a prerelease version string ("version =
\"0.47.7a0\"") but the classifiers still indicate a production/stable release;
update the Development Status classifier to match an alpha prerelease (for
example change "Development Status :: 5 - Production/Stable" to "Development
Status :: 3 - Alpha" or similar) so the metadata aligns with the prerelease
version; locate the classifiers block in pyproject.toml and replace the
Production/Stable entry with the appropriate prerelease classifier.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0d183932-6ffd-4bcb-8b9d-9f850fec8ba9
📒 Files selected for processing (6)
CHANGELOG.mdplugwise_usb/connection/manager.pyplugwise_usb/connection/receiver.pypyproject.tomlrequirements_test.txttests/test_usb.py
There was a problem hiding this comment.
The read: https://developers.home-assistant.io/blog/2026/04/27/pyserial-to-serialx/
Furthermore works functional as is.. but it does seem to eat quite some cpu, and I see more no answer within 15s
There was a problem hiding this comment.
♻️ Duplicate comments (1)
plugwise_usb/connection/manager.py (1)
137-143:⚠️ Potential issue | 🟠 MajorCatch
OSErrorto preserve theStickErrorboundary for serial-open failures.Line 138 only catches
FileNotFoundError; other open failures (e.g., permission/IO errors) can still leak as rawOSErrorsubclasses instead of being normalized toStickError.Suggested patch
- except ( - FileNotFoundError, - SerialException, - TimeoutError, - TypeError, - ValueError, - ) as err: + except ( + OSError, + SerialException, + TimeoutError, + TypeError, + ValueError, + ) as err: raise StickError( f"Failed to open serial connection to {serial_path}" ) from errIn serialx, what exceptions can create_serial_connection(...) raise when opening an invalid/unavailable serial URL (missing device, permission denied, malformed path)? Please include whether these are OSError subclasses.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugwise_usb/connection/manager.py` around lines 137 - 143, The except block that currently catches (FileNotFoundError, SerialException, TimeoutError, TypeError, ValueError) should also include OSError so serial-open failures (permission/IO/malformed paths) are normalized to the StickError boundary; update the tuple in the except (...) as err clause (the handler that raises or wraps into StickError) to add OSError, ensuring the same error path is followed for create_serial_connection/open attempts and preserve the StickError semantics used elsewhere in the manager (refer to the existing SerialException and StickError handling).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@plugwise_usb/connection/manager.py`:
- Around line 137-143: The except block that currently catches
(FileNotFoundError, SerialException, TimeoutError, TypeError, ValueError) should
also include OSError so serial-open failures (permission/IO/malformed paths) are
normalized to the StickError boundary; update the tuple in the except (...) as
err clause (the handler that raises or wraps into StickError) to add OSError,
ensuring the same error path is followed for create_serial_connection/open
attempts and preserve the StickError semantics used elsewhere in the manager
(refer to the existing SerialException and StickError handling).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 414fe56d-1730-4dc1-89ba-badfcde3273d
📒 Files selected for processing (2)
CHANGELOG.mdplugwise_usb/connection/manager.py
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugwise_usb/connection/__init__.py`:
- Line 237: The except clause in send() uses invalid Python-2 syntax "except
NodeError, StickError:"; change it to the Python-3 tuple form by using "except
(NodeError, StickError):" (and add an exception variable like "as e" if the
block references the exception). Update the except block around send() to catch
(NodeError, StickError) so the module imports cleanly and any referenced
exception variable inside the block continues to work.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e7ff14a2-c51c-4b4d-8967-6b2eb49701e0
📒 Files selected for processing (2)
plugwise_usb/connection/__init__.pyplugwise_usb/connection/manager.py
Upload version to test pypi for testing
coderabbitai
Bot
added
dependencies
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
plugwise_usb/connection/manager.py (1)
124-140:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCatch
TimeoutErrorfromwait_forand re-raise asStickError.At Line 128,
wait_for(..., timeout=5)can raiseTimeoutError, but Line 137 does not catch it. That can leak a raw timeout exception and break theStickErrorcontract for setup failures.Suggested fix
- except (OSError, TypeError, ValueError) as err: + except (OSError, TimeoutError, TypeError, ValueError) as err: raise StickError( f"Failed to open serial connection to {serial_path}" ) from err#!/bin/bash # Verify timeout path and exception mapping in manager. rg -nP -C4 'wait_for\(|create_serial_connection\(|except\s+\(OSError,\s*TypeError,\s*ValueError\)' plugwise_usb/connection/manager.py # Verify how callers handle setup_connection_to_stick failures. rg -nP -C3 '\bsetup_connection_to_stick\s*\('🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugwise_usb/connection/manager.py` around lines 124 - 140, The wait_for call in setup_connection_to_stick can raise TimeoutError which is not currently caught, so update the exception handling around wait_for/create_serial_connection in setup_connection_to_stick to also catch TimeoutError (either add TimeoutError to the existing except tuple or add a separate except TimeoutError as err) and re-raise it as a StickError with a clear message that includes the serial_path, preserving the original exception as the __cause__ (raise StickError(...) from err) so callers of setup_connection_to_stick continue to receive StickError for all setup failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pyproject.toml`:
- Around line 7-15: The package metadata is inconsistent: version = "0.47.7a1"
indicates an alpha pre-release while the classifiers list contains "Development
Status :: 5 - Production/Stable"; update the classifiers to match the
pre-release status (for example change the Development Status classifier to
"Development Status :: 4 - Beta" or "Development Status :: 3 - Alpha" as
appropriate) and ensure the classifiers array (the classifiers symbol) and the
version string (the version symbol) reflect the same development phase so
packaging tools and consumers are not misled.
---
Duplicate comments:
In `@plugwise_usb/connection/manager.py`:
- Around line 124-140: The wait_for call in setup_connection_to_stick can raise
TimeoutError which is not currently caught, so update the exception handling
around wait_for/create_serial_connection in setup_connection_to_stick to also
catch TimeoutError (either add TimeoutError to the existing except tuple or add
a separate except TimeoutError as err) and re-raise it as a StickError with a
clear message that includes the serial_path, preserving the original exception
as the __cause__ (raise StickError(...) from err) so callers of
setup_connection_to_stick continue to receive StickError for all setup failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f46c237d-dc96-4d9f-861f-fd3bb2bc8d0d
📒 Files selected for processing (4)
.github/workflows/merge.yml.github/workflows/verify.ymlplugwise_usb/connection/manager.pypyproject.toml
✅ Files skipped from review due to trivial changes (2)
- .github/workflows/verify.yml
- .github/workflows/merge.yml
|
@coderabbitai TimeoutError is a subclass of OSError, the suggested fix adding TimeoutError is wrong. |
|
Tip For best results, initiate chat on the files or code changes.
✏️ Learnings added
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Line 5: Update the changelog line referencing the PR so the link text matches
the actual PR number: change the link text "PR [433]" to "PR [443]" in the entry
that reads "PR [433](https://github.com/plugwise/python-plugwise-usb/pull/443):
Migrate to serialx" so the displayed PR number and the URL both show 443.
In `@pyproject.toml`:
- Line 32: The pyproject.toml dependency entry for "serialx" is missing a
version constraint; update the dependency string "serialx" in pyproject.toml to
include a conservative version specifier (for example "serialx>=1.8.0" or
"serialx>=1.7.0,<2.0.0") to prevent pulling breaking changes from future
releases.
- Line 27: Update the Python requirement in pyproject.toml by replacing the
overly strict requires-python = ">=3.14.0" with a lower, justified minimum such
as requires-python = ">=3.10.0" to match the highest minimum dependency
(serialx) and restore compatibility with 3.10–3.13; ensure the change is made to
the requires-python entry so packaging tools pick up the new constraint.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 960b482e-47ad-4552-ba41-01f742c6e632
📒 Files selected for processing (2)
CHANGELOG.mdpyproject.toml
| { name = "dirixmjm" } | ||
| ] | ||
| requires-python = ">=3.13.0" | ||
| requires-python = ">=3.14.0" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check serialx Python version requirements
pip index versions serialx 2>/dev/null || echo "Package not found on PyPI"
# If available, check the package metadata
python -m pip download --no-deps serialx 2>&1 | head -20Repository: plugwise/python-plugwise-usb
Length of output: 541
🌐 Web query:
What is the minimum Python version required by the serialx library?
💡 Result:
The minimum Python version required by the serialx library is Python 3.10 [1]. This requirement is explicitly stated in the project's metadata on PyPI [1].
Citations:
🏁 Script executed:
#!/bin/bash
# Get the METADATA from the downloaded serialx wheel to see exact requires-python
cd /tmp && unzip -q serialx-1.8.0-py3-none-any.whl 'serialx*' 2>/dev/null || true
find . -name 'METADATA' -o -name 'WHEEL' 2>/dev/null | head -5
# Alternative: inspect wheel directly with zipinfo
cd /tmp && python3 << 'EOF'
import zipfile
import re
try:
with zipfile.ZipFile('serialx-1.8.0-py3-none-any.whl', 'r') as z:
for file in z.namelist():
if 'METADATA' in file or 'WHEEL' in file:
print(f"\n=== {file} ===")
content = z.read(file).decode('utf-8')
# Print only the first 100 lines or until we find requires-python
lines = content.split('\n')
for i, line in enumerate(lines[:100]):
if 'Requires-Python' in line or 'requires' in line.lower() or i < 20:
print(line)
except Exception as e:
print(f"Error: {e}")
EOFRepository: plugwise/python-plugwise-usb
Length of output: 149
🏁 Script executed:
#!/bin/bash
# Check plugwise pyproject.toml for all dependencies
cat pyproject.toml | grep -A 50 "dependencies\|requires-python"Repository: plugwise/python-plugwise-usb
Length of output: 1433
🏁 Script executed:
#!/bin/bash
# Check the Python version requirements for the dependencies listed
# aiofiles, crcmod, semver, serialx
for package in aiofiles crcmod semver serialx; do
echo "=== $package ==="
pip index versions "$package" 2>/dev/null | head -1
done
# Also check if there are any Python 3.14 specific features used in the codebase
echo -e "\n=== Checking for Python 3.14 specific features ==="
rg "from __future__ import" pyproject.toml src/ 2>/dev/null || echo "No future imports found"
rg "3\.14|3\.13|3\.12|3\.11|3\.10" . --type py --max-count 10 2>/dev/null | head -20Repository: plugwise/python-plugwise-usb
Length of output: 273
🌐 Web query:
Python version requirements for aiofiles crcmod semver packages - what is the minimum Python version supported
💡 Result:
The minimum Python version requirements for the requested packages are as follows: aiofiles: The current version (25.1.0) requires Python 3.9 or higher [1][2]. Support for older versions was dropped in previous releases (e.g., Python 3.8 support was dropped in version 25.0.0/24.1.0, and Python 3.7 support was dropped in version 24.0.0/23.2.1) [3]. crcmod: The package (version 1.7) supports both Python 2.x and 3.x series [4][5]. Specifically, it has been tested on Python 2.4–2.7 and Python 3.1 [4][6]. While it may work on other versions, these are the explicitly documented versions [5]. semver: The current major version (3.0.0 and later) requires Python 3.7 or higher [7]. The 2.x series supported Python 2.7 through 3.5, but this branch is frozen and no longer receives updates [7].
Citations:
- 1: https://pypi.org/project/aiofiles/
- 2: https://generalistprogrammer.com/tutorials/aiofiles-python-package-guide
- 3: https://github.com/Tinche/aiofiles/blob/main/CHANGELOG.md
- 4: https://pypi.python.org/pypi/crcmod/1.7
- 5: https://crcmod.sourceforge.net/intro.html
- 6: https://pypi.org/project/crcmod/
- 7: https://pypi.org/project/semver/3.0.0/
Remove the unnecessary Python 3.14 minimum requirement.
The requires-python = ">=3.14.0" constraint is overly restrictive and not justified by the package's dependencies. The minimum Python versions for the stated dependencies are:
serialx1.8.0: Python 3.10+aiofiles25.1.0: Python 3.9+semver3.0.4: Python 3.7+crcmod1.7: Python 2.7+ (no constraint)
The codebase contains no Python 3.14-specific features. Lowering the requirement to >=3.10.0 (matching serialx) would maintain compatibility with Python 3.10, 3.11, 3.12, and 3.13 users while preserving all functionality.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyproject.toml` at line 27, Update the Python requirement in pyproject.toml
by replacing the overly strict requires-python = ">=3.14.0" with a lower,
justified minimum such as requires-python = ">=3.10.0" to match the highest
minimum dependency (serialx) and restore compatibility with 3.10–3.13; ensure
the change is made to the requires-python entry so packaging tools pick up the
new constraint.
| "aiofiles", | ||
| "crcmod", | ||
| "semver", | ||
| "serialx", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if serialx is available and get version info
pip index versions serialx 2>/dev/null | head -20
# Check the package metadata if available
curl -s https://pypi.org/pypi/serialx/json | jq -r '.info | {name, version, author, home_page, requires_python, summary}'Repository: plugwise/python-plugwise-usb
Length of output: 546
🌐 Web query:
What is the latest stable version of the serialx Python library and what are its Python version requirements?
💡 Result:
As of May 18, 2026, the latest stable version of the serialx Python library is v1.7.3 [1]. This library requires Python 3.10 or newer [1].
Citations:
Add a version constraint to the serialx dependency.
The dependency on serialx lacks a version specifier. Since serialx 1.8.0 is the latest stable release, consider pinning to at least >=1.8.0 or >=1.7.0,<2.0.0 to prevent unexpected breaking changes from future releases.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyproject.toml` at line 32, The pyproject.toml dependency entry for "serialx"
is missing a version constraint; update the dependency string "serialx" in
pyproject.toml to include a conservative version specifier (for example
"serialx>=1.8.0" or "serialx>=1.7.0,<2.0.0") to prevent pulling breaking changes
from future releases.
|
2 participants
Summary by CodeRabbit
Chores
Documentation