Optimize 3 #839
Optimize 3 #839
Conversation
|
Warning Rate limit exceeded@bouwew has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 39 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
WalkthroughThe PR replaces many direct XML attribute accesses ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
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 #839 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 21 21
Lines 3435 3433 -2
=========================================
- Hits 3435 3433 -2 ☔ 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: 3
🧹 Nitpick comments (2)
plugwise/legacy/helper.py (1)
414-414: Consider adding a None check for rule_id.While the
.get("id")change is safer than direct attribute access, therule_idis used in string formatting on line 424 and comparisons. IfNone, this could lead to unexpected behavior. Consider adding a None check similar to the pattern used inplugwise/legacy/smile.pyline 196.🔎 Suggested defensive check
if (result := search.find("./rule[name='Thermostat schedule']")) is not None: name = "Thermostat schedule" rule_id = result.get("id") + if rule_id is None: + return available, selectedplugwise/smile.py (1)
428-432: Consider adding a None check for switch_id.The
switch_idobtained from.get("id")is used in URI construction on line 433. While line 431 appears to always execute for single-item actuators, ifitem.get("id")returnsNone, the URI would be malformed. Consider adding a None check before line 433.🔎 Suggested defensive check
else: # actuators with a single item like relay_functionality switch_id = item.get("id") + if switch_id is None: + return current_state # or raise appropriate error uri = f"{APPLIANCES};id={appl_id}/{switch.device};id={switch_id}"
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
plugwise/common.py(2 hunks)plugwise/helper.py(11 hunks)plugwise/legacy/helper.py(5 hunks)plugwise/legacy/smile.py(4 hunks)plugwise/smile.py(5 hunks)plugwise/util.py(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-12-22T09:37:24.648Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 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.
Applied to files:
plugwise/helper.pyplugwise/legacy/helper.py
📚 Learning: 2025-10-11T14:05:29.022Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 797
File: fixtures/adam_anna_new_2/data.json:1-304
Timestamp: 2025-10-11T14:05:29.022Z
Learning: In Plugwise fixture data (data.json files), location IDs referenced in device entries don't always need corresponding top-level location objects. Only climate zones (ThermoZone entries with dev_class "climate") require full top-level definitions with thermostats, schedules, and presets. Simple organizational locations that just group devices (like single plugs) can be referenced without top-level entries.
Applied to files:
plugwise/helper.py
📚 Learning: 2025-12-21T10:56:14.461Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 838
File: plugwise/helper.py:210-211
Timestamp: 2025-12-21T10:56:14.461Z
Learning: In plugwise/helper.py, when parsing location XML elements, the `type` element is always present for a location, so null checks are not required when accessing `location.find("type").text`.
Applied to files:
plugwise/legacy/helper.py
🧬 Code graph analysis (2)
plugwise/smile.py (1)
plugwise/common.py (1)
_get_groups(191-219)
plugwise/legacy/smile.py (1)
plugwise/common.py (1)
_get_groups(191-219)
🔇 Additional comments (18)
plugwise/legacy/smile.py (3)
85-85: LGTM! Correctly aligns with the new_get_groups()signature.The change to call
_get_groups()without using its return value is consistent with the refactoring inplugwise/common.py, where_get_groups()now returnsNoneand populatesself.gw_entitiesdirectly.
193-199: Good defensive programming with None check.The code properly checks if
schedule_rule_idisNonebefore using it, which handles the case where the rule element might not have an "id" attribute. This is the correct pattern when using.get("id").
206-206: Verify that template elements always have an "id" attribute.Similar to line 161, if the template element doesn't have an "id" attribute,
template_idwill beNoneand the XML construction on line 212 will includeid='None'. Consider adding a None check or ensuring the attribute is always present.plugwise/legacy/helper.py (3)
151-162: Verify that location elements always have an "id" attribute.The
loc_idfrom.get("id")is used as a dictionary key inself._loc_dataon line 162 and asself._home_loc_idon line 160. IfNone, this could cause unexpected behavior in location lookups and home location identification.
435-435: Verify that appliance elements always have an "id" attribute.The
appliance_idfrom.get("id")is used in the URI construction on line 436. IfNone, the URI would be malformed as{APPLIANCES};id=None/thermostat.
110-110: Add validation to ensure appliance elements have the "id" attribute before using as entity_id.While all test fixtures consistently include id attributes on appliances, the code should not silently accept
Noneforentity_id. Since line 110 usesappliance.get("id")which returnsNoneif missing, andentity_idis subsequently used as a dictionary key inself._create_gw_entities()on line 137 and compared againstself.heater_idon line 134, missing or None values would cause issues. Add explicit validation rather than relying on defensive.get()alone.plugwise/helper.py (6)
120-122: Verify that appliance elements always have an "id" attribute.The
entity_idis a critical identifier used throughout the entity lifecycle. If.get("id")returnsNone, it would cause issues when:
- Creating gateway entities
- Using as dictionary keys in
self.gw_entities- Performing entity lookups
272-272: Verify that gateway appliance elements always have an "id" attribute.The
self._gateway_idis a fundamental identifier for the gateway entity. IfNone, it would break gateway identification and various gateway-related operations throughout the codebase.
393-409: Good parameter rename for clarity.Renaming the parameter from
entity_idtogroup_idon line 396 improves code readability by making it clear that this function operates on group entities specifically.
505-508: Verify that notification elements always have an "id" attribute.If
.get("id")returnsNone, it will be used as a dictionary key inself._notificationson line 508. While this won't crash, it could lead to confusing notification tracking.
1005-1005: Verify that thermostat_functionality elements always have an "id" attribute.The
thermostat_functionality_idfrom.get("id")is used in URI construction on line 1007. IfNone, the resulting URI would be malformed:{LOCATIONS};id={loc_id}/thermostat;id=None.
906-909: The review comment describes code that does not match the actual repository.The actual code at lines 906-909 uses
directive.attrib["preset"], notdirective.get("preset"). The concern aboutNonebeing used as a dictionary key does not apply to the current implementation, as.attrib["preset"]will raise aKeyErrorif the attribute is missing, providing fail-fast behavior rather than silent dictionary key pollution.Likely an incorrect or invalid review comment.
plugwise/common.py (2)
266-266: Verify that service elements always have an "id" attribute.The
link_idfrom.get("id")is used in XPath construction on line 267 for module lookup. IfNone, the XPath would be malformed and the module lookup would fail.
191-219: Consider adding explicit validation for group id attributes.The concern about missing "id" attributes is theoretically valid. While all examined fixtures (40+ files) consistently include id attributes on group and appliance elements, the code lacks explicit safeguards. On line 208,
item.get("id") in self.gw_entitiesimplicitly preventsNonemembers from being added sinceNonewon't exist as a key. However, on line 202,group_id = group.get("id")could returnNoneand be used as a dictionary key on line 212 if a group lacked an id attribute.To make this more robust, consider explicitly validating that both
group_idand appliance ids are notNonebefore processing, rather than relying on implicit safeguards.plugwise/smile.py (4)
116-116: LGTM! Correctly aligns with the refactored_get_groups()method.The change to call
_get_groups()without capturing or using its return value is consistent with the method signature change inplugwise/common.py, where the method now returnsNoneand populatesself.gw_entitiesdirectly.
179-182: Verify that thermostat_functionality elements always have an "id" attribute.The
thermostat_idfrom.get("id")is checked forNoneon line 181 (good defensive programming!), but verify that under normal circumstances, thermostat_functionality elements should always have an "id" attribute. If they don't, the error message on line 182 will be raised, which might not accurately reflect the root cause.
359-360: Verify that template elements always have an "id" attribute.The
template_idfrom.get("id")is used directly in XML construction on line 360. IfNone, the template XML would containid="None", which might not be the intended behavior for legacy Anna devices.
457-457: Verify that functionality elements always have an "id" attribute.The
switch_idfrom.get("id")is used in URI construction on line 458. IfNone, the URI would be malformed. Consider adding a None check or verifying that the XML structure guarantees the presence of this attribute.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
plugwise/legacy/smile.py (1)
161-163: Critical: Still crashes with AttributeError if find() returns None.If
find(locator)returnsNone, calling.get("id")will raiseAttributeErrorbefore the None check executes. The walrus operator assigns the result of.get("id"), but the method call happens on the potentially-None return value. Follow the pattern fromset_schedule_state()(lines 192-196): validatefind()returns an element before accessing attributes.🔎 Proposed fix
- locator = f'rule/directives/when/then[@icon="{preset}"].../.../...' - if (rule_id := self._domain_objects.find(locator).get("id")) is None: - raise PlugwiseError("Plugwise: no preset id found.") # pragma: no cover + locator = f'rule/directives/when/then[@icon="{preset}"].../.../...' + if (elem := self._domain_objects.find(locator)) is None or (rule_id := elem.get("id")) is None: + raise PlugwiseError("Plugwise: no preset id found.") # pragma: no cover
🧹 Nitpick comments (1)
plugwise/util.py (1)
88-89: Optional: Redundant None-check for element from findall().
findall()never yieldsNoneelements, so the check at lines 88-89 is redundant. You can safely remove this defensive guard.🔎 Proposed simplification
for heater_central in xml.findall(locator): - if not heater_central: - continue # pragma: no cover - heater_central_count += 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
plugwise/legacy/helper.py(5 hunks)plugwise/legacy/smile.py(4 hunks)plugwise/util.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- plugwise/legacy/helper.py
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-06T16:03:27.250Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 630
File: tests/data/adam/adam_g_o.json:0-0
Timestamp: 2024-10-06T16:03:27.250Z
Learning: The minimal information for the `heater_central` device is due to a known bug still present in the code.
Applied to files:
plugwise/util.py
📚 Learning: 2025-12-21T10:56:14.461Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 838
File: plugwise/helper.py:210-211
Timestamp: 2025-12-21T10:56:14.461Z
Learning: In plugwise/helper.py, when parsing location XML elements, the `type` element is always present for a location, so null checks are not required when accessing `location.find("type").text`.
Applied to files:
plugwise/util.py
🧬 Code graph analysis (2)
plugwise/legacy/smile.py (4)
plugwise/common.py (1)
_get_groups(191-219)plugwise/legacy/helper.py (1)
_presets(392-407)plugwise/helper.py (1)
_presets(893-911)plugwise/exceptions.py (1)
PlugwiseError(24-25)
plugwise/util.py (2)
plugwise/common.py (1)
heater_id(63-65)plugwise/__init__.py (1)
heater_id(101-103)
🔇 Additional comments (5)
plugwise/legacy/smile.py (2)
85-85: LGTM! Aligns with _get_groups() refactor.The removal of the conditional merge correctly reflects that
_get_groups()now populatesself.gw_entitiesdirectly rather than returning a dictionary.
192-196: LGTM! Safe attribute access with proper validation.The change from
.attrib["id"]to.get("id")with subsequent None-checking (line 198) correctly handles missing id attributes.plugwise/util.py (3)
92-93: LGTM! Robust None-handling for missing id attribute.The guard correctly prevents
Nonefrom being used as a dictionary key (addressed the concern from previous review). By continuing when id isNone, the code ensures only valid IDs reach line 101.
106-112: LGTM! Sound selection logic for heater_id.The logic correctly defaults to the first valid heater_central and upgrades to one with actuators when multiple candidates exist. The control flow ensures
heater_idis always a valid string when returned.
100-101: The filtering behavior is correct and aligns with the PR objective.The code intentionally restricts heater_central collection to devices named "Central heating boiler" to exclude orphaned or misconfigured appliances (such as SmartPlug Floor devices) that are incorrectly marked as heater_central type. This filtering addresses Pw-Beta Issue #739 regarding Circle/Stealth device handling and is consistent with the known heater_central issues documented in the codebase.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
plugwise/legacy/helper.py (2)
418-418: Consider the impact ofNonein the search path.If
result.get("id")returnsNone, line 428's f-string becomes'./rule[@id="None"]...', which would fail to match any rule. While this may be acceptable (sincedirectiveswould be False and the function returns appropriate defaults), explicitly checking for None and keepingrule_id = NONEwould make the intent clearer.🔎 Suggested refinement
- rule_id = result.get("id") + if (rule_id_val := result.get("id")) is not None: + rule_id = rule_id_val
439-439: Add defensive checks for thermostat appliance lookup.The function assumes
.find(locator)returns a valid element and that it has anidattribute. If either assumption fails, you'd get AttributeError or construct an invalid URI. Consider adding explicit checks or documenting that this method requires valid thermostat XML structure.🔎 Proposed defensive pattern
- appliance_id = self._appliances.find(locator).get("id") - return f"{APPLIANCES};id={appliance_id}/thermostat" + if (thermostat := self._appliances.find(locator)) is None: + return "" # or raise appropriate error + if (appliance_id := thermostat.get("id")) is None: + return "" # or raise appropriate error + return f"{APPLIANCES};id={appliance_id}/thermostat"
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
plugwise/legacy/helper.py(5 hunks)plugwise/legacy/smile.py(4 hunks)plugwise/util.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- plugwise/legacy/smile.py
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-10-06T16:03:27.250Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 630
File: tests/data/adam/adam_g_o.json:0-0
Timestamp: 2024-10-06T16:03:27.250Z
Learning: The minimal information for the `heater_central` device is due to a known bug still present in the code.
Applied to files:
plugwise/util.py
📚 Learning: 2024-12-22T09:37:24.648Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 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.
Applied to files:
plugwise/util.pyplugwise/legacy/helper.py
📚 Learning: 2025-12-21T10:56:14.461Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 838
File: plugwise/helper.py:210-211
Timestamp: 2025-12-21T10:56:14.461Z
Learning: In plugwise/helper.py, when parsing location XML elements, the `type` element is always present for a location, so null checks are not required when accessing `location.find("type").text`.
Applied to files:
plugwise/util.pyplugwise/legacy/helper.py
🧬 Code graph analysis (1)
plugwise/util.py (2)
plugwise/common.py (1)
heater_id(63-65)plugwise/__init__.py (1)
heater_id(101-103)
🪛 GitHub Check: Run pytest using Python 3.13
plugwise/util.py
[warning] 88-88:
Testing an element's truth value will always return True in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
[warning] 88-88:
Testing an element's truth value will always return True in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
[warning] 88-88:
Testing an element's truth value will always return True in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
[warning] 88-88:
Testing an element's truth value will always return True in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
[warning] 88-88:
Testing an element's truth value will always return True in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
[warning] 88-88:
Testing an element's truth value will always return True in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
[warning] 88-88:
Testing an element's truth value will always return True in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
[warning] 88-88:
Testing an element's truth value will always return True in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
[warning] 88-88:
Testing an element's truth value will always return True in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
[warning] 88-88:
Testing an element's truth value will always return True in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
🪛 GitHub Check: Run pytest using Python 3.14
plugwise/util.py
[warning] 88-88:
Testing an element's truth value will always return True in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
[warning] 88-88:
Testing an element's truth value will always return True in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
[warning] 88-88:
Testing an element's truth value will always return True in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
[warning] 88-88:
Testing an element's truth value will always return True in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
[warning] 88-88:
Testing an element's truth value will always return True in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
[warning] 88-88:
Testing an element's truth value will always return True in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
[warning] 88-88:
Testing an element's truth value will always return True in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
[warning] 88-88:
Testing an element's truth value will always return True in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
[warning] 88-88:
Testing an element's truth value will always return True in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
[warning] 88-88:
Testing an element's truth value will always return True in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
⏰ 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: Process test coverage
🔇 Additional comments (5)
plugwise/legacy/helper.py (3)
396-405: Good defensive validation of directive attributes.The enhanced checks ensure both
iconandtemperatureattributes are present before constructing preset entries, avoiding potential TypeError fromfloat(None). The migration to.get()improves robustness.
151-151: No action needed. The code at line 151 correctly useslocation.get("id"), which is already the defensive approach. Test fixtures confirm all location elements have id attributes, and the XML schema guarantees this requirement.
110-110: The change to.get("id")improves defensive coding, and test fixtures confirm appliances always have id attributes.The shift from
appliance.attrib["id"]toappliance.get("id")prevents KeyError, which is good practice. However, examination of XML test fixtures (legacy_anna, legacy_anna_2, etc.) shows that all appliance elements consistently include theidattribute, making the None risk minimal in practice. That said, explicit None-checking after line 110 would strengthen robustness if malformed XML ever occurs.plugwise/util.py (2)
91-101: Robust None-checking and filtering logic.The explicit checks for
heater_central_idandheater_central_namebeingNoneprevent potential errors and make the code more defensive. The filtering by name "Central heating boiler" correctly implements the requirement from Pw-Beta Issue #739.
103-114: Clear selection logic for multiple heater_central devices.The refactored logic correctly handles the Core Issue #104433 scenario by selecting the first heater_central with actuators when multiple devices exist. The fallback to the first entry is sensible and the empty-list guard prevents errors.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
plugwise/legacy/smile.py (1)
210-210: Critical: Missing None-check before calling.get()onfind()result.If
find(locator)returnsNone, calling.get("id")will raiseAttributeError. This issue was previously flagged and marked as addressed, but the problematic code is still present.🔎 Proposed fix
locator = f'.//*[@id="{schedule_rule_id}"]/template' - template_id = self._domain_objects.find(locator).get("id") + if (template_elem := self._domain_objects.find(locator)) is None or (template_id := template_elem.get("id")) is None: + raise PlugwiseError( + "Plugwise: template not found for schedule." + ) # pragma: no cover
🧹 Nitpick comments (2)
plugwise/util.py (2)
94-99: Consider additional guard for Element.text being None.While the check at lines 94-95 ensures the
nameelement exists,Element.textcan theoretically beNone. If robustness is a priority, consider adding:if (heater_central_name := heater_central.find("name")) is None or heater_central_name.text is None: continue # pragma: no coverHowever, in this domain context,
nameelements should always have text content, so this is optional defensive coding.
86-114: Simplify data structure for better readability.The current implementation uses a list of single-key dictionaries (
list[dict[str, bool]]), which makes extraction at lines 106 and 109 unnecessarily complex:
- Line 106:
list(heater_central_list[0].keys())[0]- Line 109:
next(iter(item.items()))Consider using a simpler structure such as a list of tuples:
🔎 Proposed refactor using tuples
heater_central_count = 0 - heater_central_list: list[dict[str, bool]] = [] + heater_central_list: list[tuple[str, bool]] = [] if not (result := xml.findall(locator)): - return # pragma: no cover + return NONE # pragma: no cover for heater_central in result: if (heater_central_id := heater_central.get("id")) is None: continue # pragma: no cover if (heater_central_name := heater_central.find("name")) is None: continue # pragma: no cover has_actuators = heater_central.find("actuator_functionalities/") is not None # Filter for Plug/Circle/Stealth heater_central -- Pw-Beta Issue #739 if heater_central_name.text == "Central heating boiler": - heater_central_list.append({heater_central_id: has_actuators}) + heater_central_list.append((heater_central_id, has_actuators)) heater_central_count += 1 if not heater_central_list: return NONE # pragma: no cover - heater_id = list(heater_central_list[0].keys())[0] + heater_id = heater_central_list[0][0] if heater_central_count > 1: for item in heater_central_list: - heater_central_id, has_actuators = next(iter(item.items())) + heater_central_id, has_actuators = item if has_actuators: heater_id = heater_central_id break return heater_id
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
plugwise/legacy/helper.py(5 hunks)plugwise/legacy/smile.py(4 hunks)plugwise/util.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- plugwise/legacy/helper.py
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-10-06T16:03:27.250Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 630
File: tests/data/adam/adam_g_o.json:0-0
Timestamp: 2024-10-06T16:03:27.250Z
Learning: The minimal information for the `heater_central` device is due to a known bug still present in the code.
Applied to files:
plugwise/util.py
📚 Learning: 2024-12-22T09:37:24.648Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 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.
Applied to files:
plugwise/util.py
📚 Learning: 2025-12-21T10:56:14.461Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 838
File: plugwise/helper.py:210-211
Timestamp: 2025-12-21T10:56:14.461Z
Learning: In plugwise/helper.py, when parsing location XML elements, the `type` element is always present for a location, so null checks are not required when accessing `location.find("type").text`.
Applied to files:
plugwise/util.py
🧬 Code graph analysis (2)
plugwise/util.py (2)
plugwise/common.py (1)
heater_id(63-65)plugwise/__init__.py (1)
heater_id(101-103)
plugwise/legacy/smile.py (4)
plugwise/common.py (1)
_get_groups(191-219)plugwise/legacy/helper.py (1)
_presets(392-407)plugwise/helper.py (1)
_presets(893-911)plugwise/exceptions.py (1)
PlugwiseError(24-25)
🪛 GitHub Actions: Latest commit
plugwise/util.py
[error] 88-88: mypy: Return value expected [return-value]. Found 1 error in 1 file (checked 12 source files).
🔇 Additional comments (1)
plugwise/legacy/smile.py (1)
85-85: LGTM: Aligned with refactored_get_groups().The direct call without capturing a return value is correct. According to the relevant code from plugwise/common.py,
_get_groups()now returnsNoneand populatesself.gw_entitiesin-place.
|
Changes:
.attrib[]by.get()_get_groups()function_collect_group_sensors(): correct togroup_idSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.