From c4ea0560b8e0b366a6e8defdfde6a957d5e5a0ac Mon Sep 17 00:00:00 2001 From: Christopher Berry Date: Sat, 22 Aug 2026 09:44:31 -0700 Subject: [PATCH] Add guarded pool heat controls --- README.md | 20 ++++++- SKILL.md | 11 +++- docs/operations.md | 16 +++++- docs/protocol.md | 9 ++- docs/roadmap.md | 1 + justfile | 1 + poolctl/cli.py | 66 +++++++++++++++++++++- poolctl/control.py | 80 ++++++++++++++++++++++++++ poolctl/render.py | 8 +++ pyproject.toml | 2 +- tests/test_cli.py | 23 ++++++++ tests/test_control.py | 127 +++++++++++++++++++++++++++++++++++++++++- tests/test_render.py | 9 +++ 13 files changed, 362 insertions(+), 11 deletions(-) create mode 100644 tests/test_cli.py diff --git a/README.md b/README.md index ff015a8..c653cbf 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,7 @@ guarded cleaner and delay controls. - discovers a ScreenLogic adapter on the local subnet; - reads controller, body, circuit, pump, and sensor state; - reports cleaner and system-delay state; +- reads and controls pool/spa heat mode and setpoint; - turns the cleaner on or off after an explicit `--yes` guard; - cancels active delays after an explicit `--yes` guard; - cancels a cleaner delay before enabling the cleaner, then reports final state. @@ -63,6 +64,7 @@ poolctl bodies poolctl pumps poolctl cleaner status poolctl delay status +poolctl heat status ``` Add `--json` after a command for structured output. `poolctl status --raw` @@ -82,6 +84,20 @@ cancels an active cleaner delay, performs the circuit write, and reads status again before reporting success. See [operations](docs/operations.md) for the full behavior and safety boundary. +## Control pool and spa heat + +```bash +poolctl heat status +poolctl heat status pool --json +poolctl heat set pool solar-preferred --yes +poolctl heat temp pool 88 --yes +``` + +Heat modes are `off`, `solar`, `solar-preferred`, and `heater`. Body selection +requires an exact case-insensitive name or numeric ID. Mutating commands read +the body first, enforce the controller-reported temperature limits, require +`--yes`, and return post-write state. See [operations](docs/operations.md). + ## Runtime data | Data | Default path | Git policy | @@ -94,8 +110,8 @@ full behavior and safety boundary. ## Reliability and scope ScreenLogic discovery is LAN-bound, and equipment names and supported sensors -vary by controller configuration. `poolctl` deliberately exposes only the -cleaner circuit and delay cancellation as writes; it is not a general arbitrary +vary by controller configuration. `poolctl` deliberately exposes only cleaner, +delay, and body heat settings as writes; it is not a general arbitrary circuit-toggle interface. See [protocol notes](docs/protocol.md), [troubleshooting](docs/troubleshooting.md), diff --git a/SKILL.md b/SKILL.md index 561c50c..a0db97a 100644 --- a/SKILL.md +++ b/SKILL.md @@ -1,6 +1,6 @@ --- name: poolctl -description: Inspect and control a local Pentair ScreenLogic pool system with the poolctl CLI. Use for pool status, circuits, bodies, pumps, cleaner state, cleaner control, and delay inspection or cancellation. +description: Inspect and control a local Pentair ScreenLogic pool system with the poolctl CLI. Use for pool status, circuits, bodies, pumps, cleaner state, heat mode and setpoint, cleaner control, and delay inspection or cancellation. --- # poolctl @@ -15,7 +15,7 @@ already exists. - Use `--yes` only after the requested equipment and action are clear. - Report final state from the command, not merely that a write was submitted. - Never invent support for an arbitrary circuit; the public write surface is - intentionally limited to cleaner and delay commands. + intentionally limited to cleaner, delay, and pool/spa heat commands. ## Commands @@ -30,6 +30,9 @@ poolctl cleaner on --yes poolctl cleaner off --yes poolctl delay status poolctl delay cancel --yes +poolctl heat status +poolctl heat set pool solar-preferred --yes +poolctl heat temp pool 88 --yes ``` Use `--json` for structured results. Put a one-off direct host before the @@ -38,3 +41,7 @@ subcommand, for example `poolctl --host 192.0.2.10 status`. Cleaner enable already checks and cancels cleaner delay when necessary, then reports the post-action cleaner and delay state. If a command fails, quote the short error and do not claim the hardware reached the requested state. + +Heat writes require an exact body name or ID and `--yes`. Read the current heat +status first when the requested mode or setpoint is ambiguous, and report the +returned `status_after` rather than assuming the request succeeded. diff --git a/docs/operations.md b/docs/operations.md index 2e4b09c..2ddde9a 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -28,8 +28,20 @@ refreshes controller state, and reports cleaner, pool, and spa delay values. Canceling a delay can cause scheduled equipment to resume; inspect the system and physical area first. +## Pool and spa heat + +Use `poolctl heat status` before a heat write. Mode changes accept only `off`, +`solar`, `solar-preferred`, or `heater`; temperature changes must fall within +the selected body's controller-reported minimum and maximum. Exact body names +and numeric IDs are accepted, and every write requires `--yes`. + +Both heat write commands refresh the controller and return `status_before` and +`status_after`. Treat the returned state as authoritative; request acceptance +alone is not success. + ## Live validation Automated tests never contact pool hardware. Before a release that changes write -behavior, validate status, cleaner off/on/off, and delay reporting on supervised -equipment, recording only sanitized results. +behavior, validate status and guarded write/readback behavior on supervised +equipment, recording only sanitized results. Do not change live heat settings +solely for an automated deployment check. diff --git a/docs/protocol.md b/docs/protocol.md index c594140..5e902c9 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -1,8 +1,13 @@ # Protocol notes `poolctl` uses `screenlogicpy` for adapter discovery, connection, state refresh, -and cleaner-circuit writes. The adapter is normally discovered by LAN broadcast -and cached outside the repository. +cleaner-circuit writes, heat-mode writes, and heat-setpoint writes. The adapter +is normally discovered by LAN broadcast and cached outside the repository. + +Heat control uses the dependency's public `async_set_heat_mode` and +`async_set_heat_temp` methods. `poolctl` resolves an exact body, validates the +controller-provided temperature range, performs the write, refreshes state, and +returns both pre-write and post-write body data. Delay cancellation is implemented in `poolctl/protocol.py` because the pinned dependency does not expose that request as a public helper. The request opcode diff --git a/docs/roadmap.md b/docs/roadmap.md index 4b9180d..41f7ab5 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -7,6 +7,7 @@ - JSON and raw diagnostic output - guarded cleaner on/off with delay handling and post-write readback - guarded delay cancellation +- guarded pool/spa heat mode and setpoint control with post-write readback ## Next diff --git a/justfile b/justfile index f7c5625..3a966bb 100644 --- a/justfile +++ b/justfile @@ -26,6 +26,7 @@ test: test-integration: poolctl status --help >/dev/null + poolctl heat status --help >/dev/null test-all: just test diff --git a/poolctl/cli.py b/poolctl/cli.py index f89b553..e9a3695 100644 --- a/poolctl/cli.py +++ b/poolctl/cli.py @@ -4,7 +4,16 @@ import asyncio import json -from poolctl.control import cancel_delay, cleaner_status, delay_status, set_circuit_state +from poolctl.control import ( + HEAT_MODES, + cancel_delay, + cleaner_status, + delay_status, + heat_status, + set_circuit_state, + set_heat_mode, + set_heat_temp, +) from poolctl.gateway import discover_adapter, fetch_status from poolctl.render import render_bodies, render_circuits, render_pumps, render_status, summarize @@ -42,6 +51,26 @@ async def async_main() -> None: ) delay_cancel_parser.add_argument("--json", action="store_true") + heat_parser = subparsers.add_parser("heat") + heat_sub = heat_parser.add_subparsers(dest="heat_command", required=True) + heat_status_parser = heat_sub.add_parser("status") + heat_status_parser.add_argument("body", nargs="?", help="body name or numeric ID") + heat_status_parser.add_argument("--json", action="store_true") + heat_set_parser = heat_sub.add_parser("set") + heat_set_parser.add_argument("body", help="body name or numeric ID") + heat_set_parser.add_argument("mode", choices=tuple(HEAT_MODES)) + heat_set_parser.add_argument( + "--yes", action="store_true", help="actually perform the hardware write" + ) + heat_set_parser.add_argument("--json", action="store_true") + heat_temp_parser = heat_sub.add_parser("temp") + heat_temp_parser.add_argument("body", help="body name or numeric ID") + heat_temp_parser.add_argument("temperature", type=int) + heat_temp_parser.add_argument( + "--yes", action="store_true", help="actually perform the hardware write" + ) + heat_temp_parser.add_argument("--json", action="store_true") + args = parser.parse_args() if args.command == "discover": @@ -119,6 +148,41 @@ async def async_main() -> None: print(f"Delays: cleaner={result['cleaner']} pool={result['pool']} spa={result['spa']}") return + if args.command == "heat": + if args.heat_command == "status": + status = await heat_status(args.body, args.host) + if args.json: + print(json.dumps(status, indent=2, sort_keys=True, default=str)) + elif args.body: + print( + f"{status['name']}: {status['temp_f']}°F, heat_mode={status['heat_mode']}, " + f"setpoint={status['heat_setpoint_f']}°F, heat_state={status['heat_state']}" + ) + else: + print(render_bodies({"bodies": status})) + return + + if not args.yes: + if args.heat_command == "set": + example = f"poolctl heat set {args.body} {args.mode} --yes" + else: + example = f"poolctl heat temp {args.body} {args.temperature} --yes" + raise SystemExit(f"Refusing to change heat settings without --yes. Run: {example}") + + if args.heat_command == "set": + result = await set_heat_mode(args.body, args.mode, args.host) + else: + result = await set_heat_temp(args.body, args.temperature, args.host) + if args.json: + print(json.dumps(result, indent=2, sort_keys=True, default=str)) + else: + status = result["status_after"] + print( + f"{status['name']}: {status['temp_f']}°F, heat_mode={status['heat_mode']}, " + f"setpoint={status['heat_setpoint_f']}°F, heat_state={status['heat_state']}" + ) + return + payload = await fetch_status(args.host) if args.command == "status" and args.raw: print(json.dumps(payload, indent=2, sort_keys=True, default=str)) diff --git a/poolctl/control.py b/poolctl/control.py index 1f51d82..ca1a113 100644 --- a/poolctl/control.py +++ b/poolctl/control.py @@ -8,6 +8,13 @@ from poolctl.protocol import async_request_cancel_delay from poolctl.render import summarize +HEAT_MODES = { + "off": 0, + "solar": 1, + "solar-preferred": 2, + "heater": 3, +} + def normalize_name(value: str) -> str: return " ".join(value.strip().lower().split()) @@ -27,6 +34,21 @@ def find_circuit(summary: dict[str, Any], query: str) -> dict[str, Any]: raise ValueError(f"Ambiguous circuit name {query!r}: {names}") +def find_body(summary: dict[str, Any], query: str) -> dict[str, Any]: + q = normalize_name(query) + matches = [ + body + for body in summary["bodies"].values() + if q in {normalize_name(str(body.get("name", ""))), str(body.get("id"))} + ] + if len(matches) == 1: + return matches[0] + if not matches: + raise ValueError(f"No body matched {query!r}") + names = ", ".join(str(body.get("name") or body.get("id")) for body in matches) + raise ValueError(f"Ambiguous body {query!r}: {names}") + + def extract_delay(data: dict[str, Any]) -> dict[str, int | None]: sensors = data.get("controller", {}).get("sensor", {}) return { @@ -51,6 +73,64 @@ async def delay_status(host: str | None = None) -> dict[str, int | None]: return extract_delay(payload["data"]) +async def heat_status(body_name: str | None = None, host: str | None = None) -> dict[str, Any]: + summary = summarize(await fetch_status(host)) + if body_name is None: + return summary["bodies"] + return find_body(summary, body_name) + + +async def set_heat_mode(body_name: str, mode: str, host: str | None = None) -> dict[str, Any]: + if mode not in HEAT_MODES: + choices = ", ".join(HEAT_MODES) + raise ValueError(f"Invalid heat mode {mode!r}; choose from: {choices}") + + adapter = await resolve_adapter(host) + gateway = ScreenLogicGateway() + await gateway.async_connect(**adapter) + try: + await gateway.async_update() + before = find_body(summarize({"adapter": adapter, "data": gateway.get_data()}), body_name) + await gateway.async_set_heat_mode(int(before["id"]), HEAT_MODES[mode]) + await gateway.async_update() + after = find_body(summarize({"adapter": adapter, "data": gateway.get_data()}), body_name) + return { + "requested": {"body": body_name, "mode": mode}, + "status_before": before, + "status_after": after, + } + finally: + await gateway.async_disconnect() + + +async def set_heat_temp( + body_name: str, temperature: int, host: str | None = None +) -> dict[str, Any]: + adapter = await resolve_adapter(host) + gateway = ScreenLogicGateway() + await gateway.async_connect(**adapter) + try: + await gateway.async_update() + before = find_body(summarize({"adapter": adapter, "data": gateway.get_data()}), body_name) + minimum = before.get("min_setpoint_f") + maximum = before.get("max_setpoint_f") + if isinstance(minimum, (int, float)) and isinstance(maximum, (int, float)): + if not minimum <= temperature <= maximum: + raise ValueError( + f"Temperature for {before['name']} must be between {minimum} and {maximum}°F" + ) + await gateway.async_set_heat_temp(int(before["id"]), temperature) + await gateway.async_update() + after = find_body(summarize({"adapter": adapter, "data": gateway.get_data()}), body_name) + return { + "requested": {"body": body_name, "temperature_f": temperature}, + "status_before": before, + "status_after": after, + } + finally: + await gateway.async_disconnect() + + async def set_circuit_state( circuit_name: str, enabled: bool, host: str | None = None ) -> dict[str, Any]: diff --git a/poolctl/render.py b/poolctl/render.py index 9e00986..b5aa3c1 100644 --- a/poolctl/render.py +++ b/poolctl/render.py @@ -27,12 +27,20 @@ def summarize(payload: dict[str, Any]) -> dict[str, Any]: body_summary = {} for body_id, body in bodies.items(): + resolved_id = body.get("body_type", body_id) + try: + resolved_id = int(resolved_id) + except (TypeError, ValueError): + pass body_summary[body_id] = { + "id": resolved_id, "name": body.get("name"), "temp_f": body.get("last_temperature", {}).get("value"), "heat_mode": enum_value(body.get("heat_mode")), "heat_setpoint_f": body.get("heat_setpoint", {}).get("value"), "heat_state": enum_value(body.get("heat_state")), + "min_setpoint_f": body.get("min_setpoint"), + "max_setpoint_f": body.get("max_setpoint"), } circuit_summary = [] diff --git a/pyproject.toml b/pyproject.toml index 0d6804c..a0e0896 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "poolctl" -version = "0.2.0" +version = "0.3.0" description = "Terminal-first Pentair ScreenLogic inspection and guarded control" readme = "README.md" requires-python = ">=3.11" diff --git a/tests/test_cli.py b/tests/test_cli.py new file mode 100644 index 0000000..75087e8 --- /dev/null +++ b/tests/test_cli.py @@ -0,0 +1,23 @@ +import subprocess +import sys + + +def run_cli(*args): + return subprocess.run( + [sys.executable, "-m", "poolctl.cli", *args], + check=False, + capture_output=True, + text=True, + ) + + +def test_heat_command_help_is_available_without_hardware(): + result = run_cli("heat", "status", "--help") + assert result.returncode == 0 + assert "body name or numeric ID" in result.stdout + + +def test_heat_writes_require_yes_before_hardware_access(): + result = run_cli("heat", "set", "pool", "solar-preferred") + assert result.returncode != 0 + assert "Refusing to change heat settings without --yes" in result.stderr diff --git a/tests/test_control.py b/tests/test_control.py index 812d0b0..648df1f 100644 --- a/tests/test_control.py +++ b/tests/test_control.py @@ -1,6 +1,9 @@ +import asyncio + import pytest -from poolctl.control import extract_delay, find_circuit, normalize_name +from poolctl import control +from poolctl.control import HEAT_MODES, extract_delay, find_body, find_circuit, normalize_name from poolctl.protocol import CANCEL_DELAY_QUERY @@ -49,3 +52,125 @@ def test_extract_delay(): def test_cancel_delay_opcode_is_pinned(): assert CANCEL_DELAY_QUERY == 12580 + + +def heat_data(): + return { + "controller": {"sensor": {}}, + "body": { + "0": { + "body_type": 0, + "name": "Pool", + "last_temperature": {"value": 79}, + "heat_mode": { + "value": 1, + "enum_options": ["Off", "Solar", "Solar Preferred", "Heater"], + }, + "heat_setpoint": {"value": 88}, + "heat_state": {"value": 1, "enum_options": ["Off", "Solar", "Heater"]}, + "min_setpoint": 40, + "max_setpoint": 104, + }, + "1": { + "body_type": 1, + "name": "Spa", + "last_temperature": {"value": 70}, + "heat_mode": { + "value": 0, + "enum_options": ["Off", "Solar", "Solar Preferred", "Heater"], + }, + "heat_setpoint": {"value": 100}, + "heat_state": {"value": 0, "enum_options": ["Off", "Solar", "Heater"]}, + "min_setpoint": 40, + "max_setpoint": 104, + }, + }, + "circuit": {}, + "pump": {}, + } + + +def heat_summary(): + return control.summarize({"adapter": {}, "data": heat_data()}) + + +def test_find_body_accepts_exact_name_or_id(): + assert find_body(heat_summary(), "pool")["id"] == 0 + assert find_body(heat_summary(), "1")["name"] == "Spa" + with pytest.raises(ValueError, match="No body matched"): + find_body(heat_summary(), "hot tub") + + +def test_heat_mode_contract_is_stable(): + assert HEAT_MODES == {"off": 0, "solar": 1, "solar-preferred": 2, "heater": 3} + + +def test_heat_status_returns_all_bodies_or_one_body(monkeypatch): + async def fetch_status(host=None): + return {"adapter": {}, "data": heat_data()} + + monkeypatch.setattr(control, "fetch_status", fetch_status) + bodies = asyncio.run(control.heat_status()) + assert set(bodies) == {"0", "1"} + assert asyncio.run(control.heat_status("pool"))["heat_setpoint_f"] == 88 + + +class FakeHeatGateway: + instances = [] + + def __init__(self): + self.data = heat_data() + self.mode_writes = [] + self.temp_writes = [] + self.instances.append(self) + + async def async_connect(self, **adapter): + self.adapter = adapter + + async def async_update(self): + return None + + def get_data(self): + return self.data + + async def async_set_heat_mode(self, body, mode): + self.mode_writes.append((body, mode)) + self.data["body"][str(body)]["heat_mode"]["value"] = mode + + async def async_set_heat_temp(self, body, temperature): + self.temp_writes.append((body, temperature)) + self.data["body"][str(body)]["heat_setpoint"]["value"] = temperature + + async def async_disconnect(self): + return None + + +def install_fake_heat_gateway(monkeypatch): + FakeHeatGateway.instances.clear() + + async def resolve_adapter(host=None): + return {"ip": host or "192.0.2.10", "port": 80} + + monkeypatch.setattr(control, "ScreenLogicGateway", FakeHeatGateway) + monkeypatch.setattr(control, "resolve_adapter", resolve_adapter) + + +def test_set_heat_mode_reports_post_write_state(monkeypatch): + install_fake_heat_gateway(monkeypatch) + result = asyncio.run(control.set_heat_mode("pool", "solar-preferred")) + gateway = FakeHeatGateway.instances[0] + assert gateway.mode_writes == [(0, 2)] + assert result["status_before"]["heat_mode"] == "Solar" + assert result["status_after"]["heat_mode"] == "Solar Preferred" + + +def test_set_heat_temp_validates_limits_and_reports_state(monkeypatch): + install_fake_heat_gateway(monkeypatch) + result = asyncio.run(control.set_heat_temp("pool", 90)) + gateway = FakeHeatGateway.instances[0] + assert gateway.temp_writes == [(0, 90)] + assert result["status_after"]["heat_setpoint_f"] == 90 + + with pytest.raises(ValueError, match="between 40 and 104"): + asyncio.run(control.set_heat_temp("pool", 105)) + assert FakeHeatGateway.instances[1].temp_writes == [] diff --git a/tests/test_render.py b/tests/test_render.py index 17ae67b..8b37abe 100644 --- a/tests/test_render.py +++ b/tests/test_render.py @@ -22,18 +22,24 @@ def sample_payload(): }, "body": { "0": { + "body_type": 0, "name": "Pool", "last_temperature": {"value": 66}, "heat_mode": {"value": 1, "enum_options": ["Off", "Solar"]}, "heat_setpoint": {"value": 85}, "heat_state": {"value": 0, "enum_options": ["Off", "Heater"]}, + "min_setpoint": 40, + "max_setpoint": 104, }, "1": { + "body_type": 1, "name": "Spa", "last_temperature": {"value": 65}, "heat_mode": {"value": 0, "enum_options": ["Off", "Solar"]}, "heat_setpoint": {"value": 100}, "heat_state": {"value": 0, "enum_options": ["Off", "Heater"]}, + "min_setpoint": 40, + "max_setpoint": 104, }, }, "circuit": { @@ -82,6 +88,9 @@ def test_summarize(): assert summary["air_temp_f"] == 64 assert summary["salt_ppm"] == 2750 assert summary["bodies"]["0"]["heat_mode"] == "Solar" + assert summary["bodies"]["0"]["id"] == 0 + assert summary["bodies"]["0"]["min_setpoint_f"] == 40 + assert summary["bodies"]["0"]["max_setpoint_f"] == 104 assert summary["circuits"][1]["name"] == "Pool" assert summary["pumps"]["0"]["rpm"] == 2750