Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
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

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
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)

vendor_models = result.findall("./module/vendor_model")
models: list[str] = []
for model in vendor_models:
models.append(model.text)
await self._request(DOMAIN_OBJECTS, new=True)

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

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

async def _smile_detect(
self, result: etree.Element, dsmrmain: etree.Element
) -> None:
async def _smile_detect(self) -> None:

Check failure on line 174 in plugwise/__init__.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 17 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=plugwise_python-plugwise&issues=AZ1ejLn3rHfHDGLiEckB&open=AZ1ejLn3rHfHDGLiEckB&pullRequest=870
"""Helper-function for connect().

Detect which type of Plugwise Gateway is being connected.
"""
print(f"HOI14 {self}")
print(f"HOI14 {self.smile}")
Comment on lines +179 to +180
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Remove debug print statements.

These print statements are debug artifacts that should not be in production code:

  • Lines 179-180: print(f"HOI14 ...")
  • Lines 191-192: print(f"HOI11a/b ...")
  • Lines 197-199: print(f"HOI11c ...")
🧹 Proposed fix
-        print(f"HOI14 {self}")
-        print(f"HOI14 {self.smile}")
         model: str = "Unknown"
-            print(f"HOI11a {self.data.gateway}")
-            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}"
-                )

Also applies to: 191-192, 197-199

🧰 Tools
🪛 GitHub Actions: Latest commit

[error] 179-179: ruff check failed: T201 print found. (Remove print)


[error] 180-180: ruff check failed: T201 print found. (Remove print)

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

In `@plugwise/__init__.py` around lines 179 - 180, Remove the leftover debug print
statements that output "HOI14", "HOI11a/b" and "HOI11c" (the prints referencing
self and self.smile) from plugwise/__init__.py; locate the prints in the methods
where self and self.smile are printed and delete those print(...) calls, or
replace them with appropriate logger.debug(...) calls using the module/class
logger if you need retained diagnostics, ensuring no raw print statements remain
in production code.

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}")
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

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

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