Implement resetting of energylogs #252
Conversation
|
""" WalkthroughThis update introduces an asynchronous energy reset feature to the Plugwise USB library. It adds new methods for sending energy reset requests to nodes, enforces node-type validation for energy-related operations, and documents the changes in the changelog. The project version is incremented to 0.42.0. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Stick
participant StickNetwork
participant Node
User->>Stick: energy_reset_request(mac)
Stick->>StickNetwork: energy_reset_request(mac)
StickNetwork->>StickNetwork: _validate_energy_node(mac)
StickNetwork->>Node: Send CircleClockSetRequest(reset=True)
Node-->>StickNetwork: Response (acknowledgment)
StickNetwork-->>Stick: Raise error or return success
Stick->>StickNetwork: set_energy_intervals(mac, 60, 0)
StickNetwork->>StickNetwork: _validate_energy_node(mac)
StickNetwork-->>Stick: Return result or False
Stick-->>User: Return result or False
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
bouwew
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #252 +/- ##
==========================================
- Coverage 80.97% 80.74% -0.24%
==========================================
Files 36 36
Lines 7418 7453 +35
==========================================
+ Hits 6007 6018 +11
- Misses 1411 1435 +24 ☔ 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: 6
🧹 Nitpick comments (2)
plugwise_usb/nodes/node.py (1)
103-110: Simplifynode_protocolsproperty.The explicit
Nonecheck is redundant—returningself._node_protocolsalready yieldsNonewhen unset. Streamline for clarity.Proposed diff:
- @property - def node_protocols(self) -> SupportedVersions | None: - """Return the node_protocols for the Node.""" - if self._node_protocols is None: - return None - - return self._node_protocols + @property + def node_protocols(self) -> SupportedVersions | None: + """Return supported protocol versions for this node, or None if not determined.""" + return self._node_protocolsplugwise_usb/messages/requests.py (1)
15-15: Clarify the reason for commenting out LOGADDR_OFFSET import.The
LOGADDR_OFFSETimport is commented out but it's unclear if this is a temporary change or permanent removal. If this constant is no longer needed, consider removing the import entirely rather than commenting it out.-# LOGADDR_OFFSET,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
CHANGELOG.md(1 hunks)plugwise_usb/__init__.py(3 hunks)plugwise_usb/messages/requests.py(2 hunks)plugwise_usb/network/__init__.py(4 hunks)plugwise_usb/nodes/node.py(1 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
plugwise_usb/messages/requests.py (1)
plugwise_usb/messages/properties.py (1)
String(80-97)
plugwise_usb/nodes/node.py (1)
plugwise_usb/nodes/helpers/firmware.py (1)
SupportedVersions(19-23)
🪛 Ruff (0.11.9)
plugwise_usb/messages/requests.py
767-767: Trailing whitespace
Remove trailing whitespace
(W291)
🔇 Additional comments (4)
plugwise_usb/__init__.py (2)
13-13: LGTM!Import reordering is clean and maintains alphabetical order.
311-311: LGTM!Good cleanup of trailing whitespace.
plugwise_usb/network/__init__.py (2)
9-9: LGTM!Good addition of
UTCimport for proper timezone handling in energy reset functionality.
18-20: LGTM!Import reordering maintains good organization and adds the necessary
CircleClockSetRequestfor the new energy reset feature.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
plugwise_usb/messages/requests.py (3)
15-15: Remove unused import
The constantLOGADDR_OFFSETis imported but not used anywhere in this file. Please remove it to clean up unused imports.🧰 Tools
🪛 Ruff (0.11.9)
15-15:
..constants.LOGADDR_OFFSETimported but unusedRemove unused import:
..constants.LOGADDR_OFFSET(F401)
767-767: Remove trailing whitespace
The linethis_date,has trailing spaces. Please trim the extra whitespace.🧰 Tools
🪛 Ruff (0.11.9)
767-767: Trailing whitespace
Remove trailing whitespace
(W291)
768-768: Introduce named constant for reset log address
Using the literal0inLogAddr(0, 8, False)is a magic value. Consider defining a descriptive constant (e.g.,ENERGY_RESET_LOG_ADDR_INDEX = 0) at the top of the file and using it here for better readability and maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugwise_usb/messages/requests.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
plugwise_usb/messages/requests.py (1)
plugwise_usb/messages/properties.py (1)
LogAddr(384-404)
🪛 Ruff (0.11.9)
plugwise_usb/messages/requests.py
767-767: Trailing whitespace
Remove trailing whitespace
(W291)
🪛 GitHub Actions: Latest commit
plugwise_usb/messages/requests.py
[warning] 12-12: pylint: Unused LOGADDR_OFFSET imported from constants (unused-import)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
plugwise_usb/network/__init__.py (1)
548-576:⚠️ Potential issueFix response attribute inconsistency in error message.
The method implementation looks good overall with proper validation, but there's an inconsistency in the error handling at the end.
Apply this diff to fix the inconsistent response attribute usage:
if response.ack_id != NodeResponseType.CLOCK_ACCEPTED: raise MessageError( - f"Unknown NodeResponseType '{response.response_type.name}' received" + f"Unknown NodeResponseType '{response.ack_id.name}' received" )The code checks
response.ack_idbut referencesresponse.response_type.namein the error message, which is inconsistent.🧰 Tools
🪛 Ruff (0.11.9)
555-555: Blank line contains whitespace
Remove whitespace from blank line
(W293)
🧹 Nitpick comments (1)
plugwise_usb/network/__init__.py (1)
555-555: Remove whitespace from blank line.- +🧰 Tools
🪛 Ruff (0.11.9)
555-555: Blank line contains whitespace
Remove whitespace from blank line
(W293)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
plugwise_usb/__init__.py(3 hunks)plugwise_usb/messages/requests.py(1 hunks)plugwise_usb/network/__init__.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- plugwise_usb/messages/requests.py
- plugwise_usb/init.py
🧰 Additional context used
🪛 Ruff (0.11.9)
plugwise_usb/network/__init__.py
555-555: Blank line contains whitespace
Remove whitespace from blank line
(W293)
🔇 Additional comments (3)
plugwise_usb/network/__init__.py (3)
9-9: LGTM! Import additions support the new functionality.The new imports are appropriate for the added energy reset functionality and MAC validation.
Also applies to: 16-16, 19-19, 21-21
37-38: Excellent use of constants for improved maintainability.Defining
ENERGY_NODE_TYPESas a module-level constant improves code maintainability and makes the supported node types explicit. This addresses the previous suggestion to avoid hardcoded values.
585-595: Great addition of input validation to existing method.The addition of MAC validation, node existence checks, and node type validation to the
set_energy_intervalsmethod improves robustness and consistency with the newenergy_reset_requestmethod.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
plugwise_usb/network/__init__.py (1)
548-577: Improve error message formatting.The method implementation is solid with proper validation and error handling. However, the error message on lines 574-576 uses string concatenation instead of a more readable f-string.
Apply this diff to improve readability:
- raise MessageError( - f"Unexpected NodeResponseType {response.ack_id!r} " - +"received in response to CircleClockSetRequest " - ) + raise MessageError( + f"Unexpected NodeResponseType {response.ack_id!r} received in response to CircleClockSetRequest" + )Note: Static analysis indicates this new functionality lacks test coverage. Consider adding unit tests to verify the validation logic and error handling paths.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 550-551: plugwise_usb/network/init.py#L550-L551
Added lines #L550 - L551 were not covered by tests
[warning] 553-554: plugwise_usb/network/init.py#L553-L554
Added lines #L553 - L554 were not covered by tests
[warning] 556-557: plugwise_usb/network/init.py#L556-L557
Added lines #L556 - L557 were not covered by tests
[warning] 561-562: plugwise_usb/network/init.py#L561-L562
Added lines #L561 - L562 were not covered by tests
[warning] 569-570: plugwise_usb/network/init.py#L569-L570
Added lines #L569 - L570 were not covered by tests
[warning] 572-573: plugwise_usb/network/init.py#L572-L573
Added lines #L572 - L573 were not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugwise_usb/network/__init__.py(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
plugwise_usb/network/__init__.py (5)
plugwise_usb/api.py (3)
NodeType(57-68)mac(270-271)name(242-243)plugwise_usb/helpers/util.py (1)
validate_mac(13-21)plugwise_usb/exceptions.py (2)
MessageError(20-21)NodeError(24-25)plugwise_usb/messages/requests.py (18)
CircleClockSetRequest(730-783)CircleMeasureIntervalRequest(1261-1292)send(364-373)send(386-395)send(437-439)send(475-484)send(511-520)send(541-552)send(565-574)send(587-596)send(620-629)send(666-675)send(718-727)send(774-783)send(807-816)send(847-856)send(879-888)response(124-128)plugwise_usb/__init__.py (2)
energy_reset_request(213-225)name(141-143)
🪛 GitHub Check: codecov/patch
plugwise_usb/network/__init__.py
[warning] 550-551: plugwise_usb/network/init.py#L550-L551
Added lines #L550 - L551 were not covered by tests
[warning] 553-554: plugwise_usb/network/init.py#L553-L554
Added lines #L553 - L554 were not covered by tests
[warning] 556-557: plugwise_usb/network/init.py#L556-L557
Added lines #L556 - L557 were not covered by tests
[warning] 561-562: plugwise_usb/network/init.py#L561-L562
Added lines #L561 - L562 were not covered by tests
[warning] 569-570: plugwise_usb/network/init.py#L569-L570
Added lines #L569 - L570 were not covered by tests
[warning] 572-573: plugwise_usb/network/init.py#L572-L573
Added lines #L572 - L573 were not covered by tests
[warning] 586-587: plugwise_usb/network/init.py#L586-L587
Added lines #L586 - L587 were not covered by tests
[warning] 589-590: plugwise_usb/network/init.py#L589-L590
Added lines #L589 - L590 were not covered by tests
[warning] 592-593: plugwise_usb/network/init.py#L592-L593
Added lines #L592 - L593 were not covered by tests
🔇 Additional comments (3)
plugwise_usb/network/__init__.py (3)
9-9: LGTM! Import additions are appropriate.All new imports (
UTC,validate_mac,CircleClockSetRequest) are properly used in the new energy reset functionality.Also applies to: 16-16, 19-19
37-37: Excellent use of constants for node type validation.Defining
ENERGY_NODE_TYPESimproves code maintainability and makes the supported node types (Circle+, Circle, Stealth) explicit rather than using magic numbers.
586-595: Excellent validation consistency.The added validation logic creates a consistent pattern with the new
energy_reset_requestmethod. Using the sameENERGY_NODE_TYPESconstant and validation sequence improves code maintainability.Note: Static analysis indicates the new validation logic lacks test coverage. Consider adding tests to verify MAC validation, node existence checks, and node type validation for both methods.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 586-587: plugwise_usb/network/init.py#L586-L587
Added lines #L586 - L587 were not covered by tests
[warning] 589-590: plugwise_usb/network/init.py#L589-L590
Added lines #L589 - L590 were not covered by tests
[warning] 592-593: plugwise_usb/network/init.py#L592-L593
Added lines #L592 - L593 were not covered by tests
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
plugwise_usb/network/__init__.py (1)
548-594: Consider extracting common validation logic for better maintainability.Both
energy_reset_requestandset_energy_intervalsshare identical validation logic for MAC format, node existence, and node type checking. Consider extracting this into a private helper method to follow the DRY principle.def _validate_energy_node(self, mac: str) -> None: """Validate node for energy operations.""" if not validate_mac(mac): raise NodeError(f"MAC '{mac}' invalid") if mac not in self._nodes: raise NodeError(f"Node {mac} not present in network") if self._nodes[mac].node_info.node_type.value not in ENERGY_NODE_TYPES: raise NodeError( f"Energy operations not supported for {self._nodes[mac].node_info.node_type.name}" )Then both methods can simply call
self._validate_energy_node(mac)at the beginning.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 550-551: plugwise_usb/network/init.py#L550-L551
Added lines #L550 - L551 were not covered by tests
[warning] 553-554: plugwise_usb/network/init.py#L553-L554
Added lines #L553 - L554 were not covered by tests
[warning] 556-557: plugwise_usb/network/init.py#L556-L557
Added lines #L556 - L557 were not covered by tests
[warning] 561-562: plugwise_usb/network/init.py#L561-L562
Added lines #L561 - L562 were not covered by tests
[warning] 569-570: plugwise_usb/network/init.py#L569-L570
Added lines #L569 - L570 were not covered by tests
[warning] 572-573: plugwise_usb/network/init.py#L572-L573
Added lines #L572 - L573 were not covered by tests
[warning] 585-586: plugwise_usb/network/init.py#L585-L586
Added lines #L585 - L586 were not covered by tests
[warning] 588-589: plugwise_usb/network/init.py#L588-L589
Added lines #L588 - L589 were not covered by tests
[warning] 591-592: plugwise_usb/network/init.py#L591-L592
Added lines #L591 - L592 were not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugwise_usb/network/__init__.py(4 hunks)pyproject.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
🧰 Additional context used
🧬 Code Graph Analysis (1)
plugwise_usb/network/__init__.py (6)
plugwise_usb/api.py (3)
NodeType(57-68)mac(270-271)name(242-243)plugwise_usb/helpers/util.py (1)
validate_mac(13-21)plugwise_usb/exceptions.py (2)
MessageError(20-21)NodeError(24-25)plugwise_usb/messages/requests.py (19)
CircleClockSetRequest(730-783)CircleMeasureIntervalRequest(1261-1292)CirclePlusAllowJoiningRequest(453-484)send(364-373)send(386-395)send(437-439)send(475-484)send(511-520)send(541-552)send(565-574)send(587-596)send(620-629)send(666-675)send(718-727)send(774-783)send(807-816)send(847-856)send(879-888)response(124-128)plugwise_usb/__init__.py (2)
energy_reset_request(213-225)name(141-143)plugwise_usb/messages/responses.py (1)
NodeResponseType(56-72)
🪛 GitHub Check: codecov/patch
plugwise_usb/network/__init__.py
[warning] 550-551: plugwise_usb/network/init.py#L550-L551
Added lines #L550 - L551 were not covered by tests
[warning] 553-554: plugwise_usb/network/init.py#L553-L554
Added lines #L553 - L554 were not covered by tests
[warning] 556-557: plugwise_usb/network/init.py#L556-L557
Added lines #L556 - L557 were not covered by tests
[warning] 561-562: plugwise_usb/network/init.py#L561-L562
Added lines #L561 - L562 were not covered by tests
[warning] 569-570: plugwise_usb/network/init.py#L569-L570
Added lines #L569 - L570 were not covered by tests
[warning] 572-573: plugwise_usb/network/init.py#L572-L573
Added lines #L572 - L573 were not covered by tests
[warning] 585-586: plugwise_usb/network/init.py#L585-L586
Added lines #L585 - L586 were not covered by tests
[warning] 588-589: plugwise_usb/network/init.py#L588-L589
Added lines #L588 - L589 were not covered by tests
[warning] 591-592: plugwise_usb/network/init.py#L591-L592
Added lines #L591 - L592 were not covered by tests
🔇 Additional comments (4)
plugwise_usb/network/__init__.py (4)
9-9: LGTM: Import additions are correct and necessary.The new imports (
UTC,validate_mac,CircleClockSetRequest) are properly placed and required for the energy reset functionality.Also applies to: 16-16, 19-19, 21-21
37-37: Excellent: Addresses previous review feedback.The constant
ENERGY_NODE_TYPESsuccessfully addresses the past review comment about creating a constant for supported node types (Circle+, Circle, Stealth). This improves maintainability and makes the supported types explicit.
548-576: LGTM: Well-implemented energy reset method with proper validation.The implementation correctly addresses all the issues raised in past reviews:
- ✅ Uses f-string formatting properly
- ✅ Validates MAC format before use
- ✅ Checks node existence before access
- ✅ Uses the new
ENERGY_NODE_TYPESconstant- ✅ Proper error handling with meaningful messages
- ✅ Consistent response validation using
response.ack_idThe method follows established patterns in the codebase and integrates well with the existing architecture.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 550-551: plugwise_usb/network/init.py#L550-L551
Added lines #L550 - L551 were not covered by tests
[warning] 553-554: plugwise_usb/network/init.py#L553-L554
Added lines #L553 - L554 were not covered by tests
[warning] 556-557: plugwise_usb/network/init.py#L556-L557
Added lines #L556 - L557 were not covered by tests
[warning] 561-562: plugwise_usb/network/init.py#L561-L562
Added lines #L561 - L562 were not covered by tests
[warning] 569-570: plugwise_usb/network/init.py#L569-L570
Added lines #L569 - L570 were not covered by tests
[warning] 572-573: plugwise_usb/network/init.py#L572-L573
Added lines #L572 - L573 were not covered by tests
585-594: LGTM: Consistent validation pattern applied.The addition of MAC validation, node existence check, and node type validation to
set_energy_intervalsmaintains consistency with the newenergy_reset_requestmethod. The validation logic follows the same pattern and uses the sharedENERGY_NODE_TYPESconstant.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 585-586: plugwise_usb/network/init.py#L585-L586
Added lines #L585 - L586 were not covered by tests
[warning] 588-589: plugwise_usb/network/init.py#L588-L589
Added lines #L588 - L589 were not covered by tests
[warning] 591-592: plugwise_usb/network/init.py#L591-L592
Added lines #L591 - L592 were not covered by tests
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
plugwise_usb/__init__.py (1)
222-225:⚠️ Potential issueFix unreachable code in return logic.
The current logic has an issue: since
set_energy_intervalsalways returnsTrue(line 235), thereturn Falseon line 225 is unreachable code.Consider this fix:
# Follow up by an energy-intervals (re)set -if (result := await self.set_energy_intervals(mac, 60, 0)): - return result - -return False +await self.set_energy_intervals(mac, 60, 0) +return TrueThis makes the intent clearer and removes the unreachable code.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 222-223: plugwise_usb/init.py#L222-L223
Added lines #L222 - L223 were not covered by tests
[warning] 225-225: plugwise_usb/init.py#L225
Added line #L225 was not covered by tests
🧹 Nitpick comments (1)
plugwise_usb/network/__init__.py (1)
596-607: Fix whitespace issues in validation method.The validation logic is sound, but there are whitespace issues that need to be addressed.
Apply this diff to fix the whitespace issues:
def _validate_energy_node(self, mac: str) -> None: """Validate node for energy operations.""" if not validate_mac(mac): raise NodeError(f"MAC '{mac}' invalid") - + if mac not in self._nodes: raise NodeError(f"Node {mac} not present in network") - + if self._nodes[mac].node_info.node_type.value not in ENERGY_NODE_TYPES: raise NodeError( f"Energy operations not supported for {self._nodes[mac].node_info.node_type.name}" )🧰 Tools
🪛 Ruff (0.11.9)
600-600: Blank line contains whitespace
Remove whitespace from blank line
(W293)
603-603: Blank line contains whitespace
Remove whitespace from blank line
(W293)
🪛 GitHub Check: codecov/patch
[warning] 598-599: plugwise_usb/network/init.py#L598-L599
Added lines #L598 - L599 were not covered by tests
[warning] 601-602: plugwise_usb/network/init.py#L601-L602
Added lines #L601 - L602 were not covered by tests
[warning] 604-605: plugwise_usb/network/init.py#L604-L605
Added lines #L604 - L605 were not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
CHANGELOG.md(1 hunks)plugwise_usb/__init__.py(3 hunks)plugwise_usb/messages/requests.py(1 hunks)plugwise_usb/network/__init__.py(5 hunks)plugwise_usb/nodes/node.py(1 hunks)pyproject.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- pyproject.toml
- CHANGELOG.md
- plugwise_usb/messages/requests.py
🧰 Additional context used
🧠 Learnings (1)
plugwise_usb/__init__.py (1)
Learnt from: bouwew
PR: plugwise/python-plugwise-usb#252
File: plugwise_usb/__init__.py:0-0
Timestamp: 2025-06-03T09:58:44.592Z
Learning: In the plugwise_usb codebase, bouwew prefers not to add excessive validation decorators or protective error handling when they would amount to "too much protection". The existing validation level is considered sufficient.
🧬 Code Graph Analysis (1)
plugwise_usb/nodes/node.py (1)
plugwise_usb/nodes/helpers/firmware.py (1)
SupportedVersions(19-23)
🪛 GitHub Check: codecov/patch
plugwise_usb/__init__.py
[warning] 215-219: plugwise_usb/init.py#L215-L219
Added lines #L215 - L219 were not covered by tests
[warning] 222-223: plugwise_usb/init.py#L222-L223
Added lines #L222 - L223 were not covered by tests
[warning] 225-225: plugwise_usb/init.py#L225
Added line #L225 was not covered by tests
plugwise_usb/network/__init__.py
[warning] 550-552: plugwise_usb/network/init.py#L550-L552
Added lines #L550 - L552 were not covered by tests
[warning] 559-560: plugwise_usb/network/init.py#L559-L560
Added lines #L559 - L560 were not covered by tests
[warning] 562-563: plugwise_usb/network/init.py#L562-L563
Added lines #L562 - L563 were not covered by tests
[warning] 575-575: plugwise_usb/network/init.py#L575
Added line #L575 was not covered by tests
[warning] 598-599: plugwise_usb/network/init.py#L598-L599
Added lines #L598 - L599 were not covered by tests
[warning] 601-602: plugwise_usb/network/init.py#L601-L602
Added lines #L601 - L602 were not covered by tests
[warning] 604-605: plugwise_usb/network/init.py#L604-L605
Added lines #L604 - L605 were not covered by tests
plugwise_usb/nodes/node.py
[warning] 106-107: plugwise_usb/nodes/node.py#L106-L107
Added lines #L106 - L107 were not covered by tests
[warning] 109-109: plugwise_usb/nodes/node.py#L109
Added line #L109 was not covered by tests
🪛 Ruff (0.11.9)
plugwise_usb/network/__init__.py
600-600: Blank line contains whitespace
Remove whitespace from blank line
(W293)
603-603: Blank line contains whitespace
Remove whitespace from blank line
(W293)
🔇 Additional comments (3)
plugwise_usb/nodes/node.py (1)
103-109: LGTM! Clean property implementation.The
node_protocolsproperty is well-implemented with proper documentation and clear logic.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 106-107: plugwise_usb/nodes/node.py#L106-L107
Added lines #L106 - L107 were not covered by tests
[warning] 109-109: plugwise_usb/nodes/node.py#L109
Added line #L109 was not covered by testsplugwise_usb/network/__init__.py (2)
37-37: Good addition of the constant for maintainability.The
ENERGY_NODE_TYPESconstant improves code maintainability by centralizing the supported node types for energy operations.
548-565: Well-implemented energy reset functionality.The
energy_reset_requestmethod follows good patterns with proper validation, error handling, and clear logic flow.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 550-552: plugwise_usb/network/init.py#L550-L552
Added lines #L550 - L552 were not covered by tests
[warning] 559-560: plugwise_usb/network/init.py#L559-L560
Added lines #L559 - L560 were not covered by tests
[warning] 562-563: plugwise_usb/network/init.py#L562-L563
Added lines #L562 - L563 were not covered by tests
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
plugwise_usb/network/__init__.py (1)
595-607: Fix whitespace issues in validation method.The validation method logic is correct and follows good practices by centralizing validation for energy operations. However, there are whitespace issues that need to be addressed.
Apply this diff to fix the whitespace issues:
def _validate_energy_node(self, mac: str) -> None: """Validate node for energy operations.""" if not validate_mac(mac): raise NodeError(f"MAC '{mac}' invalid") - + if mac not in self._nodes: raise NodeError(f"Node {mac} not present in network") - + if self._nodes[mac].node_info.node_type.value not in ENERGY_NODE_TYPES: raise NodeError( f"Energy operations not supported for {self._nodes[mac].node_info.node_type.name}" )🧰 Tools
🪛 Ruff (0.11.9)
599-599: Blank line contains whitespace
Remove whitespace from blank line
(W293)
602-602: Blank line contains whitespace
Remove whitespace from blank line
(W293)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugwise_usb/network/__init__.py(5 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
plugwise_usb/network/__init__.py
599-599: Blank line contains whitespace
Remove whitespace from blank line
(W293)
602-602: Blank line contains whitespace
Remove whitespace from blank line
(W293)
🔇 Additional comments (4)
plugwise_usb/network/__init__.py (4)
9-9: LGTM: Import additions support the new energy reset functionality.The new imports are appropriately added to support the energy reset feature:
UTCfrom datetime for timezone-aware timestampsvalidate_macfor MAC address validationCircleClockSetRequestfor the energy reset requestAlso applies to: 16-16, 19-19, 21-21
37-37: Good practice: Centralized constant for supported energy node types.The
ENERGY_NODE_TYPESconstant improves maintainability by centralizing the definition of supported node types (Circle, Circle+, Stealth) and makes the code more readable.
548-566: Well-implemented energy reset functionality.The
energy_reset_requestmethod is properly implemented with:
- Appropriate validation using the helper method
- Correct use of
CircleClockSetRequestwith reset flag- Proper error handling and response validation
- Clear error messages
The implementation follows the established patterns in the codebase.
575-575: Good refactor: Consistent validation across energy methods.Adding the
_validate_energy_node(mac)call toset_energy_intervalsimproves consistency and reduces code duplication. Both energy-related methods now use the same validation logic.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
plugwise_usb/__init__.py (1)
214-230:⚠️ Potential issueFix the unreachable code and potential NoneType error.
The method has two issues:
Unreachable code: The
set_energy_intervalsmethod always returnsTrueon success (see lines 236-240), making thereturn Falsestatement unreachable.Potential AttributeError: If
self._networkisNone, callingenergy_reset_requeston it will raise an AttributeError.Apply this diff to fix the logic:
async def energy_reset_request(self, mac: str) -> bool: """Send an energy-reset request to a Node.""" _LOGGER.debug("Resetting energy logs for %s", mac) + if self._network is None: + raise NodeError("Network is not initialized") + try: await self._network.energy_reset_request(mac) except (MessageError, NodeError) as exc: raise NodeError(f"{exc}") from exc # Follow up by an energy-intervals (re)set - if ( - result := await self.set_energy_intervals( - mac, DEFAULT_CONS_INTERVAL, NO_PRODUCTION_INTERVAL - ) - ): - return result - - return False + try: + await self.set_energy_intervals( + mac, DEFAULT_CONS_INTERVAL, NO_PRODUCTION_INTERVAL + ) + return True + except NodeError: + # Energy reset succeeded but interval setting failed + # Return True since the main operation succeeded + return True🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 216-220: plugwise_usb/init.py#L216-L220
Added lines #L216 - L220 were not covered by tests
[warning] 223-223: plugwise_usb/init.py#L223
Added line #L223 was not covered by tests
[warning] 228-228: plugwise_usb/init.py#L228
Added line #L228 was not covered by tests
[warning] 230-230: plugwise_usb/init.py#L230
Added line #L230 was not covered by tests
🧹 Nitpick comments (1)
plugwise_usb/__init__.py (1)
214-230: Add test coverage for the new energy reset functionality.The static analysis indicates that the new
energy_reset_requestmethod lacks test coverage, which is important for ensuring the energy reset functionality works correctly.Would you like me to help generate unit tests for this new method to improve the test coverage?
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 216-220: plugwise_usb/init.py#L216-L220
Added lines #L216 - L220 were not covered by tests
[warning] 223-223: plugwise_usb/init.py#L223
Added line #L223 was not covered by tests
[warning] 228-228: plugwise_usb/init.py#L228
Added line #L228 was not covered by tests
[warning] 230-230: plugwise_usb/init.py#L230
Added line #L230 was not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugwise_usb/__init__.py(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
plugwise_usb/__init__.py (1)
Learnt from: bouwew
PR: plugwise/python-plugwise-usb#252
File: plugwise_usb/__init__.py:0-0
Timestamp: 2025-06-03T09:58:44.592Z
Learning: In the plugwise_usb codebase, bouwew prefers not to add excessive validation decorators or protective error handling when they would amount to "too much protection". The existing validation level is considered sufficient.
🪛 GitHub Check: codecov/patch
plugwise_usb/__init__.py
[warning] 216-220: plugwise_usb/init.py#L216-L220
Added lines #L216 - L220 were not covered by tests
[warning] 223-223: plugwise_usb/init.py#L223
Added line #L223 was not covered by tests
[warning] 228-228: plugwise_usb/init.py#L228
Added line #L228 was not covered by tests
[warning] 230-230: plugwise_usb/init.py#L230
Added line #L230 was not covered by tests
🔇 Additional comments (2)
plugwise_usb/__init__.py (2)
13-13: LGTM: Import reorganization looks good.The typing imports reorganization and addition of the new constants are appropriate for the new functionality.
Also applies to: 17-17
317-317: Good cleanup: Trailing whitespace removal.Good practice to remove trailing whitespace.
|
@coderabbitai please suggest test code for the added lines of code. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@bouwew I'll analyze the existing test structure and suggest comprehensive test code for the new energy reset functionality. Let me first examine the current test patterns. Based on the codebase analysis, here are comprehensive test suggestions for the new energy reset functionality: Test Code Suggestions1. Tests for
|
|
@coderabbitai please correct the code to add to stick_test_data.py to using 1111111111111111 as mac address. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@bouwew I'll examine the current Based on the test data structure, here's the corrected code for Corrected Test Data for
|
|
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores