From 2317b2a4d0ac5b5abedd27fb62fe831374b54579 Mon Sep 17 00:00:00 2001 From: kmbulebu <3845898+kmbulebu@users.noreply.github.com> Date: Sun, 19 Jul 2026 17:32:03 -0400 Subject: [PATCH 1/2] feat: add chlorinator diagnostics Add local API requests for chlorinator measurements and relay polarity. Parse cell voltage, current, cell and board temperatures, and instant and average salt levels while preserving the raw controller values. --- pyomnilogic_local/api/api.py | 63 ++++ pyomnilogic_local/models/__init__.py | 3 + .../models/chlorinator_diagnostics.py | 296 ++++++++++++++++++ pyomnilogic_local/omnitypes.py | 3 + tests/test_api.py | 42 +++ tests/test_chlorinator_diagnostics.py | 140 +++++++++ 6 files changed, 547 insertions(+) create mode 100644 pyomnilogic_local/models/chlorinator_diagnostics.py create mode 100644 tests/test_chlorinator_diagnostics.py diff --git a/pyomnilogic_local/api/api.py b/pyomnilogic_local/api/api.py index 072c17f..d11a428 100644 --- a/pyomnilogic_local/api/api.py +++ b/pyomnilogic_local/api/api.py @@ -5,6 +5,7 @@ import xml.etree.ElementTree as ET from typing import TYPE_CHECKING, Literal, overload +from pyomnilogic_local.models.chlorinator_diagnostics import ChlorinatorMeasurement, ChlorinatorRelayPolarity from pyomnilogic_local.models.filter_diagnostics import FilterDiagnostics from pyomnilogic_local.models.mspconfig import MSPConfig from pyomnilogic_local.models.telemetry import Telemetry @@ -223,6 +224,68 @@ async def async_get_filter_diagnostics(self, pool_id: int, equipment_id: int, ra return resp return FilterDiagnostics.load_xml(resp) + @overload + async def async_get_chlorinator_measurement(self, pool_id: int, chlorinator_id: int, raw: Literal[True]) -> str: ... + @overload + async def async_get_chlorinator_measurement(self, pool_id: int, chlorinator_id: int, raw: Literal[False]) -> ChlorinatorMeasurement: ... + @overload + async def async_get_chlorinator_measurement(self, pool_id: int, chlorinator_id: int) -> ChlorinatorMeasurement: ... + async def async_get_chlorinator_measurement(self, pool_id: int, chlorinator_id: int, raw: bool = False) -> ChlorinatorMeasurement | str: + """Retrieve electrical, temperature, and salt measurements from a chlorinator. + + Args: + pool_id: Body-of-water system ID. + chlorinator_id: Physical chlorinator equipment ID. + raw: Return the raw XML response instead of a parsed model. + + Returns: + Parsed chlorinator measurements or the raw XML response. + """ + req_body = self._build_chlorinator_diagnostic_request("GetCHLORMeasurement", pool_id, chlorinator_id) + resp = await self.async_send_and_receive(MessageType.GET_DIAGNOSTIC, req_body) + if raw: + return resp + return ChlorinatorMeasurement.load_xml(resp) + + @overload + async def async_get_chlorinator_relay_polarity(self, pool_id: int, chlorinator_id: int, raw: Literal[True]) -> str: ... + @overload + async def async_get_chlorinator_relay_polarity( + self, pool_id: int, chlorinator_id: int, raw: Literal[False] + ) -> ChlorinatorRelayPolarity: ... + @overload + async def async_get_chlorinator_relay_polarity(self, pool_id: int, chlorinator_id: int) -> ChlorinatorRelayPolarity: ... + async def async_get_chlorinator_relay_polarity( + self, pool_id: int, chlorinator_id: int, raw: bool = False + ) -> ChlorinatorRelayPolarity | str: + """Retrieve the relay polarity from physical chlorinator equipment. + + Args: + pool_id: Body-of-water system ID. + chlorinator_id: Physical chlorinator equipment ID. + raw: Return the raw XML response instead of a parsed model. + + Returns: + Parsed relay polarity or the raw XML response. + """ + req_body = self._build_chlorinator_diagnostic_request("GetCHLORRelayPolarity", pool_id, chlorinator_id) + resp = await self.async_send_and_receive(MessageType.GET_DIAGNOSTIC, req_body) + if raw: + return resp + return ChlorinatorRelayPolarity.load_xml(resp) + + @staticmethod + def _build_chlorinator_diagnostic_request(operation: str, pool_id: int, chlorinator_id: int) -> str: + body_element = ET.Element("Request", {"xmlns": XML_NAMESPACE}) + name_element = ET.SubElement(body_element, "Name") + name_element.text = operation + parameters_element = ET.SubElement(body_element, "Parameters") + parameter = ET.SubElement(parameters_element, "Parameter", name="PoolID", dataType="int") + parameter.text = str(pool_id) + parameter = ET.SubElement(parameters_element, "Parameter", name="ChlorID", dataType="int") + parameter.text = str(chlorinator_id) + return ET.tostring(body_element, xml_declaration=True, encoding="unicode") + @overload async def async_get_telemetry(self, raw: Literal[True]) -> str: ... @overload diff --git a/pyomnilogic_local/models/__init__.py b/pyomnilogic_local/models/__init__.py index 320c5d4..b10a251 100644 --- a/pyomnilogic_local/models/__init__.py +++ b/pyomnilogic_local/models/__init__.py @@ -2,11 +2,14 @@ from __future__ import annotations +from .chlorinator_diagnostics import ChlorinatorMeasurement, ChlorinatorRelayPolarity from .filter_diagnostics import FilterDiagnostics from .mspconfig import MSPConfig, MSPConfigType, MSPEquipmentType from .telemetry import Telemetry, TelemetryType __all__ = [ + "ChlorinatorMeasurement", + "ChlorinatorRelayPolarity", "FilterDiagnostics", "MSPConfig", "MSPConfigType", diff --git a/pyomnilogic_local/models/chlorinator_diagnostics.py b/pyomnilogic_local/models/chlorinator_diagnostics.py new file mode 100644 index 0000000..fd06122 --- /dev/null +++ b/pyomnilogic_local/models/chlorinator_diagnostics.py @@ -0,0 +1,296 @@ +"""Models for OmniLogic chlorinator diagnostic responses.""" + +from __future__ import annotations + +import math +from typing import ClassVar, Self + +from pydantic import BaseModel, ConfigDict, Field, PrivateAttr, ValidationError +from xmltodict import parse as xml_parse + +from pyomnilogic_local.models.exceptions import OmniParsingError + +_INVALID_DIAGNOSTIC_VALUE = 0xFFFF +_ADC_REFERENCE_VOLTAGE = 5 +_ADC_MAX_VALUE = 255 +_VOLTAGE_SCALE = 8.15 +_CURRENT_SCALE = 0.489 + +# Lookup table used by OmniLogic to convert the cell and board thermistor ADC +# readings to tenths of a degree Fahrenheit. Values between entries are +# interpolated using the low three bits of the raw reading. +_TEMPERATURE_TENTHS_F = ( + 8000, + 3740, + 3136, + 2817, + 2603, + 2444, + 2318, + 2214, + 2125, + 2048, + 1980, + 1919, + 1864, + 1813, + 1767, + 1723, + 1683, + 1645, + 1610, + 1576, + 1544, + 1514, + 1485, + 1458, + 1431, + 1406, + 1381, + 1358, + 1335, + 1313, + 1291, + 1270, + 1250, + 1230, + 1211, + 1193, + 1174, + 1156, + 1139, + 1122, + 1105, + 1088, + 1072, + 1056, + 1041, + 1025, + 1010, + 995, + 980, + 966, + 951, + 937, + 923, + 909, + 895, + 881, + 868, + 854, + 841, + 828, + 815, + 802, + 789, + 776, + 763, + 750, + 737, + 725, + 712, + 699, + 687, + 674, + 661, + 649, + 636, + 624, + 611, + 598, + 586, + 573, + 560, + 547, + 534, + 521, + 508, + 495, + 482, + 469, + 455, + 442, + 428, + 414, + 400, + 386, + 372, + 357, + 342, + 327, + 312, + 297, + 281, + 265, + 248, + 231, + 214, + 196, + 178, + 159, + 140, + 120, + 99, + 77, + 54, + 31, + 6, +) + + +def _combine_bytes(high_byte: int, low_byte: int) -> int: + return high_byte << 8 | low_byte + + +def _decode_temperature(raw_value: int) -> float | None: + if raw_value == 0: + return 0.0 + table_index, interpolation = divmod(raw_value, 8) + if raw_value >= _INVALID_DIAGNOSTIC_VALUE or table_index >= len(_TEMPERATURE_TENTHS_F) - 1: + return None + + temperature = _TEMPERATURE_TENTHS_F[table_index] + if interpolation: + difference = temperature - _TEMPERATURE_TENTHS_F[table_index + 1] + temperature -= math.floor(((difference * interpolation * 10 / 8) + 5) / 10) + return temperature / 10 + + +class ChlorinatorDiagnosticParameter(BaseModel): + """A named value from a chlorinator diagnostic response.""" + + model_config = ConfigDict(from_attributes=True) + + name: str = Field(alias="@name") + data_type: str = Field(alias="@dataType") + value: int = Field(alias="#text") + + +class ChlorinatorDiagnosticResponse(BaseModel): + """Base model for parameter-based chlorinator diagnostic responses.""" + + model_config = ConfigDict(from_attributes=True) + expected_response_name: ClassVar[str] + _raw: str = PrivateAttr(default="") + + name: str = Field(alias="Name") + parameters: list[ChlorinatorDiagnosticParameter] = Field(alias="Parameters") + + def get_param_by_name(self, name: str) -> int: + """Return a diagnostic parameter by its wire name.""" + parameter = next((parameter for parameter in self.parameters if parameter.name == name), None) + if parameter is None: + msg = f"Missing chlorinator diagnostic parameter: {name}" + raise OmniParsingError(msg) + return parameter.value + + @classmethod + def load_xml(cls, xml: str) -> Self: + """Parse a chlorinator diagnostic XML response.""" + data = xml_parse(xml, force_list=("Parameter",)) + response_name = data.get("Response", {}).get("Name") + if response_name != cls.expected_response_name: + msg = f"Expected {cls.expected_response_name} response, got {response_name!r}" + raise OmniParsingError(msg) + data["Response"]["Parameters"] = data["Response"]["Parameters"]["Parameter"] + try: + instance = cls.model_validate(data["Response"]) + instance._raw = xml + except ValidationError as exc: + msg = f"Failed to parse chlorinator diagnostics: {exc}" + raise OmniParsingError(msg) from exc + return instance + + +class ChlorinatorMeasurement(ChlorinatorDiagnosticResponse): + """Electrical, temperature, and salt measurements from a chlorinator cell.""" + + expected_response_name = "GetCHLORMeasurementRsp" + + @property + def pool_id(self) -> int: + """Body-of-water system ID.""" + return self.get_param_by_name("PoolID") + + @property + def chlorinator_id(self) -> int: + """Physical chlorinator equipment ID.""" + return self.get_param_by_name("ChlorID") + + def _combined_parameter(self, prefix: str) -> int: + return _combine_bytes(self.get_param_by_name(f"{prefix}HighByte"), self.get_param_by_name(f"{prefix}LowByte")) + + @property + def voltage_raw(self) -> int: + """Raw cell-voltage ADC reading.""" + return self._combined_parameter("Voltage") + + @property + def current_raw(self) -> int: + """Raw cell-current ADC reading.""" + return self._combined_parameter("Current") + + @property + def cell_temperature_raw(self) -> int: + """Raw cell-temperature thermistor reading.""" + return self._combined_parameter("CellTemp") + + @property + def board_temperature_raw(self) -> int: + """Raw controller-board temperature thermistor reading.""" + return self._combined_parameter("BoardTemp") + + @property + def voltage(self) -> float | None: + """Cell voltage in volts, or None when the controller reports an invalid sentinel.""" + if self.voltage_raw >= _INVALID_DIAGNOSTIC_VALUE: + return None + return self.voltage_raw * _ADC_REFERENCE_VOLTAGE / _ADC_MAX_VALUE * _VOLTAGE_SCALE + + @property + def current(self) -> float | None: + """Cell current in amperes, or None when the controller reports an invalid sentinel.""" + if self.current_raw >= _INVALID_DIAGNOSTIC_VALUE: + return None + return (self.current_raw * _ADC_REFERENCE_VOLTAGE / _ADC_MAX_VALUE) / _CURRENT_SCALE + + @property + def cell_temperature_f(self) -> float | None: + """Cell temperature in degrees Fahrenheit, or None when invalid.""" + return _decode_temperature(self.cell_temperature_raw) + + @property + def board_temperature_f(self) -> float | None: + """Controller-board temperature in degrees Fahrenheit, or None when invalid.""" + return _decode_temperature(self.board_temperature_raw) + + @property + def instant_salt_level(self) -> int: + """Instantaneous salt level in parts per million.""" + return self._combined_parameter("InstantSaltLevel") + + @property + def average_salt_level(self) -> int: + """Average salt level in parts per million.""" + return self._combined_parameter("AverageSaltLevel") + + +class ChlorinatorRelayPolarity(ChlorinatorDiagnosticResponse): + """Relay polarity reported by the physical chlorinator equipment.""" + + expected_response_name = "GetCHLORRelayPolarityRsp" + + @property + def pool_id(self) -> int: + """Body-of-water system ID.""" + return self.get_param_by_name("PoolID") + + @property + def chlorinator_id(self) -> int: + """Physical chlorinator equipment ID.""" + return self.get_param_by_name("ChlorID") + + @property + def relay_setting(self) -> int: + """Raw chlorinator relay polarity setting.""" + return self.get_param_by_name("RelaySetting") diff --git a/pyomnilogic_local/omnitypes.py b/pyomnilogic_local/omnitypes.py index 588cc1c..cc96c68 100644 --- a/pyomnilogic_local/omnitypes.py +++ b/pyomnilogic_local/omnitypes.py @@ -31,13 +31,16 @@ class MessageType(PrettyEnum, IntEnum): RUN_GROUP = 317 RESTORE_IDLE_STATE = 340 GET_FILTER_DIAGNOSTIC = 386 + GET_DIAGNOSTIC = 386 HANDSHAKE = 1000 MSP_ACK = 1002 MSP_CONFIGURATIONUPDATE = 1003 MSP_TELEMETRY_UPDATE = 1004 MSP_GET_FILTER_SPEED_RESPONSE = 1010 + MSP_CHLOR_RELAY_POLARITY_RESPONSE = 1174 MSP_ALARM_LIST_RESPONSE = 1304 MSP_FILTER_DIAGNOSTIC_INFO_RESPONSE = 1386 + MSP_DIAGNOSTIC_INFO_RESPONSE = 1386 MSP_LEADMESSAGE = 1998 MSP_BLOCKMESSAGE = 1999 diff --git a/tests/test_api.py b/tests/test_api.py index ea94359..fe12da5 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -243,6 +243,48 @@ async def test_async_get_filter_diagnostics_generates_valid_xml() -> None: assert _find_param(root, "equipmentId").text == "2" +@pytest.mark.asyncio +async def test_async_get_chlorinator_measurement_generates_valid_xml() -> None: + """Test chlorinator measurement request generation.""" + api = OmniLogicAPI("192.168.1.100") + + with patch.object(api, "async_send_and_receive", new_callable=AsyncMock) as mock_send: + mock_send.return_value = 'GetCHLORMeasurementRsp' + + await api.async_get_chlorinator_measurement(pool_id=1, chlorinator_id=4, raw=True) + + mock_send.assert_awaited_once() + message_type, xml_payload = mock_send.call_args.args + root = ET.fromstring(xml_payload) + + assert message_type is MessageType.GET_DIAGNOSTIC + assert _get_xml_tag(root) == "Request" + assert _find_elem(root, "Name").text == "GetCHLORMeasurement" + assert _find_param(root, "PoolID").text == "1" + assert _find_param(root, "ChlorID").text == "4" + + +@pytest.mark.asyncio +async def test_async_get_chlorinator_relay_polarity_generates_valid_xml() -> None: + """Test chlorinator relay polarity request generation.""" + api = OmniLogicAPI("192.168.1.100") + + with patch.object(api, "async_send_and_receive", new_callable=AsyncMock) as mock_send: + mock_send.return_value = 'GetCHLORRelayPolarityRsp' + + await api.async_get_chlorinator_relay_polarity(pool_id=1, chlorinator_id=4, raw=True) + + mock_send.assert_awaited_once() + message_type, xml_payload = mock_send.call_args.args + root = ET.fromstring(xml_payload) + + assert message_type is MessageType.GET_DIAGNOSTIC + assert _get_xml_tag(root) == "Request" + assert _find_elem(root, "Name").text == "GetCHLORRelayPolarity" + assert _find_param(root, "PoolID").text == "1" + assert _find_param(root, "ChlorID").text == "4" + + @pytest.mark.asyncio async def test_async_set_heater_generates_valid_xml() -> None: """Test that async_set_heater generates valid XML with correct parameters.""" diff --git a/tests/test_chlorinator_diagnostics.py b/tests/test_chlorinator_diagnostics.py new file mode 100644 index 0000000..2d03254 --- /dev/null +++ b/tests/test_chlorinator_diagnostics.py @@ -0,0 +1,140 @@ +"""Tests for chlorinator diagnostic response parsing.""" + +from __future__ import annotations + +import math + +import pytest + +from pyomnilogic_local.models.chlorinator_diagnostics import ChlorinatorMeasurement, ChlorinatorRelayPolarity +from pyomnilogic_local.models.exceptions import OmniParsingError + +MEASUREMENT_XML = """\ + + + GetCHLORMeasurementRsp + + 7 + 8 + 0 + 198 + 0 + 9 + 1 + 200 + 1 + 23 + 11 + 53 + 11 + 120 + + +""" + +OFF_MEASUREMENT_XML = """\ + + + GetCHLORMeasurementRsp + + 7 + 8 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 11 + 84 + + +""" + +INVALID_TEMPERATURE_XML = MEASUREMENT_XML.replace( + '1', + '255', +).replace( + '200', + '255', +) + +POLARITY_XML = """\ + + + GetCHLORRelayPolarityRsp + + 7 + 8 + 1 + + +""" + + +def test_chlorinator_measurement_parses_diagnostics() -> None: + """Parse raw measurement bytes and expose engineering units.""" + measurement = ChlorinatorMeasurement.load_xml(MEASUREMENT_XML) + + assert measurement.pool_id == 7 + assert measurement.chlorinator_id == 8 + assert measurement.voltage_raw == 198 + assert measurement.current_raw == 9 + assert measurement.cell_temperature_raw == 456 + assert measurement.board_temperature_raw == 279 + assert measurement.instant_salt_level == 2869 + assert measurement.average_salt_level == 2936 + assert math.isclose(measurement.voltage, 31.64, abs_tol=0.01) + assert math.isclose(measurement.current, 0.36, abs_tol=0.01) + assert math.isclose(measurement.cell_temperature_f, 85.4, abs_tol=0.1) + assert math.isclose(measurement.board_temperature_f, 119.5, abs_tol=0.1) + + +def test_chlorinator_measurement_preserves_off_state() -> None: + """Represent zero instantaneous values returned while disabled.""" + measurement = ChlorinatorMeasurement.load_xml(OFF_MEASUREMENT_XML) + + assert measurement.voltage == 0.0 + assert measurement.current == 0.0 + assert measurement.cell_temperature_f == 0.0 + assert measurement.board_temperature_f == 0.0 + assert measurement.instant_salt_level == 0 + assert measurement.average_salt_level == 2900 + + +def test_chlorinator_measurement_reports_invalid_temperature() -> None: + """Represent the controller's invalid temperature sentinel as None.""" + measurement = ChlorinatorMeasurement.load_xml(INVALID_TEMPERATURE_XML) + + assert measurement.cell_temperature_raw == 0xFFFF + assert measurement.cell_temperature_f is None + + +def test_chlorinator_relay_polarity_parses_setting() -> None: + """Parse the chlorinator relay polarity setting.""" + polarity = ChlorinatorRelayPolarity.load_xml(POLARITY_XML) + + assert polarity.pool_id == 7 + assert polarity.chlorinator_id == 8 + assert polarity.relay_setting == 1 + + +def test_chlorinator_measurement_reports_missing_parameter() -> None: + """Raise a protocol parsing error when a required parameter is absent.""" + measurement = ChlorinatorMeasurement.load_xml(MEASUREMENT_XML.replace(' name="VoltageLowByte"', ' name="UnexpectedParameter"')) + + with pytest.raises(OmniParsingError, match="Missing chlorinator diagnostic parameter: VoltageLowByte"): + _ = measurement.voltage + + +def test_chlorinator_measurement_rejects_unexpected_response() -> None: + """Reject a response intended for a different diagnostic operation.""" + with pytest.raises( + OmniParsingError, + match="Expected GetCHLORMeasurementRsp response, got 'GetCHLORRelayPolarityRsp'", + ): + ChlorinatorMeasurement.load_xml(POLARITY_XML) From 5d24c17d57ae80fae277be9875f49d34cfd5f63e Mon Sep 17 00:00:00 2001 From: kmbulebu <3845898+kmbulebu@users.noreply.github.com> Date: Sun, 19 Jul 2026 17:46:54 -0400 Subject: [PATCH 2/2] test: address Sonar feedback --- tests/test_api.py | 4 ++-- tests/test_chlorinator_diagnostics.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test_api.py b/tests/test_api.py index fe12da5..0459685 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -246,7 +246,7 @@ async def test_async_get_filter_diagnostics_generates_valid_xml() -> None: @pytest.mark.asyncio async def test_async_get_chlorinator_measurement_generates_valid_xml() -> None: """Test chlorinator measurement request generation.""" - api = OmniLogicAPI("192.168.1.100") + api = OmniLogicAPI("controller.local") with patch.object(api, "async_send_and_receive", new_callable=AsyncMock) as mock_send: mock_send.return_value = 'GetCHLORMeasurementRsp' @@ -267,7 +267,7 @@ async def test_async_get_chlorinator_measurement_generates_valid_xml() -> None: @pytest.mark.asyncio async def test_async_get_chlorinator_relay_polarity_generates_valid_xml() -> None: """Test chlorinator relay polarity request generation.""" - api = OmniLogicAPI("192.168.1.100") + api = OmniLogicAPI("controller.local") with patch.object(api, "async_send_and_receive", new_callable=AsyncMock) as mock_send: mock_send.return_value = 'GetCHLORRelayPolarityRsp' diff --git a/tests/test_chlorinator_diagnostics.py b/tests/test_chlorinator_diagnostics.py index 2d03254..068a44b 100644 --- a/tests/test_chlorinator_diagnostics.py +++ b/tests/test_chlorinator_diagnostics.py @@ -98,10 +98,10 @@ def test_chlorinator_measurement_preserves_off_state() -> None: """Represent zero instantaneous values returned while disabled.""" measurement = ChlorinatorMeasurement.load_xml(OFF_MEASUREMENT_XML) - assert measurement.voltage == 0.0 - assert measurement.current == 0.0 - assert measurement.cell_temperature_f == 0.0 - assert measurement.board_temperature_f == 0.0 + assert measurement.voltage == pytest.approx(0.0) + assert measurement.current == pytest.approx(0.0) + assert measurement.cell_temperature_f == pytest.approx(0.0) + assert measurement.board_temperature_f == pytest.approx(0.0) assert measurement.instant_salt_level == 0 assert measurement.average_salt_level == 2900