-
Notifications
You must be signed in to change notification settings - Fork 2
implement (stateless) switch event propagation #282
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…mplementations. Update switch to publish switch group number
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant Resp as NodeSwitchGroupResponse
participant Switch as PlugwiseSwitch
participant API as SwitchGroup (dataclass)
participant Subscriber as Subscriber
Resp->>Switch: Response with state, group, timestamp
Switch->>API: Create/Update SwitchGroup(state, group, timestamp)
Switch->>Subscriber: Publish updated SwitchGroup object
Subscriber->>Switch: Request get_state(NodeFeature.SWITCH)
Switch->>Subscriber: Return SwitchGroup object
Poem
✨ Finishing Touches
🧪 Generate unit tests
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 (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #282 +/- ##
==========================================
+ Coverage 80.85% 80.90% +0.05%
==========================================
Files 36 36
Lines 7583 7584 +1
==========================================
+ Hits 6131 6136 +5
+ Misses 1452 1448 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugwise_usb/api.py (1)
208-215: LGTM! Consider consistency with frozen dataclasses.The
SwitchGroupdataclass is well-designed to encapsulate switch state information. The implementation looks correct and follows the established patterns in the codebase.However, notice that other similar state classes like
AvailableState,RelayState, etc. are marked asfrozen=Truefor immutability. Consider whetherSwitchGroupshould also be frozen for consistency, or if mutability is intentionally required for the switch implementation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
plugwise_usb/api.py(1 hunks)plugwise_usb/messages/responses.py(1 hunks)plugwise_usb/nodes/switch.py(5 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
plugwise_usb/messages/responses.py
[warning] 864-864: plugwise_usb/messages/responses.py#L864
Added line #L864 was not covered by tests
plugwise_usb/nodes/switch.py
[warning] 119-121: plugwise_usb/nodes/switch.py#L119-L121
Added lines #L119 - L121 were not covered by tests
[warning] 123-123: plugwise_usb/nodes/switch.py#L123
Added line #L123 was not covered by tests
🔇 Additional comments (8)
plugwise_usb/messages/responses.py (1)
861-864: LGTM! Simple and correct property implementation.The
switch_groupproperty correctly exposes the group value as an integer. The implementation follows the established patterns in the class.Note: Static analysis indicates this line is not covered by tests, but since this is a simple getter property, the risk is minimal.
plugwise_usb/nodes/switch.py (7)
9-11: LGTM! Proper imports for the refactoring.The addition of
Anytype hint andSwitchGroupimport correctly supports the refactoring to use the new dataclass.
39-39: LGTM! Correct initialization of SwitchGroup.The initialization of
_switchas aSwitchGroup()object properly replaces the previous boolean state and aligns with the new unified data structure.
73-73: LGTM! Method rename reflects updated functionality.The rename from
_switch_groupto_switch_responsebetter describes the method's purpose as a response handler.
92-92: LGTM! Maintains backward compatibility.The
switchproperty correctly returns a boolean value by accessingself._switch.state, maintaining the expected interface while using the newSwitchGroupstructure internally.
96-108: LGTM! Updated callback signature and usage.The method signature and call to
_switch_state_updatecorrectly incorporate the newswitch_groupparameter, properly integrating the enhanced response data.
110-126: Enhanced state update method with missing test coverage.The refactored
_switch_state_updatemethod correctly updates all attributes of theSwitchGroupobject and publishes the complete object to subscribers, which is an improvement over the previous implementation.However, static analysis indicates that lines 119-121 and 123 are not covered by tests. Consider adding test coverage for this critical functionality.
145-145: LGTM! Return the complete SwitchGroup object.Returning the entire
_switchobject forNodeFeature.SWITCHprovides more comprehensive state information to callers, which is beneficial for the new unified data structure approach.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CHANGELOG.md (1)
3-5: Strip trailing whitespace to silence lintersLanguageTool flagged spacing on these freshly-added lines. There are two superfluous spaces at EOL that do not influence rendering but will keep popping up in CI lint runs.
-## v0.44.7 - 2025-07-08␠␠ +## v0.44.7 - 2025-07-08 … -- PR [282](https://github.com/plugwise/python-plugwise-usb/pull/282): Finalize switch implementation␠␠ +- PR [282](https://github.com/plugwise/python-plugwise-usb/pull/282): Finalize switch implementation
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.md(1 hunks)plugwise_usb/api.py(1 hunks)pyproject.toml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- plugwise_usb/api.py
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[grammar] ~3-~3: Use proper spacing conventions.
Context: # Changelog ## v0.44.7 - 2025-07-08 - PR [282](https://github.com/plugwise/pyt...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~5-~5: Use proper spacing conventions.
Context: ...ull/282): Finalize switch implementation ## v0.44.6 - 2025-07-06 - PR [279](https:/...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Check commit



Summary by CodeRabbit
New Features
Refactor