From 8fa470cec3496f09914662bcf3a91f24de030528 Mon Sep 17 00:00:00 2001 From: Tony Fruzza Date: Tue, 19 May 2026 20:45:18 +0000 Subject: [PATCH 1/3] feat(pumps): expose VSP power and pump diagnostics - Parse pump power from telemetry (@power) and expose Pump.power - Add async_get_pump_diagnostics API alias for VSP feature pumps - Add debug get-pump-diagnostics CLI command - Decode diagnostics power/revision/error into human-readable fields - Add tests for pump diagnostics request and power parsing Closes #99 --- README.md | 5 +- pyomnilogic_local/api/api.py | 25 +++++++ pyomnilogic_local/cli/debug/commands.py | 43 ++++++++++- .../models/filter_diagnostics.py | 42 +++++++++++ pyomnilogic_local/models/telemetry.py | 2 + pyomnilogic_local/pump.py | 6 ++ tests/test_api.py | 71 +++++++++++++++++++ 7 files changed, 191 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index db34276..62eedb2 100644 --- a/README.md +++ b/README.md @@ -280,7 +280,10 @@ omnilogic get heaters omnilogic debug --raw get-mspconfig # View filter diagnostics -omnilogic debug get-filter-diagnostics +omnilogic debug get-filter-diagnostics + +# View VSP pump diagnostics +omnilogic debug get-pump-diagnostics ``` **Installation with CLI tools**: diff --git a/pyomnilogic_local/api/api.py b/pyomnilogic_local/api/api.py index 726fff2..bff9ae1 100644 --- a/pyomnilogic_local/api/api.py +++ b/pyomnilogic_local/api/api.py @@ -223,6 +223,31 @@ 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_pump_diagnostics(self, pool_id: int, equipment_id: int, raw: Literal[True]) -> str: ... + @overload + async def async_get_pump_diagnostics(self, pool_id: int, equipment_id: int, raw: Literal[False]) -> FilterDiagnostics: ... + @overload + async def async_get_pump_diagnostics(self, pool_id: int, equipment_id: int) -> FilterDiagnostics: ... + @overload + async def async_get_pump_diagnostics(self, pool_id: int, equipment_id: int, raw: bool) -> FilterDiagnostics | str: ... + async def async_get_pump_diagnostics(self, pool_id: int, equipment_id: int, raw: bool = False) -> FilterDiagnostics | str: + """Retrieve diagnostics for a VSP pump. + + OmniLogic uses the same underlying request type for both filter and auxiliary + VSP pump diagnostics. This helper exists to make pump diagnostics discoverable + without requiring users to call a filter-named API method. + + Args: + pool_id (int): The Pool/BodyOfWater ID that you want to address + equipment_id (int): Which equipment_id within that Pool to address + raw (bool): Do not parse the response into a Pydantic model, just return the raw XML. Defaults to False. + + Returns: + FilterDiagnostics|str: Either a parsed FilterDiagnostics object or a raw XML string. + """ + return await self.async_get_filter_diagnostics(pool_id=pool_id, equipment_id=equipment_id, raw=raw) + @overload async def async_get_telemetry(self, raw: Literal[True]) -> str: ... @overload diff --git a/pyomnilogic_local/cli/debug/commands.py b/pyomnilogic_local/cli/debug/commands.py index b21c53f..46dc7d1 100644 --- a/pyomnilogic_local/cli/debug/commands.py +++ b/pyomnilogic_local/cli/debug/commands.py @@ -87,8 +87,47 @@ def get_filter_diagnostics(ctx: click.Context, bow_id: int, equip_id: int) -> No """ omnilogic: OmniLogic = ctx.obj["OMNILOGIC"] - telemetry = asyncio.run(omnilogic._api.async_get_filter_diagnostics(pool_id=bow_id, equipment_id=equip_id, raw=ctx.obj["RAW"])) - click.echo(telemetry) + diagnostics = asyncio.run(omnilogic._api.async_get_filter_diagnostics(pool_id=bow_id, equipment_id=equip_id, raw=ctx.obj["RAW"])) + if ctx.obj["RAW"]: + click.echo(diagnostics) + return + + click.echo(f"PoolID: {bow_id}") + click.echo(f"EquipmentID: {equip_id}") + click.echo(f"Power: {diagnostics.power_watts} W") + click.echo(f"Drive Rev: {diagnostics.drive_firmware_revision or 'Unknown'}") + click.echo(f"Display Rev: {diagnostics.display_firmware_revision or 'Unknown'}") + click.echo(f"Error: {diagnostics.error_summary}") + + +@debug.command() +@click.argument("bow_id", type=int) +@click.argument("equip_id", type=int) +@click.pass_context +def get_pump_diagnostics(ctx: click.Context, bow_id: int, equip_id: int) -> None: + """Retrieve current VSP pump diagnostics from the controller. + + Pump diagnostics use the same OmniLogic request type as filter diagnostics, + but this command is provided so pump diagnostics are discoverable under pump + workflows. + + Example: + omnilogic debug get-pump-diagnostics 1 9 + omnilogic debug --raw get-pump-diagnostics 1 9 + + """ + omnilogic: OmniLogic = ctx.obj["OMNILOGIC"] + diagnostics = asyncio.run(omnilogic._api.async_get_pump_diagnostics(pool_id=bow_id, equipment_id=equip_id, raw=ctx.obj["RAW"])) + if ctx.obj["RAW"]: + click.echo(diagnostics) + return + + click.echo(f"PoolID: {bow_id}") + click.echo(f"EquipmentID: {equip_id}") + click.echo(f"Power: {diagnostics.power_watts} W") + click.echo(f"Drive Rev: {diagnostics.drive_firmware_revision or 'Unknown'}") + click.echo(f"Display Rev: {diagnostics.display_firmware_revision or 'Unknown'}") + click.echo(f"Error: {diagnostics.error_summary}") @debug.command() diff --git a/pyomnilogic_local/models/filter_diagnostics.py b/pyomnilogic_local/models/filter_diagnostics.py index c582b9c..efa4660 100644 --- a/pyomnilogic_local/models/filter_diagnostics.py +++ b/pyomnilogic_local/models/filter_diagnostics.py @@ -56,6 +56,48 @@ class FilterDiagnostics(BaseModel): def get_param_by_name(self, name: str) -> int: return next(param.value for param in self.parameters if param.name == name) + def _decode_revision(self, prefix: str) -> str: + bytes_ = [] + for index in range(1, 7): + param_name = f"{prefix}B{index}" + try: + byte_val = self.get_param_by_name(param_name) + except StopIteration: + break + if byte_val == 0: + break + bytes_.append(byte_val) + if not bytes_: + return "" + return bytes(bytes_).decode("ascii", errors="ignore").strip() + + @property + def power_watts(self) -> int: + """Current power draw in watts computed from MSB/LSB fields.""" + lsb = self.get_param_by_name("PowerLSB") + msb = self.get_param_by_name("PowerMSB") + return (msb << 8) | lsb + + @property + def drive_firmware_revision(self) -> str: + """Drive firmware revision string, if present in diagnostics payload.""" + return self._decode_revision("DriveFWRevision") + + @property + def display_firmware_revision(self) -> str: + """Display firmware revision string, if present in diagnostics payload.""" + return self._decode_revision("DisplayFWRevision") + + @property + def error_status(self) -> int: + """Raw error status code reported by controller diagnostics.""" + return self.get_param_by_name("ErrorStatus") + + @property + def error_summary(self) -> str: + """Friendly error summary for diagnostics.""" + return "No errors detected" if self.error_status == 0 else f"Error status code {self.error_status}" + @staticmethod def load_xml(xml: str) -> FilterDiagnostics: data = xml_parse( diff --git a/pyomnilogic_local/models/telemetry.py b/pyomnilogic_local/models/telemetry.py index 3c2907d..88a427b 100644 --- a/pyomnilogic_local/models/telemetry.py +++ b/pyomnilogic_local/models/telemetry.py @@ -370,6 +370,7 @@ class TelemetryPump(BaseModel): Fields: state: Current pump state (OFF, ON, FREEZE_PROTECT) speed: Current speed setting (percentage 0-100 or RPM depending on type) + power: Current power consumption in watts last_speed: Previous speed setting before state change why_on: Reason pump is running (usage similar to FilterWhyOn) """ @@ -380,6 +381,7 @@ class TelemetryPump(BaseModel): system_id: int = Field(alias="@systemId") state: PumpState = Field(alias="@pumpState") speed: int = Field(alias="@pumpSpeed") + power: int = Field(alias="@power", default=0) last_speed: int = Field(alias="@lastSpeed") why_on: int = Field(alias="@whyOn") diff --git a/pyomnilogic_local/pump.py b/pyomnilogic_local/pump.py index 2169803..ea1c508 100644 --- a/pyomnilogic_local/pump.py +++ b/pyomnilogic_local/pump.py @@ -42,6 +42,7 @@ class Pump(OmniEquipment[MSPPump, TelemetryPump]): Properties (Telemetry): state: Current operational state (OFF, ON) speed: Current operating speed + power: Current power consumption in watts last_speed: Previous speed setting why_on: Reason code for pump being on @@ -145,6 +146,11 @@ def speed(self) -> int: """Current pump speed.""" return self.telemetry.speed + @property + def power(self) -> int: + """Current power consumption.""" + return self.telemetry.power + @property def last_speed(self) -> int: """Last speed setting.""" diff --git a/tests/test_api.py b/tests/test_api.py index 019b087..c7f9ec7 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -26,6 +26,8 @@ XML_NAMESPACE, ) from pyomnilogic_local.api.exceptions import OmniValidationError +from pyomnilogic_local.models.filter_diagnostics import FilterDiagnostics +from pyomnilogic_local.models.telemetry import TelemetryPump from pyomnilogic_local.omnitypes import ColorLogicBrightness, ColorLogicShow40, ColorLogicSpeed, HeaterMode, MessageType # ============================================================================ @@ -243,6 +245,75 @@ 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_pump_diagnostics_generates_valid_xml() -> None: + """Test that async_get_pump_diagnostics generates valid XML with correct parameters.""" + api = OmniLogicAPI("192.168.1.100") + + with patch.object(api, "async_send_and_receive", new_callable=AsyncMock) as mock_send: + mock_send.return_value = 'PumpDiagnostics' + + await api.async_get_pump_diagnostics(pool_id=1, equipment_id=9, raw=True) + + mock_send.assert_called_once() + call_args = mock_send.call_args + + xml_payload = call_args[0][1] + root = ET.fromstring(xml_payload) + + assert _get_xml_tag(root) == "Request" + assert _find_elem(root, "Name").text == "GetUIFilterDiagnosticInfo" + assert _find_param(root, "poolId").text == "1" + assert _find_param(root, "equipmentId").text == "9" + + +def test_filter_diagnostics_computed_fields() -> None: + """Power and revision fields should be decoded from diagnostics payload.""" + xml = """ + + GetUIFilterDiagnosticInfoRsp + + 1 + 9 + 29 + 2 + 0 + 49 + 48 + 46 + 49 + 46 + 55 + 49 + 46 + 48 + 65 + 0 + +""" + + diagnostics = FilterDiagnostics.load_xml(xml) + assert diagnostics.power_watts == 541 + assert diagnostics.display_firmware_revision == "10.1.7" + assert diagnostics.drive_firmware_revision == "1.0A" + assert diagnostics.error_summary == "No errors detected" + + +def test_telemetry_pump_parses_power() -> None: + """TelemetryPump should parse power when present.""" + telemetry = TelemetryPump.model_validate( + { + "@systemId": "9", + "@pumpState": "1", + "@pumpSpeed": "70", + "@power": "541", + "@lastSpeed": "70", + "@whyOn": "0", + } + ) + assert telemetry.power == 541 + + @pytest.mark.asyncio async def test_async_set_heater_generates_valid_xml() -> None: """Test that async_set_heater generates valid XML with correct parameters.""" From 419ce0f746dce77cee230c31c07d773c5885d1c5 Mon Sep 17 00:00:00 2001 From: Tony Fruzza Date: Tue, 19 May 2026 20:47:28 +0000 Subject: [PATCH 2/3] fix(diagnostics): format firmware revisions like Omni UI --- pyomnilogic_local/models/filter_diagnostics.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pyomnilogic_local/models/filter_diagnostics.py b/pyomnilogic_local/models/filter_diagnostics.py index efa4660..d6fc5ec 100644 --- a/pyomnilogic_local/models/filter_diagnostics.py +++ b/pyomnilogic_local/models/filter_diagnostics.py @@ -1,5 +1,7 @@ from __future__ import annotations +import re + from pydantic import BaseModel, ConfigDict, Field, PrivateAttr, ValidationError from xmltodict import parse as xml_parse @@ -69,7 +71,15 @@ def _decode_revision(self, prefix: str) -> str: bytes_.append(byte_val) if not bytes_: return "" - return bytes(bytes_).decode("ascii", errors="ignore").strip() + raw = bytes(bytes_).decode("ascii", errors="ignore").strip() + if re.fullmatch(r"\d{4}", raw): + return f"{raw[:2]}.{raw[2]}.{raw[3]}" + + compact = raw.lstrip("0") or raw + if re.fullmatch(r"\d{2}[A-Za-z]", compact): + return f"{compact[0]}.{compact[1:]}" + + return raw @property def power_watts(self) -> int: From 8bc1e91c72b42d0e3cb3e297f3e15e099dfc7599 Mon Sep 17 00:00:00 2001 From: Tony Fruzza Date: Tue, 19 May 2026 20:55:22 +0000 Subject: [PATCH 3/3] refactor: reduce duplicated diagnostics code for quality gate --- pyomnilogic_local/api/api.py | 8 ------ pyomnilogic_local/cli/debug/commands.py | 24 ++++++++-------- tests/test_api.py | 37 +++++++------------------ 3 files changed, 22 insertions(+), 47 deletions(-) diff --git a/pyomnilogic_local/api/api.py b/pyomnilogic_local/api/api.py index bff9ae1..7e3a05f 100644 --- a/pyomnilogic_local/api/api.py +++ b/pyomnilogic_local/api/api.py @@ -223,14 +223,6 @@ 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_pump_diagnostics(self, pool_id: int, equipment_id: int, raw: Literal[True]) -> str: ... - @overload - async def async_get_pump_diagnostics(self, pool_id: int, equipment_id: int, raw: Literal[False]) -> FilterDiagnostics: ... - @overload - async def async_get_pump_diagnostics(self, pool_id: int, equipment_id: int) -> FilterDiagnostics: ... - @overload - async def async_get_pump_diagnostics(self, pool_id: int, equipment_id: int, raw: bool) -> FilterDiagnostics | str: ... async def async_get_pump_diagnostics(self, pool_id: int, equipment_id: int, raw: bool = False) -> FilterDiagnostics | str: """Retrieve diagnostics for a VSP pump. diff --git a/pyomnilogic_local/cli/debug/commands.py b/pyomnilogic_local/cli/debug/commands.py index 46dc7d1..464c127 100644 --- a/pyomnilogic_local/cli/debug/commands.py +++ b/pyomnilogic_local/cli/debug/commands.py @@ -17,10 +17,20 @@ if TYPE_CHECKING: from pyomnilogic_local import OmniLogic + from pyomnilogic_local.models.filter_diagnostics import FilterDiagnostics from pyomnilogic_local.models.telemetry import TelemetryChlorinator from pyomnilogic_local.omnitypes import MessageType +def _echo_diagnostics_summary(diagnostics: "FilterDiagnostics", bow_id: int, equip_id: int) -> None: + click.echo(f"PoolID: {bow_id}") + click.echo(f"EquipmentID: {equip_id}") + click.echo(f"Power: {diagnostics.power_watts} W") + click.echo(f"Drive Rev: {diagnostics.drive_firmware_revision or 'Unknown'}") + click.echo(f"Display Rev: {diagnostics.display_firmware_revision or 'Unknown'}") + click.echo(f"Error: {diagnostics.error_summary}") + + @click.group() @click.option("--raw/--no-raw", default=False, help="Output the raw XML from the OmniLogic, do not parse the response") @click.pass_context @@ -92,12 +102,7 @@ def get_filter_diagnostics(ctx: click.Context, bow_id: int, equip_id: int) -> No click.echo(diagnostics) return - click.echo(f"PoolID: {bow_id}") - click.echo(f"EquipmentID: {equip_id}") - click.echo(f"Power: {diagnostics.power_watts} W") - click.echo(f"Drive Rev: {diagnostics.drive_firmware_revision or 'Unknown'}") - click.echo(f"Display Rev: {diagnostics.display_firmware_revision or 'Unknown'}") - click.echo(f"Error: {diagnostics.error_summary}") + _echo_diagnostics_summary(diagnostics, bow_id, equip_id) @debug.command() @@ -122,12 +127,7 @@ def get_pump_diagnostics(ctx: click.Context, bow_id: int, equip_id: int) -> None click.echo(diagnostics) return - click.echo(f"PoolID: {bow_id}") - click.echo(f"EquipmentID: {equip_id}") - click.echo(f"Power: {diagnostics.power_watts} W") - click.echo(f"Drive Rev: {diagnostics.drive_firmware_revision or 'Unknown'}") - click.echo(f"Display Rev: {diagnostics.display_firmware_revision or 'Unknown'}") - click.echo(f"Error: {diagnostics.error_summary}") + _echo_diagnostics_summary(diagnostics, bow_id, equip_id) @debug.command() diff --git a/tests/test_api.py b/tests/test_api.py index c7f9ec7..fbd0eed 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -224,36 +224,19 @@ async def test_async_get_telemetry_generates_valid_xml() -> None: @pytest.mark.asyncio -async def test_async_get_filter_diagnostics_generates_valid_xml() -> None: - """Test that async_get_filter_diagnostics generates valid XML with correct parameters.""" - api = OmniLogicAPI("192.168.1.100") - - with patch.object(api, "async_send_and_receive", new_callable=AsyncMock) as mock_send: - mock_send.return_value = 'FilterDiagnostics' - - await api.async_get_filter_diagnostics(pool_id=1, equipment_id=2, raw=True) - - mock_send.assert_called_once() - call_args = mock_send.call_args - - xml_payload = call_args[0][1] - root = ET.fromstring(xml_payload) - - assert _get_xml_tag(root) == "Request" - assert _find_elem(root, "Name").text == "GetUIFilterDiagnosticInfo" - assert _find_param(root, "poolId").text == "1" - assert _find_param(root, "equipmentId").text == "2" - - -@pytest.mark.asyncio -async def test_async_get_pump_diagnostics_generates_valid_xml() -> None: - """Test that async_get_pump_diagnostics generates valid XML with correct parameters.""" +@pytest.mark.parametrize( + ("method_name", "equipment_id"), + [("async_get_filter_diagnostics", 2), ("async_get_pump_diagnostics", 9)], +) +async def test_async_get_diagnostics_generates_valid_xml(method_name: str, equipment_id: int) -> None: + """Both diagnostics helpers should generate valid XML with correct parameters.""" api = OmniLogicAPI("192.168.1.100") with patch.object(api, "async_send_and_receive", new_callable=AsyncMock) as mock_send: - mock_send.return_value = 'PumpDiagnostics' + mock_send.return_value = 'Diagnostics' - await api.async_get_pump_diagnostics(pool_id=1, equipment_id=9, raw=True) + method = getattr(api, method_name) + await method(pool_id=1, equipment_id=equipment_id, raw=True) mock_send.assert_called_once() call_args = mock_send.call_args @@ -264,7 +247,7 @@ async def test_async_get_pump_diagnostics_generates_valid_xml() -> None: assert _get_xml_tag(root) == "Request" assert _find_elem(root, "Name").text == "GetUIFilterDiagnosticInfo" assert _find_param(root, "poolId").text == "1" - assert _find_param(root, "equipmentId").text == "9" + assert _find_param(root, "equipmentId").text == str(equipment_id) def test_filter_diagnostics_computed_fields() -> None: