Reduce complexity #765
Reduce complexity #765
Conversation
WalkthroughThe changes remove the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SmileLegacyHelper
participant Appliances
User->>SmileLegacyHelper: Call _all_appliances()
SmileLegacyHelper->>Appliances: Create gateway entities
SmileLegacyHelper->>SmileLegacyHelper: Call _reorder_devices()
SmileLegacyHelper-->>User: Return reordered gw_entities
sequenceDiagram
participant set_switch_state
participant model_to_switch_items
participant switch
set_switch_state->>model_to_switch_items: Pass model, state, switch
model_to_switch_items-->>set_switch_state: Return (state, switch)
set_switch_state->>switch: Construct XML payload with updated values
Possibly related PRs
Suggested labels
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Pylint (3.3.7)plugwise/constants.pyplugwise/helper.pyplugwise/common.py
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learningsplugwise/helper.py (1)🧬 Code Graph Analysis (1)plugwise/helper.py (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (3)
✨ 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 ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #765 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 21 21
Lines 3388 3382 -6
=========================================
- Hits 3388 3382 -6 ☔ 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: 0
🧹 Nitpick comments (1)
plugwise/legacy/helper.py (1)
141-151: Consider simplifying the dictionary reordering logic.The method correctly implements device reordering, but the implementation could be more concise. The current approach works but is somewhat verbose.
Consider this more direct approach:
def _reorder_devices(self) -> None: """Place the gateway and optional heater_central devices as 1st and 2nd.""" + reordered = {} for dev_class in PRIORITY_DEVICE_CLASSES: for entity_id, entity in dict(self.gw_entities).items(): if entity["dev_class"] == dev_class: - tmp_entity = entity - self.gw_entities.pop(entity_id) - cleared_dict = self.gw_entities - add_to_front = {entity_id: tmp_entity} - self.gw_entities = {**add_to_front, **cleared_dict} + reordered[entity_id] = self.gw_entities.pop(entity_id) break + self.gw_entities = {**reordered, **self.gw_entities}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugwise/legacy/helper.py(1 hunks)plugwise/smile.py(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: bouwew
PR: plugwise/python-plugwise#678
File: plugwise/legacy/helper.py:69-69
Timestamp: 2024-12-22T09:37:24.648Z
Learning: The code in plugwise/legacy/helper.py is fully separated from plugwise/helper.py. The user indicates that the problem signaled about the mismatch of the variable name (_home_location vs _home_loc_id) does not actually apply.
plugwise/legacy/helper.py (1)
Learnt from: bouwew
PR: plugwise/python-plugwise#678
File: plugwise/legacy/helper.py:69-69
Timestamp: 2024-12-22T09:37:24.648Z
Learning: The code in plugwise/legacy/helper.py is fully separated from plugwise/helper.py. The user indicates that the problem signaled about the mismatch of the variable name (_home_location vs _home_loc_id) does not actually apply.
plugwise/smile.py (1)
Learnt from: bouwew
PR: plugwise/python-plugwise#678
File: plugwise/legacy/helper.py:69-69
Timestamp: 2024-12-22T09:37:24.648Z
Learning: The code in plugwise/legacy/helper.py is fully separated from plugwise/helper.py. The user indicates that the problem signaled about the mismatch of the variable name (_home_location vs _home_loc_id) does not actually apply.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run pytest using Python 3.13
🔇 Additional comments (3)
plugwise/legacy/helper.py (1)
139-139: Good refactoring: extracted device ordering logic.The call to
_reorder_devices()successfully separates the device ordering concern from the main appliance processing logic, improving code organization.plugwise/smile.py (2)
40-58: Excellent refactoring: centralized model-specific switch logic.The
model_to_switch_items()function successfully centralizes model-specific switch attribute and state transformations. The use of a match statement provides clear, readable handling of different models, and the function signature is well-designed.
405-405: Clean integration of the helper function.The replacement of inline conditional logic with a call to
model_to_switch_items()successfully reduces complexity in the main method while maintaining the same functionality.
CoMPaTech
left a comment
CoMPaTech
left a comment
There was a problem hiding this comment.
Nice working improving the quality with all those PRs
|
Summary by CodeRabbit