Repository navigation
Add v1.14.2 changes to main branch - #905
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #905 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 21 21
Lines 3502 3502
=========================================
Hits 3502 3502 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugwise/smile.py (1)
354-407: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAlign integer schedule-state typing and runtime behavior across all layers.
set_schedule_state()acceptsint | str | None, but rejects numeric inputs before normalizing state, while the wrapper keeps integer states in the typing contract. Keep narrowed string-only states/types, or normalize0/1(or accepted numeric values) to canonical"on"/"off"before validation, context construction, and storage/delivery to both current and legacy paths.🤖 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 `@plugwise/smile.py` around lines 354 - 407, Align schedule-state handling across plugwise/smile.py:354-407, plugwise/smile.py:239-239, plugwise/__init__.py:356-379, and plugwise/legacy/smile.py:176-199. In set_schedule_state(), either narrow the accepted type to strings everywhere or normalize supported integer values such as 0/1 to canonical STATE_OFF/STATE_ON before validation, determine_contexts(), request delivery, and _schedule_old_states storage; update the wrapper and both current and legacy paths consistently, with no direct changes needed at sites already compatible with the chosen contract.
🤖 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 `@plugwise/smile.py`:
- Around line 260-274: Update the DHW setter validation in the relevant smile.py
method to always reject unsupported mode values when _dhw_allowed_modes is
available, while requiring length to be one of the device-supported integer
values. Align the public wrapper in plugwise/__init__.py with this contract by
accepting only the supported length representations and forwarding them
consistently.
---
Outside diff comments:
In `@plugwise/smile.py`:
- Around line 354-407: Align schedule-state handling across
plugwise/smile.py:354-407, plugwise/smile.py:239-239,
plugwise/__init__.py:356-379, and plugwise/legacy/smile.py:176-199. In
set_schedule_state(), either narrow the accepted type to strings everywhere or
normalize supported integer values such as 0/1 to canonical STATE_OFF/STATE_ON
before validation, determine_contexts(), request delivery, and
_schedule_old_states storage; update the wrapper and both current and legacy
paths consistently, with no direct changes needed at sites already compatible
with the chosen contract.
🪄 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 Plus
Run ID: 6aaa98cf-e007-4e06-a95c-92d9eefb34ae
📒 Files selected for processing (5)
CHANGELOG.mdplugwise/__init__.pyplugwise/legacy/smile.pyplugwise/smile.pypyproject.toml
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)plugwise/(smile\.py|__init__\.py)$' || true
echo "== relevant smile.py section =="
sed -n '220,290p' plugwise/smile.py
echo "== relevant __init__.py section =="
sed -n '430,490p' plugwise/__init__.py
echo "== dhw_allowed_modes and dhw method usages =="
rg -n "_dhw_allowed_modes|dhw_allowed_modes|set_dhw_mode|dh[ _]modes|length" plugwise -SRepository: plugwise/python-plugwise
Length of output: 7019
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== helper.py dhw mode sections =="
sed -n '70,100p' plugwise/helper.py
sed -n '250,290p' plugwise/helper.py
sed -n '355,375p' plugwise/helper.py
sed -n '480,520p' plugwise/helper.py
echo "== tests mentioning dhw mode/length =="
git ls-files | rg -i 'test.*dhw|dhw.*test|smile|plugwise' | sed -n '1,120p'
rg -n "dhw_mode|dhw_modes|select_dhw_mode|length.*dhw|domestic_hot_water|domestic_hot_water_mode_control_functionality" . -S --glob '!*.pyc'Repository: plugwise/python-plugwise
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== __init__.py docstring around set_dhw_mode with duration mention =="
rg -n -A 8 -B 8 "Set the domestic hot water heating regulation mode|duration|int| str | None = None" plugwise/__init__.py
echo "== legacy set_dhw_mode for comparison =="
sed -n '120,170p' plugwise/legacy/smile.py
echo "== behavioral model of current validation for missing allowed modes and string/integer lengths =="
python3 - <<'PY'
def current_validation(dhw_allowed_modes, mode, length):
return (
dhw_allowed_modes
and mode not in dhw_allowed_modes
or length is None
or not isinstance(length, int)
)
api_path_allowed = True
api_textual_accepted = False
cases = [
("no dhw metadata / mode='comfort' / length=2", None, "comfort", 2),
("no dhw metadata / mode='invalid' / length=2", None, "invalid", 2),
("no dhw metadata / mode='comfort' / length='short'", None, "comfort", "short"),
("lora modes / mode='invalid' / length=2", ["auto", "eco"], "invalid", 2),
("lora modes / mode='auto' / length='short'", ["auto", "eco"], "auto", "short"),
("supported length not two routes differently / length=3", ["comfort", "eco"], "comfort", 3),
]
for name, dhw_allowed_modes, mode, length in cases:
print(f"{name}: core_rejected={current_validation(dhw_allowed_modes, mode, length)}")
PYRepository: plugwise/python-plugwise
Length of output: 10980
Ensure DHW mode validation reaches the device layer.
plugwise/__init__.py allows textual length values, while plugwise/smile.py only rejects text and passes any integer through; when _dhw_allowed_modes is unavailable, invalid mode values bypass rejection on the length != 2 path. Require a supported mode when explicit modes are known and align the public wrapper’s length contract with the accepted values.
📍 Affects 2 files
plugwise/smile.py#L260-L274(this comment)plugwise/__init__.py#L464-L473
🤖 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 `@plugwise/smile.py` around lines 260 - 274, Update the DHW setter validation
in the relevant smile.py method to always reject unsupported mode values when
_dhw_allowed_modes is available, while requiring length to be one of the
device-supported integer values. Align the public wrapper in
plugwise/__init__.py with this contract by accepting only the supported length
representations and forwarding them consistently.
There was a problem hiding this comment.
@bouwew, acknowledged. I’ll leave this finding unresolved for now. Would you like me to open a follow-up issue for the DHW validation and wrapper-contract alignment?
🐇💭




Summary by CodeRabbit