Skip to content

Typing 1 b #870

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.

Sign up for GitHub

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

Draft
bouwew wants to merge 29 commits into
base: main
Choose a base branch
from
Draft

Typing 1 b #870

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
eca817e
Initial typed xml reading attempt (incomplete)
CoMPaTech Dec 28, 2025
408682b
Further progress
CoMPaTech Dec 29, 2025
6c6b3f2
Further progress - appliance typos
CoMPaTech Dec 29, 2025
709e84c
Some progress
CoMPaTech Dec 29, 2025
660acd8
More progress
CoMPaTech Dec 29, 2025
826c387
Further cleaning
CoMPaTech Dec 29, 2025
baf3419
Gateway progress
CoMPaTech Dec 31, 2025
9cd1b54
Fix after rebase
bouwew Jan 24, 2026
91b6877
Contents -> content
bouwew Jan 24, 2026
e2ad185
Fix module_data
bouwew Jan 24, 2026
9c011df
Fix another contents
bouwew Jan 24, 2026
6d09ec6
Add xmltodict to requirements
bouwew Apr 5, 2026
04698e7
Change index from 5 to 4
bouwew Apr 5, 2026
7e393f5
Update _appl_heater_central_info()
bouwew Apr 6, 2026
1d36e07
Update check_heater_central()
bouwew Apr 7, 2026
7c84129
And use
bouwew Apr 7, 2026
39ab2b3
Fixes
bouwew Apr 7, 2026
d7fa9f7
More _appl_heater_central_info() updating
bouwew Apr 13, 2026
85bfdda
Fix contents
bouwew Apr 13, 2026
613cbb3
Update _appl_thermostat_info()
bouwew Apr 13, 2026
aeee690
Update _get_appliances_with_offset_functionality()
bouwew Apr 13, 2026
8e3c64c
get_groups()
bouwew Apr 13, 2026
a4a8822
Disable prints, add logger
bouwew Apr 17, 2026
6728175
Disable initial Adam testcase
bouwew Apr 17, 2026
bbe0e9d
Add _create_gw_entities() call
bouwew Apr 17, 2026
a998ae0
Extend Appliance class
bouwew Apr 18, 2026
51f9db4
Correct to appl.id
bouwew Apr 18, 2026
17e6798
Various improvements
bouwew Apr 18, 2026
e9d0d42
fixup: typing_1_b Python code fixed using ruff
Apr 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
- Remove biome (as prettier was reinstated)
- Replace node-based markdownlint with pythonic library

- Attempt to ditch untyped Munch for the existing TypedDicts by leveraging pydantic to type xmltodict XML conversion
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Move this bullet back under ## Ongoing.

At its current position Line 17 renders as part of v1.11.3, not the ongoing section.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@CHANGELOG.md` at line 17, The changelog bullet "Attempt to ditch untyped
Munch for the existing TypedDicts by leveraging pydantic to type xmltodict XML
conversion" is currently under the v1.11.3 section and should be moved back to
the ongoing section; edit CHANGELOG.md to cut that bullet from its current
location (line containing that exact text) and paste it under the "## Ongoing"
heading so it renders as part of Ongoing instead of v1.11.3.

## v1.11.2

- Add/update model-data for Jip, Tom and Floor via PR [#842](https://github.com/plugwise/python-plugwise/pull/842)
Expand Down
135 changes: 70 additions & 65 deletions plugwise/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
from munch import Munch
from packaging.version import Version, parse

from .model import GatewayData, PlugwiseData


class Smile(SmileComm):
"""The main Plugwise Smile API class."""
Expand Down Expand Up @@ -74,18 +76,8 @@
self._smile_api: SmileAPI | SmileLegacyAPI
self._stretch_v2 = False
self._target_smile: str = NONE
self.smile: Munch = Munch()
self.smile.anna_p1 = False
self.smile.hostname = NONE
self.smile.hw_version = None
self.smile.legacy = False
self.smile.mac_address = None
self.smile.model = NONE
self.smile.model_id = None
self.smile.name = NONE
self.smile.type = NONE
self.smile.version = Version("0.0.0")
self.smile.zigbee_mac_address = None
self.data: PlugwiseData
self.smile = GatewayData(hostname="smile")

@property
def cooling_present(self) -> bool:
Expand Down Expand Up @@ -117,23 +109,19 @@

async def connect(self) -> Version:
"""Connect to the Plugwise Gateway and determine its name, type, version, and other data."""
result = await self._request(DOMAIN_OBJECTS)
# Work-around for Stretch fw 2.7.18
if not (vendor_names := result.findall("./module/vendor_name")):
result = await self._request(MODULES)
vendor_names = result.findall("./module/vendor_name")

names: list[str] = []
for name in vendor_names:
names.append(name.text)
await self._request(DOMAIN_OBJECTS, new=True)

vendor_models = result.findall("./module/vendor_model")
models: list[str] = []
for model in vendor_models:
models.append(model.text)

dsmrmain = result.find("./module/protocols/dsmrmain")
if "Plugwise" not in names and dsmrmain is None: # pragma: no cover
# Work-around for Stretch fw 2.7.18
dsmrmain: bool = False
vendor_names: list = []
vendor_models: list = []
for module in self.data.module:
vendor_names.append(module.vendor_name)
vendor_models.append(module.vendor_model)
if "dsmrmain" in module.protocols:
dsmrmain = True
Comment on lines +115 to +122
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm Module.protocols typing and any fixtures with protocols=None
ast-grep --pattern 'class Module($_):
  $$$'
rg -nP '"dsmrmain"' --type=py

Repository: plugwise/python-plugwise

Length of output: 1189

🏁 Script executed:

#!/bin/bash
# Read the full context around the problematic code
sed -n '105,130p' plugwise/__init__.py | cat -n

# Search for test fixtures or actual data with None values
grep -r "protocols.*None" --include="*.py" | head -20
grep -r "vendor_name.*None" --include="*.py" | head -20

Repository: plugwise/python-plugwise

Length of output: 2469

🏁 Script executed:

#!/bin/bash
# Find where Module objects are created/populated with data
grep -n "Module(" plugwise/*.py | head -20

# Check the XML parsing in common.py to see how protocols are set
sed -n '1,100p' plugwise/common.py | grep -A 5 -B 5 "protocols"

# Search for test data or fixtures with Module objects
find . -path ./node_modules -prune -o -name "*.json" -o -name "*.xml" | xargs grep -l "protocols" 2>/dev/null | head -5

Repository: plugwise/python-plugwise

Length of output: 1573

🏁 Script executed:

#!/bin/bash
# Check test data structure to see if protocols/vendor_name are optional
head -200 ./userdata/adam_jip/core.domain_objects.xml | grep -A 5 -B 5 "module" | head -50

# Also check if there's any module without protocols in test data
grep -c "<protocols" ./userdata/adam_jip/core.domain_objects.xml || echo "No protocols found"
</parameter>

Repository: plugwise/python-plugwise

Length of output: 1837

🏁 Script executed:

#!/bin/bash
# Count total modules and modules with protocols to see if any are missing
echo "Total modules in adam_jip:"
grep -c "<module " ./userdata/adam_jip/core.domain_objects.xml
echo "Modules with protocols:"
grep -c "<protocols" ./userdata/adam_jip/core.domain_objects.xml

# Check another test file
echo ""
echo "Total modules in adam_bad_thermostat:"
grep -c "<module " ./userdata/adam_bad_thermostat/core.domain_objects.xml
echo "Modules with protocols in adam_bad_thermostat:"
grep -c "<protocols" ./userdata/adam_bad_thermostat/core.domain_objects.xml

# Look at the actual code that parses modules to see what happens with missing protocols
sed -n '1,300p' plugwise/common.py | grep -A 10 "def.*module" | head -30

Repository: plugwise/python-plugwise

Length of output: 1110

Guard against module.protocols and vendor_name/vendor_model being None when iterating.

Module.protocols is typed dict[str, Protocols] | list[Protocols] | None and vendor_name/vendor_model are str | None. The loop unconditionally calls "dsmrmain" in module.protocols (raises TypeError when None) and appends None values into vendor_names/vendor_models, making downstream checks fragile. The codebase elsewhere explicitly sets these fields to None and guards against None protocols in helper functions, confirming this is a real issue.

🩹 Proposed fix 
-        for module in self.data.module:
-            vendor_names.append(module.vendor_name)
-            vendor_models.append(module.vendor_model)
-            if "dsmrmain" in module.protocols:
-                dsmrmain = True
+        for module in self.data.module:
+            if module.vendor_name:
+                vendor_names.append(module.vendor_name)
+            if module.vendor_model:
+                vendor_models.append(module.vendor_model)
+            if module.protocols and "dsmrmain" in module.protocols:
+                dsmrmain = True
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@plugwise/__init__.py` around lines 115 - 122, The loop over self.data.module
must guard against module.protocols and module.vendor_name/module.vendor_model
being None: only append module.vendor_name to vendor_names and
module.vendor_model to vendor_models when they are not None, and determine
dsmrmain only when module.protocols is present and supports membership;
implement this by checking if module.protocols is not None (or
isinstance(module.protocols, dict) and then "dsmrmain" in module.protocols, or
if it's a list check elements appropriately), or by wrapping the "dsmrmain"
membership test in a try/except TypeError to avoid calling "in" on None, and
update the variables vendor_names/vendor_models and dsmrmain accordingly.


if "Plugwise" not in vendor_names and dsmrmain is None: # pragma: no cover

Check failure on line 124 in plugwise/__init__.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this identity check; it will always be False. 


See more on https://sonarcloud.io/project/issues?id=plugwise_python-plugwise&issues=AZ1ejLn3rHfHDGLiEckA&open=AZ1ejLn3rHfHDGLiEckA&pullRequest=870
LOGGER.error(
"Connected but expected text not returned, we got %s. Please create"
" an issue on http://github.com/plugwise/python-plugwise",
Expand All @@ -142,14 +130,14 @@
raise ResponseError

# Check if Anna is connected to an Adam
if "159.2" in models:
if "159.2" in vendor_models:
LOGGER.error(
"Your Anna is connected to an Adam, make sure to only add the Adam as integration."
)
raise InvalidSetupError

# Determine smile specifics
await self._smile_detect(result, dsmrmain)
await self._smile_detect()

self._smile_api = (
SmileAPI(
Expand All @@ -162,6 +150,7 @@
self._request,
self._schedule_old_states,
self.smile,
self.data,
)
if not self.smile.legacy
else SmileLegacyAPI(
Expand All @@ -173,45 +162,57 @@
self._stretch_v2,
self._target_smile,
self.smile,
self.data,
)
)

# Update all endpoints on first connect
await self._smile_api.full_xml_update()

return cast(Version, self.smile.version)
return self.smile.firmware_version
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Return-type mismatch: connect() is annotated -> Version but returns str | None.

GatewayData.firmware_version is str | None, so self.smile.firmware_version cannot satisfy the declared Version return type. Either change the signature to -> str | None (and update callers) or parse before returning:

-        return self.smile.firmware_version
+        return parse(self.smile.firmware_version) if self.smile.firmware_version else Version("0.0.0")

Note also that self.smile.version in GatewayData.model_post_init stores a Version object into a str-annotated field (see plugwise/model.py line 457–462) — same annotation-vs-runtime issue at the other end.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@plugwise/__init__.py` at line 172, The connect() function currently annotated
as returning Version actually returns self.smile.firmware_version which is a str
| None (GatewayData.firmware_version); fix by either changing connect()'s return
annotation to -> str | None and update callers accordingly, or convert/parse
self.smile.firmware_version to a Version before returning (e.g., handle None and
construct a Version object or raise), and also reconcile
GatewayData.model_post_init where self.smile.version (runtime Version) is stored
into a str-annotated field—make the field/assignment types consistent (either
annotate firmware_version/version as Version or ensure stored value is a str).


async def _smile_detect(
self, result: etree.Element, dsmrmain: etree.Element
) -> None:
async def _smile_detect(self) -> None:
"""Helper-function for connect().

Detect which type of Plugwise Gateway is being connected.
"""
# print(f"HOI14 {self}")

Check warning on line 179 in plugwise/__init__.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this commented out code. 


See more on https://sonarcloud.io/project/issues?id=plugwise_python-plugwise&issues=AZ2lNrlLWySueEBauAgm&open=AZ2lNrlLWySueEBauAgm&pullRequest=870
# print(f"HOI14 {self.smile}")
model: str = "Unknown"
if (gateway := result.find("./gateway")) is not None:
self.smile.version = parse(gateway.find("firmware_version").text)
self.smile.hw_version = gateway.find("hardware_version").text
self.smile.hostname = gateway.find("hostname").text
self.smile.mac_address = gateway.find("mac_address").text
if (vendor_model := gateway.find("vendor_model")) is None:
if self.data.gateway is not None:
if self.data.gateway.vendor_model is None:
return # pragma: no cover

model = vendor_model.text
elec_measurement = gateway.find(
"gateway_environment/electricity_consumption_tariff_structure"
)
self.smile.firmware_version = self.data.gateway.firmware_version
self.smile.hardware_version = self.data.gateway.hardware_version
self.smile.hostname = self.data.gateway.hostname
self.smile.mac_address = self.data.gateway.mac_address

# print(f"HOI11a {self.data.gateway}")

Check warning on line 191 in plugwise/__init__.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this commented out code. 


See more on https://sonarcloud.io/project/issues?id=plugwise_python-plugwise&issues=AZ2lNrlLWySueEBauAgn&open=AZ2lNrlLWySueEBauAgn&pullRequest=870
# print(f"HOI11b {self.data.gateway.gateway_environment}")
# if (
# "electricity_consumption_tariff_structure"
# in self.data.gateway.gateway_environment
# ):
# print(
# f"HOI11c {self.data.gateway.gateway_environment.electricity_consumption_tariff_structure}"
# )
if (
elec_measurement is not None
and elec_measurement.text
and model == "smile_thermo"
"electricity_consumption_tariff_structure"
in self.data.gateway.gateway_environment
and self.data.gateway.gateway_environment.electricity_consumption_tariff_structure
and self.smile.vendor_model == "smile_thermo"
):
self.smile.anna_p1 = True
else:
model = await self._smile_detect_legacy(result, dsmrmain, model)
# TODO

Check warning on line 208 in plugwise/__init__.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Complete the task associated to this "TODO" comment. 


See more on https://sonarcloud.io/project/issues?id=plugwise_python-plugwise&issues=AZ1ejLn3rHfHDGLiEckC&open=AZ1ejLn3rHfHDGLiEckC&pullRequest=870
self.smile.vendor_model = await self._smile_detect_legacy(
result, dsmrmain, model
)
Comment on lines +208 to +211
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Undefined variables result and dsmrmain in legacy detection call.

The variables result and dsmrmain are passed to _smile_detect_legacy() but they are not defined in the scope of _smile_detect(). The dsmrmain variable exists only in connect(), and result was removed entirely during refactoring.

💡 Suggested approach

Either:

  1. Pass dsmrmain as a parameter to _smile_detect() from connect(), or
  2. Store it as an instance variable, or
  3. Refactor _smile_detect_legacy() to work with the new model data
-    async def _smile_detect(self) -> None:
+    async def _smile_detect(self, dsmrmain: bool) -> None:

And in connect():

-        await self._smile_detect()
+        await self._smile_detect(dsmrmain)

For result, you'll need to determine what XML data _smile_detect_legacy() actually needs and pass it appropriately.

🧰 Tools
🪛 GitHub Actions: Latest commit

[error] 210-210: ruff check failed: F821 Undefined name result.

[error] 210-210: ruff check failed: F821 Undefined name dsmrmain.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@plugwise/__init__.py` around lines 208 - 211, The call to
_smile_detect_legacy in _smile_detect is using undefined variables result and
dsmrmain; update the code so legacy detection receives the correct data: either
add dsmrmain as a parameter to _smile_detect and pass it from connect(), or
store dsmrmain on self in connect() and read self.dsmrmain inside _smile_detect,
and replace result with the actual XML/object the legacy routine needs (or
refactor _smile_detect_legacy to accept the current model data). Locate usages
in connect(), _smile_detect(), and _smile_detect_legacy to make the
parameter/attribute consistent and ensure all callers are updated accordingly.


if model == "Unknown" or self.smile.version == Version(
"0.0.0"
if (
self.data.gateway.vendor_model == "Unknown"

Check warning on line 214 in plugwise/__init__.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Fix this attribute access on a value that can be 'None'. 


See more on https://sonarcloud.io/project/issues?id=plugwise_python-plugwise&issues=AZ1ejLn3rHfHDGLiEckD&open=AZ1ejLn3rHfHDGLiEckD&pullRequest=870
or self.smile.firmware_version == Version("0.0.0")
): # pragma: no cover
# Corner case check
LOGGER.error(
Expand All @@ -220,8 +221,8 @@
)
raise UnsupportedDeviceError

version_major = str(self.smile.version.major)
self._target_smile = f"{model}_v{version_major}"
version_major = Version(self.smile.firmware_version).major
self._target_smile = f"{self.data.gateway.vendor_model}_v{version_major}"
LOGGER.debug("Plugwise identified as %s", self._target_smile)
if self._target_smile not in SMILES:
LOGGER.error(
Expand All @@ -242,7 +243,7 @@
raise UnsupportedDeviceError # pragma: no cover

self.smile.model = "Gateway"
self.smile.model_id = model
self.smile.model_id = self.data.gateway.vendor_model
self.smile.name = SMILES[self._target_smile].smile_name
self.smile.type = SMILES[self._target_smile].smile_type
if self.smile.name == "Smile Anna" and self.smile.anna_p1:
Expand All @@ -251,9 +252,9 @@
if self.smile.type == "stretch":
self._stretch_v2 = int(version_major) == 2

self._process_for_thermostat(result)
self._process_for_thermostat()

def _process_for_thermostat(self, result: etree.Element) -> None:
def _process_for_thermostat(self) -> None:
"""Extra processing for thermostats."""
if self.smile.type != "thermostat":
return
Expand All @@ -262,18 +263,22 @@
# For Adam, Anna, determine the system capabilities:
# Find the connected heating/cooling device (heater_central),
# e.g. heat-pump or gas-fired heater
onoff_boiler = result.find("./module/protocols/onoff_boiler")
open_therm_boiler = result.find("./module/protocols/open_therm_boiler")
self._on_off_device = onoff_boiler is not None
self._opentherm_device = open_therm_boiler is not None
self._on_off_device: bool = (
True
if "protocols" in self.data.module
and "on_off_boiler" in self.data.module.protocols
else False
)
self._opentherm_device: bool = (
True
if "protocols" in self.data.module
and "open_therm_boiler" in self.data.module.protocols
else False
)
Comment on lines +266 to +277
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Type error: self.data.module is a list, not an object with protocols attribute.

Lines 118-122 iterate over self.data.module as a list, but lines 268-276 access self.data.module.protocols as if it's a single object. This inconsistency will cause an AttributeError.

🐛 Proposed fix - iterate over modules 
-        self._on_off_device: bool = (
-            True
-            if "protocols" in self.data.module
-            and "on_off_boiler" in self.data.module.protocols
-            else False
-        )
-        self._opentherm_device: bool = (
-            True
-            if "protocols" in self.data.module
-            and "open_therm_boiler" in self.data.module.protocols
-            else False
-        )
+        self._on_off_device = False
+        self._opentherm_device = False
+        for module in self.data.module:
+            if module.protocols:
+                if "on_off_boiler" in module.protocols:
+                    self._on_off_device = True
+                if "open_therm_boiler" in module.protocols:
+                    self._opentherm_device = True
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
self._on_off_device: bool = (
True
if "protocols" in self.data.module
and "on_off_boiler" in self.data.module.protocols
else False
)
self._opentherm_device: bool = (
True
if "protocols" in self.data.module
and "open_therm_boiler" in self.data.module.protocols
else False
)
self._on_off_device = False
self._opentherm_device = False
for module in self.data.module:
if module.protocols:
if "on_off_boiler" in module.protocols:
self._on_off_device = True
if "open_therm_boiler" in module.protocols:
self._opentherm_device = True
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@plugwise/__init__.py` around lines 266 - 277, self.data.module is a list, but
the flags _on_off_device and _opentherm_device treat it like a single object;
update the checks for self._on_off_device and self._opentherm_device to iterate
over the modules (e.g. use a loop or any(...) across self.data.module) and check
each module's protocols for "on_off_boiler" and "open_therm_boiler"
respectively, setting the boolean to True if any module contains the protocol;
ensure you reference self.data.module and the attribute protocols on each module
item when implementing the fix.


# Determine the presence of special features
locator_1 = "./gateway/features/cooling"
locator_2 = "./gateway/features/elga_support"
if result.find(locator_1) is not None:
self._cooling_present = True
if result.find(locator_2) is not None:
self._elga = True
self._cooling_present = "cooling" in self.data.gateway.features
self._elga = "elga_support" in self.data.gateway.features

async def _smile_detect_legacy(
self, result: etree.Element, dsmrmain: etree.Element, model: str
Expand Down
Loading
Loading