diff --git a/README.md b/README.md index a85442b..d491adc 100644 --- a/README.md +++ b/README.md @@ -11,28 +11,29 @@ greenfix is released under the MIT License. greenfix is designed for Linux desktop sessions where `xrandr` can control the active display output. - X11: best supported. -- Wayland: may not work because most Wayland sessions do not allow `xrandr` to change display settings. +- Wayland (COSMIC and others): supported for **external monitors that speak DDC/CI**. greenfix shells out to `ddcutil` and adjusts VCP brightness (`0x10`) plus per-channel RGB gain (`0x16/0x18/0x1A`). Laptop panels are not supported under Wayland because they don't expose DDC/CI and cosmic-comp does not yet expose `wlr-gamma-control-unstable-v1` (tracking [pop-os/cosmic-comp#2059](https://github.com/pop-os/cosmic-comp/issues/2059)). - Root access: not required. -If `XDG_SESSION_TYPE` is `wayland`, the app warns that display changes may not work but still lets you try. +If `XDG_SESSION_TYPE` is `wayland`, greenfix uses the DDC/CI backend automatically. If no DDC/CI-capable monitors are detected, the UI says so and explains the limitation. ## Requirements - Python 3.10 or newer. - `xrandr`, usually provided by `x11-xserver-utils`, `xorg-xrandr`, or a similar package. - GTK 3 and PyGObject for the desktop UI. +- `ddcutil` (only on Wayland; not needed for X11). Your user must be in the `i2c` group for `ddcutil` to access `/dev/i2c-*` without sudo. Common dependency packages: ```bash # Debian, Ubuntu, Pop!_OS -sudo apt install python3-gi gir1.2-gtk-3.0 x11-xserver-utils +sudo apt install python3-gi gir1.2-gtk-3.0 x11-xserver-utils ddcutil # Fedora -sudo dnf install python3-gobject gtk3 xrandr +sudo dnf install python3-gobject gtk3 xrandr ddcutil # Arch Linux -sudo pacman -S python-gobject gtk3 xorg-xrandr +sudo pacman -S python-gobject gtk3 xorg-xrandr ddcutil ``` The greenfix installer does not run a system package manager. If Python or `xrandr` is missing, it prints suggestions and exits. If GTK/PyGObject is missing, it prints suggestions and still installs the CLI launcher so non-UI commands can work. diff --git a/docs/superpowers/plans/2026-06-03-wayland-ddc-backend.md b/docs/superpowers/plans/2026-06-03-wayland-ddc-backend.md new file mode 100644 index 0000000..1ff6d40 --- /dev/null +++ b/docs/superpowers/plans/2026-06-03-wayland-ddc-backend.md @@ -0,0 +1,1890 @@ +# Wayland DDC/CI Backend Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. **Every code-writing turn must invoke `superpowers:test-driven-development` before writing code and `superpowers:simplify` before every commit. This applies to subagents too — if you dispatch one, its prompt must include those invocations explicitly.** + +**Goal:** Add a second display backend that drives external monitors via `ddcutil` on Wayland (COSMIC and others), behind a backend abstraction. Keep the `xrandr` backend unchanged for X11 users. + +**Architecture:** Introduce `greenfix/display.py` defining a `Backend` Protocol and a `select_backend()` selector. `greenfix/xrandr.py` gains an `XrandrBackend` adapter over its existing functions; `greenfix/ddc.py` is new and wraps `ddcutil`. `cli.py` and `ui.py` call backend methods through `display`, not the backend modules directly. + +**Tech Stack:** Python 3.10+, GTK 3 via PyGObject, `unittest`, `unittest.mock`. New: `ddcutil` (external binary). + +**Spec:** `docs/superpowers/specs/2026-06-03-wayland-ddc-backend-design.md` + +--- + +## File Plan + +- Create: `greenfix/display.py` — Backend protocol, SliderSpec, Output, select_backend. +- Create: `greenfix/ddc.py` — DDC/CI backend (parsers, commands, DdcBackend, validate). +- Create: `tests/test_display.py` — backend selection and dataclass tests. +- Create: `tests/test_ddc.py` — DDC parsing, command construction, backend behavior. +- Modify: `greenfix/xrandr.py` — add `XrandrBackend` class, `validate()` free function, `SLIDER_DEBOUNCE_MS` constant. +- Modify: `greenfix/config.py` — add `backend` field, dispatch validation, load/save with backend tag, optional `active_backend` filter. +- Modify: `greenfix/cli.py` — route through `display.select_backend()`, drop `warn_if_wayland`. +- Modify: `greenfix/ui.py` — build sliders from `backend.slider_spec`, gate live preview on `backend.live_preview`, new status messages. +- Modify: `tests/test_xrandr.py` — cover XrandrBackend adapter and `validate()`. +- Modify: `tests/test_config.py` — cover backend tag round-trip and mismatch behavior. +- Modify: `tests/test_cli.py` — patch the backend instead of `xrandr.*` directly. +- Modify: `tests/test_ui.py` — pass `backend_id` and cover DDC empty case. +- Modify: `README.md` — describe Wayland (DDC/CI) support and laptop-panel limitation. + +--- + +## Task 1: Backend protocol + value types + +**Files:** +- Create: `greenfix/display.py` +- Create: `tests/test_display.py` + +- [ ] **Step 1: Write the failing test** + +`tests/test_display.py`: +```python +import unittest + +from greenfix import display + + +class SliderSpecTests(unittest.TestCase): + def test_holds_fields(self) -> None: + spec = display.SliderSpec(minimum=0.6, maximum=1.8, neutral=1.0, step=0.01, digits=2) + + self.assertEqual(spec.minimum, 0.6) + self.assertEqual(spec.maximum, 1.8) + self.assertEqual(spec.neutral, 1.0) + self.assertEqual(spec.step, 0.01) + self.assertEqual(spec.digits, 2) + + def test_is_frozen(self) -> None: + spec = display.SliderSpec(minimum=0.0, maximum=1.0, neutral=1.0, step=0.01, digits=2) + + with self.assertRaises(Exception): + spec.minimum = 2.0 # type: ignore[misc] + + +class OutputTests(unittest.TestCase): + def test_holds_id_and_label(self) -> None: + out = display.Output(id="eDP-1", label="eDP-1") + + self.assertEqual(out.id, "eDP-1") + self.assertEqual(out.label, "eDP-1") + + +if __name__ == "__main__": + unittest.main() +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `python3 -m unittest tests.test_display -v` +Expected: FAIL with `ModuleNotFoundError: No module named 'greenfix.display'`. + +- [ ] **Step 3: Create `greenfix/display.py`** + +```python +"""Display backend abstraction shared by xrandr and ddc backends.""" + +from __future__ import annotations + +import os +from dataclasses import dataclass +from typing import Protocol, runtime_checkable + + +@dataclass(frozen=True) +class SliderSpec: + minimum: float + maximum: float + neutral: float + step: float + digits: int + + +@dataclass(frozen=True) +class Output: + id: str + label: str + + +@runtime_checkable +class Backend(Protocol): + backend_id: str + live_preview: bool + + def query_outputs(self) -> list[Output]: ... + def slider_spec(self, channel: str, output_id: str) -> SliderSpec: ... + def neutral_settings(self, output_id: str): + ... + def apply(self, settings) -> None: ... + def reset(self, output_id: str) -> None: ... +``` + +(`select_backend` is added in Task 4 once a backend exists to return.) + +- [ ] **Step 4: Run test to verify it passes** + +Run: `python3 -m unittest tests.test_display -v` +Expected: PASS, 3 tests. + +- [ ] **Step 5: Commit** + +```bash +git add greenfix/display.py tests/test_display.py +git commit -m "feat: add backend protocol and slider spec primitives" +``` + +--- + +## Task 2: Add `backend` field to Settings + +This task only adds the field and load/save plumbing. Validation stays as-is (still calling `xrandr.validate_*` helpers directly). Task 3 switches validation to dispatched form. + +**Files:** +- Modify: `greenfix/config.py` +- Modify: `tests/test_config.py` + +- [ ] **Step 1: Write the failing tests** + +Append to `tests/test_config.py`: +```python +class ConfigBackendFieldTests(unittest.TestCase): + def test_settings_default_backend_is_xrandr(self) -> None: + self.assertEqual(config.Settings(output="eDP-1").backend, "xrandr") + + def test_settings_accepts_explicit_backend(self) -> None: + self.assertEqual(config.Settings(output="eDP-1", backend="xrandr").backend, "xrandr") + + def test_save_includes_backend_field(self) -> None: + with tempfile.TemporaryDirectory() as directory: + path = Path(directory) / "settings.json" + config.save_settings(config.Settings(output="eDP-1", red_gamma=1.2), path) + payload = json.loads(path.read_text(encoding="utf-8")) + + self.assertEqual(payload["backend"], "xrandr") + + def test_load_untagged_file_defaults_to_xrandr(self) -> None: + with tempfile.TemporaryDirectory() as directory: + path = Path(directory) / "settings.json" + path.write_text(json.dumps({"output": "eDP-1", "red_gamma": 1.1}), encoding="utf-8") + loaded = config.load_settings(path) + + self.assertIsNotNone(loaded) + self.assertEqual(loaded.backend, "xrandr") + self.assertEqual(loaded.red_gamma, 1.1) +``` + +Replace the existing `test_saved_json_has_expected_keys` body to include the new key: +```python +def test_saved_json_has_expected_keys(self) -> None: + with tempfile.TemporaryDirectory() as directory: + path = Path(directory) / "settings.json" + + config.save_settings(config.Settings(output="eDP-1", red_gamma=1.2), path) + + self.assertEqual( + json.loads(path.read_text(encoding="utf-8")), + { + "output": "eDP-1", + "backend": "xrandr", + "red_gamma": 1.2, + "green_gamma": 1.0, + "blue_gamma": 1.0, + "brightness": 1.0, + }, + ) +``` + +Replace `test_save_and_load_settings_round_trip` so the constructed Settings includes `backend="xrandr"` explicitly (it's the default, but being explicit guards against silent regressions): +```python +def test_save_and_load_settings_round_trip(self) -> None: + with tempfile.TemporaryDirectory() as directory: + path = Path(directory) / "greenfix" / "settings.json" + settings = config.Settings( + output="eDP-1", + backend="xrandr", + red_gamma=1.2, + green_gamma=1.0, + blue_gamma=1.25, + brightness=1.0, + ) + + config.save_settings(settings, path) + + self.assertEqual(config.load_settings(path), settings) +``` + +- [ ] **Step 2: Run tests to verify failure** + +Run: `python3 -m unittest tests.test_config -v` +Expected: FAIL with `TypeError: __init__() got an unexpected keyword argument 'backend'`. + +- [ ] **Step 3: Update `greenfix/config.py`** + +Add `backend: str = "xrandr"` to the dataclass (immediately after `output`), and update `load_settings` to read it. The rest of the file is unchanged. + +```python +@dataclass(frozen=True) +class Settings: + output: str + backend: str = "xrandr" + red_gamma: float = xrandr.NEUTRAL_GAMMA + green_gamma: float = xrandr.NEUTRAL_GAMMA + blue_gamma: float = xrandr.NEUTRAL_GAMMA + brightness: float = xrandr.NEUTRAL_BRIGHTNESS + + def __post_init__(self) -> None: + try: + xrandr.validate_output_name(self.output) + xrandr.validate_gamma_value("red gamma", self.red_gamma) + xrandr.validate_gamma_value("green gamma", self.green_gamma) + xrandr.validate_gamma_value("blue gamma", self.blue_gamma) + xrandr.validate_brightness_value(self.brightness) + except xrandr.XrandrError as exc: + raise ConfigError(str(exc)) from exc +``` + +And in `load_settings`, add the backend read inside the existing `return Settings(...)`: +```python +return Settings( + output=output, + backend=str(data.get("backend", "xrandr")), + red_gamma=float(data.get("red_gamma", xrandr.NEUTRAL_GAMMA)), + green_gamma=float(data.get("green_gamma", xrandr.NEUTRAL_GAMMA)), + blue_gamma=float(data.get("blue_gamma", xrandr.NEUTRAL_GAMMA)), + brightness=float(data.get("brightness", xrandr.NEUTRAL_BRIGHTNESS)), +) +``` + +- [ ] **Step 4: Run tests to verify pass** + +Run: `python3 -m unittest discover -s tests -v` +Expected: All pass. + +- [ ] **Step 5: Commit** + +```bash +git add greenfix/config.py tests/test_config.py +git commit -m "feat: tag settings with backend identifier" +``` + +--- + +## Task 3: XrandrBackend adapter + dispatched validation + mismatch discard + +This task adds the `XrandrBackend` class, a module-level `validate()` free function on `xrandr`, switches `Settings.__post_init__` to dispatch by backend name, and adds an `active_backend` filter to `load_settings`. After this task, `config.py` no longer references `xrandr` constants directly except in the dataclass defaults (those stay numeric `1.0` in Task 6 once the UI fully owns neutral-derivation). + +**Files:** +- Modify: `greenfix/xrandr.py` +- Modify: `greenfix/config.py` +- Modify: `tests/test_xrandr.py` +- Modify: `tests/test_config.py` + +- [ ] **Step 1: Write the failing tests for `xrandr.XrandrBackend` and `xrandr.validate`** + +Append to `tests/test_xrandr.py`: +```python +from greenfix import config, display + + +class XrandrBackendTests(unittest.TestCase): + def test_backend_id_is_xrandr(self) -> None: + self.assertEqual(xrandr.XrandrBackend().backend_id, "xrandr") + + def test_live_preview_is_enabled(self) -> None: + self.assertTrue(xrandr.XrandrBackend().live_preview) + + def test_slider_spec_for_gamma_channels(self) -> None: + backend = xrandr.XrandrBackend() + for channel in ("red", "green", "blue"): + spec = backend.slider_spec(channel, "eDP-1") + self.assertEqual((spec.minimum, spec.maximum, spec.neutral), (0.60, 1.80, 1.0)) + self.assertEqual(spec.digits, 2) + + def test_slider_spec_for_brightness(self) -> None: + spec = xrandr.XrandrBackend().slider_spec("brightness", "eDP-1") + self.assertEqual((spec.minimum, spec.maximum, spec.neutral), (0.60, 1.20, 1.0)) + + def test_query_outputs_wraps_names(self) -> None: + with patch.object(xrandr, "query_connected_outputs", return_value=["eDP-1", "DP-1"]): + outputs = xrandr.XrandrBackend().query_outputs() + self.assertEqual(outputs, [display.Output("eDP-1", "eDP-1"), display.Output("DP-1", "DP-1")]) + + def test_neutral_settings_returns_unit_values(self) -> None: + settings = xrandr.XrandrBackend().neutral_settings("eDP-1") + self.assertEqual(settings.output, "eDP-1") + self.assertEqual(settings.backend, "xrandr") + self.assertEqual(settings.red_gamma, 1.0) + self.assertEqual(settings.brightness, 1.0) + + def test_apply_calls_apply_settings(self) -> None: + settings = config.Settings(output="eDP-1", backend="xrandr", red_gamma=1.1) + with patch.object(xrandr, "apply_settings") as apply_mock: + xrandr.XrandrBackend().apply(settings) + apply_mock.assert_called_once_with("eDP-1", 1.1, 1.0, 1.0, 1.0) + + def test_reset_calls_reset_output(self) -> None: + with patch.object(xrandr, "reset_output") as reset_mock: + xrandr.XrandrBackend().reset("eDP-1") + reset_mock.assert_called_once_with("eDP-1") + + +class XrandrValidateTests(unittest.TestCase): + def test_validate_accepts_neutral(self) -> None: + xrandr.validate(config.Settings(output="eDP-1", backend="xrandr")) + + def test_validate_rejects_gamma_out_of_range(self) -> None: + with self.assertRaisesRegex(xrandr.XrandrError, "red gamma"): + xrandr.validate(config.Settings(output="eDP-1", backend="xrandr", red_gamma=0.5)) + + def test_validate_rejects_bad_output(self) -> None: + with self.assertRaisesRegex(xrandr.XrandrError, "Invalid display output"): + xrandr.validate(config.Settings(output="bad;name", backend="xrandr")) +``` + +- [ ] **Step 2: Write the failing tests for config dispatch + active_backend** + +Append to `tests/test_config.py`: +```python +class ConfigDispatchTests(unittest.TestCase): + def test_post_init_dispatches_via_backend_module(self) -> None: + # xrandr backend rejects gamma 0.5 → ConfigError + with self.assertRaisesRegex(config.ConfigError, "red gamma"): + config.Settings(output="eDP-1", backend="xrandr", red_gamma=0.5) + + def test_post_init_unknown_backend_raises(self) -> None: + with self.assertRaisesRegex(config.ConfigError, "Unknown backend"): + config.Settings(output="x", backend="nope") + + def test_load_returns_none_when_active_backend_does_not_match(self) -> None: + with tempfile.TemporaryDirectory() as directory: + path = Path(directory) / "settings.json" + path.write_text( + json.dumps({"output": "ddc:4", "backend": "ddc", "red_gamma": 80}), + encoding="utf-8", + ) + self.assertIsNone(config.load_settings(path, active_backend="xrandr")) + + def test_load_returns_settings_when_active_backend_matches(self) -> None: + with tempfile.TemporaryDirectory() as directory: + path = Path(directory) / "settings.json" + payload = {"output": "eDP-1", "backend": "xrandr", "red_gamma": 1.1} + path.write_text(json.dumps(payload), encoding="utf-8") + + loaded = config.load_settings(path, active_backend="xrandr") + + self.assertIsNotNone(loaded) + self.assertEqual(loaded.backend, "xrandr") +``` + +- [ ] **Step 3: Run tests to verify failure** + +Run: `python3 -m unittest discover -s tests -v` +Expected: FAILs on `XrandrBackend`, `xrandr.validate`, `ConfigDispatchTests` (e.g. `AttributeError`, no `active_backend` kwarg). + +- [ ] **Step 4: Update `greenfix/xrandr.py`** + +Append to the existing module (keep all existing functions): +```python +SLIDER_DEBOUNCE_MS = 250 + + +def validate(settings) -> None: + """Validate Settings whose backend is 'xrandr'. Raises XrandrError.""" + validate_output_name(settings.output) + validate_gamma_value("red gamma", settings.red_gamma) + validate_gamma_value("green gamma", settings.green_gamma) + validate_gamma_value("blue gamma", settings.blue_gamma) + validate_brightness_value(settings.brightness) + + +class XrandrBackend: + backend_id = "xrandr" + live_preview = True + + def query_outputs(self): + from greenfix.display import Output + return [Output(id=name, label=name) for name in query_connected_outputs()] + + def slider_spec(self, channel, output_id): + from greenfix.display import SliderSpec + if channel == "brightness": + return SliderSpec(BRIGHTNESS_MIN, BRIGHTNESS_MAX, NEUTRAL_BRIGHTNESS, 0.01, 2) + return SliderSpec(GAMMA_MIN, GAMMA_MAX, NEUTRAL_GAMMA, 0.01, 2) + + def neutral_settings(self, output_id): + from greenfix.config import Settings + return Settings(output=output_id, backend="xrandr") + + def apply(self, settings) -> None: + apply_settings( + settings.output, + settings.red_gamma, + settings.green_gamma, + settings.blue_gamma, + settings.brightness, + ) + + def reset(self, output_id: str) -> None: + reset_output(output_id) +``` + +The deferred `from greenfix.display import ...` and `from greenfix.config import ...` imports inside methods avoid the circular import (config imports xrandr at module load). + +- [ ] **Step 5: Update `greenfix/config.py` to dispatch validation and accept `active_backend`** + +Rewrite `Settings.__post_init__`: +```python +def __post_init__(self) -> None: + import importlib + try: + module = importlib.import_module(f"greenfix.{self.backend}") + except ModuleNotFoundError as exc: + raise ConfigError(f"Unknown backend: {self.backend!r}") from exc + try: + module.validate(self) + except Exception as exc: + raise ConfigError(str(exc)) from exc +``` + +Update `load_settings` signature and mismatch-discard: +```python +def load_settings( + path: Path | None = None, + active_backend: str | None = None, +) -> Settings | None: + settings_path = path or default_settings_path() + try: + data = json.loads(settings_path.read_text(encoding="utf-8")) + if not isinstance(data, dict): + return None + output = data["output"] + if not isinstance(output, str): + return None + backend = str(data.get("backend", "xrandr")) + if active_backend is not None and backend != active_backend: + return None + return Settings( + output=output, + backend=backend, + red_gamma=float(data.get("red_gamma", xrandr.NEUTRAL_GAMMA)), + green_gamma=float(data.get("green_gamma", xrandr.NEUTRAL_GAMMA)), + blue_gamma=float(data.get("blue_gamma", xrandr.NEUTRAL_GAMMA)), + brightness=float(data.get("brightness", xrandr.NEUTRAL_BRIGHTNESS)), + ) + except (FileNotFoundError, json.JSONDecodeError, KeyError, TypeError, ValueError, ConfigError): + return None +``` + +- [ ] **Step 6: Run the full suite** + +Run: `python3 -m unittest discover -s tests -v` +Expected: All pass. + +- [ ] **Step 7: Commit** + +```bash +git add greenfix/xrandr.py greenfix/config.py tests/test_xrandr.py tests/test_config.py +git commit -m "feat: dispatch settings validation through backend adapter" +``` + +--- + +## Task 4: `select_backend()` — X11 path + +**Files:** +- Modify: `greenfix/display.py` +- Modify: `tests/test_display.py` + +- [ ] **Step 1: Write the failing tests** + +Append to `tests/test_display.py`: +```python +import os +from unittest.mock import patch + +from greenfix import xrandr + + +class SelectBackendTests(unittest.TestCase): + def test_returns_xrandr_backend_on_x11(self) -> None: + with patch.dict(os.environ, {"XDG_SESSION_TYPE": "x11"}, clear=False): + self.assertIsInstance(display.select_backend(), xrandr.XrandrBackend) + + def test_returns_xrandr_backend_when_session_type_missing(self) -> None: + env = {k: v for k, v in os.environ.items() if k != "XDG_SESSION_TYPE"} + with patch.dict(os.environ, env, clear=True): + self.assertIsInstance(display.select_backend(), xrandr.XrandrBackend) + + def test_raises_not_implemented_for_wayland_until_ddc_lands(self) -> None: + with patch.dict(os.environ, {"XDG_SESSION_TYPE": "wayland"}, clear=False): + with self.assertRaises(NotImplementedError): + display.select_backend() +``` + +- [ ] **Step 2: Run tests to verify failure** + +Run: `python3 -m unittest tests.test_display -v` +Expected: FAIL with `AttributeError: module 'greenfix.display' has no attribute 'select_backend'`. + +- [ ] **Step 3: Add `select_backend` to `greenfix/display.py`** + +Append: +```python +def select_backend() -> Backend: + session = os.environ.get("XDG_SESSION_TYPE", "").lower() + if session == "wayland": + raise NotImplementedError("DDC backend not yet implemented") + from greenfix.xrandr import XrandrBackend + return XrandrBackend() +``` + +- [ ] **Step 4: Run tests to verify pass** + +Run: `python3 -m unittest tests.test_display -v` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add greenfix/display.py tests/test_display.py +git commit -m "feat: select xrandr backend on x11 sessions" +``` + +--- + +## Task 5: CLI uses display abstraction + +The existing `tests/test_cli.py` patches `greenfix.cli.xrandr.*` directly. After this task the CLI talks to the backend via `display.select_backend()`, so the patches target the backend mock instead. + +**Files:** +- Modify: `greenfix/cli.py` +- Modify: `tests/test_cli.py` + +- [ ] **Step 1: Rewrite `tests/test_cli.py`** + +Replace the entire file with: +```python +import io +import unittest +from pathlib import Path +from unittest.mock import MagicMock, Mock, patch + +from greenfix import cli, config, display + + +def _backend(outputs=("eDP-1",), backend_id="xrandr"): + backend = MagicMock(spec_set=("backend_id", "live_preview", "query_outputs", "apply", "reset")) + backend.backend_id = backend_id + backend.live_preview = backend_id == "xrandr" + backend.query_outputs.return_value = [display.Output(id=o, label=o) for o in outputs] + return backend + + +class CliTests(unittest.TestCase): + def test_list_outputs_prints_labels(self) -> None: + backend = _backend(("eDP-1", "DP-1")) + stdout = io.StringIO() + + with patch.object(display, "select_backend", return_value=backend): + status = cli.run(["--list-outputs"], stdout=stdout) + + self.assertEqual(status, 0) + self.assertEqual(stdout.getvalue(), "eDP-1\nDP-1\n") + backend.query_outputs.assert_called_once_with() + + def test_apply_saved_applies_existing_settings(self) -> None: + backend = _backend() + settings = config.Settings(output="eDP-1", backend="xrandr", red_gamma=1.2) + stdout = io.StringIO() + + with patch.object(display, "select_backend", return_value=backend), \ + patch.object(config, "load_settings", return_value=settings) as load_mock: + status = cli.run(["--apply-saved"], stdout=stdout, settings_path=Path("settings.json")) + + self.assertEqual(status, 0) + self.assertIn("Applied saved settings for eDP-1.", stdout.getvalue()) + load_mock.assert_called_once_with(Path("settings.json"), active_backend="xrandr") + backend.apply.assert_called_once_with(settings) + + def test_apply_saved_without_settings_exits_gracefully(self) -> None: + backend = _backend() + stdout = io.StringIO() + + with patch.object(display, "select_backend", return_value=backend), \ + patch.object(config, "load_settings", return_value=None): + status = cli.run(["--apply-saved"], stdout=stdout) + + self.assertEqual(status, 0) + self.assertIn("No saved settings", stdout.getvalue()) + backend.apply.assert_not_called() + + def test_reset_uses_saved_output_when_no_output_supplied(self) -> None: + backend = _backend() + saved = config.Settings(output="eDP-1", backend="xrandr") + stdout = io.StringIO() + + with patch.object(display, "select_backend", return_value=backend), \ + patch.object(config, "load_settings", return_value=saved): + status = cli.run(["--reset"], stdout=stdout) + + self.assertEqual(status, 0) + backend.reset.assert_called_once_with("eDP-1") + + def test_reset_uses_explicit_output(self) -> None: + backend = _backend() + stdout = io.StringIO() + + with patch.object(display, "select_backend", return_value=backend): + status = cli.run(["--reset", "--output", "HDMI-1"], stdout=stdout) + + self.assertEqual(status, 0) + backend.reset.assert_called_once_with("HDMI-1") + + def test_reset_falls_back_to_first_output_when_no_saved(self) -> None: + backend = _backend(("HDMI-1", "DP-1")) + stdout = io.StringIO() + + with patch.object(display, "select_backend", return_value=backend), \ + patch.object(config, "load_settings", return_value=None): + status = cli.run(["--reset"], stdout=stdout) + + self.assertEqual(status, 0) + backend.reset.assert_called_once_with("HDMI-1") + + def test_explicit_apply_uses_provided_values(self) -> None: + backend = _backend() + stdout = io.StringIO() + + with patch.object(display, "select_backend", return_value=backend): + status = cli.run( + [ + "--output", "eDP-1", + "--red", "1.2", + "--green", "1.0", + "--blue", "1.25", + "--brightness", "1.0", + "--apply", + ], + stdout=stdout, + ) + + self.assertEqual(status, 0) + self.assertIn("Applied settings for eDP-1.", stdout.getvalue()) + backend.apply.assert_called_once() + applied = backend.apply.call_args.args[0] + self.assertEqual(applied.output, "eDP-1") + self.assertEqual(applied.backend, "xrandr") + self.assertEqual(applied.red_gamma, 1.2) + self.assertEqual(applied.blue_gamma, 1.25) + + def test_default_launches_ui(self) -> None: + backend = _backend() + launch_ui = Mock(return_value=0) + + with patch.object(display, "select_backend", return_value=backend): + status = cli.run([], launch_ui=launch_ui) + + self.assertEqual(status, 0) + launch_ui.assert_called_once_with() + + +if __name__ == "__main__": + unittest.main() +``` + +The `test_apply_saved_warns_on_wayland` test is deleted — `warn_if_wayland` is gone. + +- [ ] **Step 2: Run tests to verify failure** + +Run: `python3 -m unittest tests.test_cli -v` +Expected: FAIL — `cli.run` still calls `xrandr.*` directly, so the mock backend's methods are not invoked. + +- [ ] **Step 3: Rewrite `greenfix/cli.py`** + +```python +"""Command-line interface for greenfix.""" + +from __future__ import annotations + +import argparse +import sys +from pathlib import Path +from typing import Callable, Sequence, TextIO + +from greenfix import config, display, xrandr + +UiLauncher = Callable[[], int] + + +def run( + argv: Sequence[str] | None = None, + *, + stdout: TextIO = sys.stdout, + stderr: TextIO = sys.stderr, + launch_ui: UiLauncher | None = None, + settings_path: Path | None = None, +) -> int: + parser = build_parser() + args = parser.parse_args(argv) + + try: + backend = display.select_backend() + if args.list_outputs: + return list_outputs(backend, stdout) + if args.apply_saved: + return apply_saved(backend, stdout, settings_path) + if args.reset: + return reset_settings(backend, args.output, stdout, settings_path) + if args.apply: + return apply_explicit(backend, args, stdout) + if launch_ui is None: + from greenfix.ui import main as launch_ui + return launch_ui() + except (config.ConfigError, xrandr.XrandrError) as exc: + print(f"greenfix: {exc}", file=stderr) + return 1 + + +def build_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser( + prog="greenfix", + description="Apply display color-balance settings.", + ) + parser.add_argument("--apply-saved", action="store_true", help="apply saved settings and exit") + parser.add_argument("--reset", action="store_true", help="reset the selected or saved output") + parser.add_argument("--list-outputs", action="store_true", help="list connected outputs") + parser.add_argument("--apply", action="store_true", help="apply explicit CLI values and exit") + parser.add_argument("--output", help="output identifier, such as eDP-1 or ddc:4") + parser.add_argument("--red", type=float, default=1.0, help="red channel value") + parser.add_argument("--green", type=float, default=1.0, help="green channel value") + parser.add_argument("--blue", type=float, default=1.0, help="blue channel value") + parser.add_argument("--brightness", type=float, default=1.0, help="brightness value") + return parser + + +def list_outputs(backend, stdout: TextIO) -> int: + for output in backend.query_outputs(): + print(output.label, file=stdout) + return 0 + + +def apply_saved(backend, stdout: TextIO, settings_path: Path | None) -> int: + settings = config.load_settings(settings_path, active_backend=backend.backend_id) + if settings is None: + print("No saved settings found for the active backend.", file=stdout) + return 0 + backend.apply(settings) + print(f"Applied saved settings for {settings.output}.", file=stdout) + return 0 + + +def reset_settings(backend, output: str | None, stdout: TextIO, settings_path: Path | None) -> int: + target_output = output + if target_output is None: + saved = config.load_settings(settings_path, active_backend=backend.backend_id) + if saved is not None: + target_output = saved.output + if target_output is None: + outputs = backend.query_outputs() + if not outputs: + raise xrandr.XrandrError("No connected display outputs were detected.") + target_output = outputs[0].id + backend.reset(target_output) + print(f"Reset {target_output}.", file=stdout) + return 0 + + +def apply_explicit(backend, args: argparse.Namespace, stdout: TextIO) -> int: + if not args.output: + raise xrandr.XrandrError("--output is required with --apply.") + settings = config.Settings( + output=args.output, + backend=backend.backend_id, + red_gamma=args.red, + green_gamma=args.green, + blue_gamma=args.blue, + brightness=args.brightness, + ) + backend.apply(settings) + print(f"Applied settings for {args.output}.", file=stdout) + return 0 +``` + +`xrandr.XrandrError` is kept as a CLI-level exception for both backends to raise via — Task 11 will mention that `DdcError` should be added to this except clause if used in CLI paths. (Current plan keeps `DdcError` confined to the DDC backend module and is wrapped by the backend's caller; the CLI catches it via a follow-up step in Task 11.) + +- [ ] **Step 4: Run tests to verify pass** + +Run: `python3 -m unittest discover -s tests -v` +Expected: All pass. + +- [ ] **Step 5: Commit** + +```bash +git add greenfix/cli.py tests/test_cli.py +git commit -m "feat: route cli through display backend abstraction" +``` + +--- + +## Task 6: UI uses display abstraction + backend-aware sliders + +**Files:** +- Modify: `greenfix/ui.py` +- Modify: `tests/test_ui.py` + +- [ ] **Step 1: Rewrite `tests/test_ui.py`** + +Replace the whole file with: +```python +import unittest + +from greenfix import display, ui + + +class StartupStatusMessageTests(unittest.TestCase): + def test_returns_none_when_outputs_exist(self) -> None: + self.assertIsNone( + ui.startup_status_message( + outputs=[display.Output("eDP-1", "eDP-1")], + startup_error=None, + backend_id="xrandr", + ) + ) + + def test_startup_error_passes_through(self) -> None: + message = ui.startup_status_message(outputs=[], startup_error="boom", backend_id="xrandr") + self.assertEqual(message, "boom") + + def test_xrandr_no_outputs_default_message(self) -> None: + message = ui.startup_status_message(outputs=[], startup_error=None, backend_id="xrandr") + self.assertIn("No connected display outputs", message) + + +if __name__ == "__main__": + unittest.main() +``` + +(Task 13 adds the DDC-empty branch test.) + +- [ ] **Step 2: Run tests to verify failure** + +Run: `python3 -m unittest tests.test_ui -v` +Expected: FAIL because `startup_status_message` does not yet take `backend_id`. + +- [ ] **Step 3: Update `greenfix/ui.py`** + +Update the imports at the top: +```python +from greenfix import config, display, xrandr +``` + +Replace `SLIDER_DEBOUNCE_MS = 250` with: +```python +from greenfix.xrandr import SLIDER_DEBOUNCE_MS +``` + +Rewrite `GreenfixWindow.__init__` (the parts that change shown below; line-prefixed numbers refer to the existing file): + +Replace lines 34–87 (from `self._debounce_id: int | None = None` through the end of `__init__`) with: +```python + self.backend = display.select_backend() + self._debounce_id: int | None = None + self.outputs = self._load_outputs() + self.saved_settings = config.load_settings(active_backend=self.backend.backend_id) + selected_output_id = self._initial_output_id() + initial_settings = self._initial_settings(selected_output_id) + spec_output = selected_output_id or (self.outputs[0].id if self.outputs else "") + + self.output_combo = Gtk.ComboBoxText() + for output in self.outputs: + self.output_combo.append_text(output.label) + if selected_output_id is not None: + for index, output in enumerate(self.outputs): + if output.id == selected_output_id: + self.output_combo.set_active(index) + break + self.output_combo.connect("changed", self._on_control_changed) + + red_spec = self.backend.slider_spec("red", spec_output) if self.outputs else display.SliderSpec(0, 1, 1, 0.01, 2) + green_spec = self.backend.slider_spec("green", spec_output) if self.outputs else red_spec + blue_spec = self.backend.slider_spec("blue", spec_output) if self.outputs else red_spec + bright_spec = self.backend.slider_spec("brightness", spec_output) if self.outputs else red_spec + + self.red_scale, self.red_value = self._create_scale(initial_settings.red_gamma, red_spec) + self.green_scale, self.green_value = self._create_scale(initial_settings.green_gamma, green_spec) + self.blue_scale, self.blue_value = self._create_scale(initial_settings.blue_gamma, blue_spec) + self.brightness_scale, self.brightness_value = self._create_scale(initial_settings.brightness, bright_spec) + + self.status_label = Gtk.Label() + self.status_label.set_xalign(0) + self.status_label.set_line_wrap(True) + + root = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=10) + self.add(root) + root.pack_start(self._output_row(), False, False, 0) + root.pack_start(self._slider_row("Red", self.red_scale, self.red_value), False, False, 0) + root.pack_start(self._slider_row("Green", self.green_scale, self.green_value), False, False, 0) + root.pack_start(self._slider_row("Blue", self.blue_scale, self.blue_value), False, False, 0) + root.pack_start(self._slider_row("Brightness", self.brightness_scale, self.brightness_value), False, False, 0) + root.pack_start(self._button_row(), False, False, 0) + root.pack_start(self.status_label, False, False, 0) + + startup_message = startup_status_message( + outputs=self.outputs, + startup_error=getattr(self, "_startup_error", None), + backend_id=self.backend.backend_id, + ) + if startup_message is not None: + self._set_status(startup_message) +``` + +Replace `_load_outputs`, `_initial_output`, and `_initial_settings`: +```python + def _load_outputs(self): + try: + return self.backend.query_outputs() + except xrandr.XrandrError as exc: + self._startup_error = str(exc) + return [] + + def _initial_output_id(self): + if self.saved_settings: + for output in self.outputs: + if output.id == self.saved_settings.output: + return output.id + for output in self.outputs: + if output.id == "eDP-1": + return output.id + return self.outputs[0].id if self.outputs else None + + def _initial_settings(self, output_id): + if output_id is None: + # No outputs available; use neutral display values so the sliders show + # something. The Apply button checks for a selected output before doing + # anything with these values. + from types import SimpleNamespace + return SimpleNamespace(red_gamma=1.0, green_gamma=1.0, blue_gamma=1.0, brightness=1.0) + if self.saved_settings and self.saved_settings.output == output_id: + return self.saved_settings + return self.backend.neutral_settings(output_id) +``` + +Replace `_create_scale` and `_on_scale_changed` so they honor `SliderSpec` and the per-backend live-preview policy: +```python + def _create_scale(self, value, spec): + adjustment = Gtk.Adjustment( + value=value, + lower=spec.minimum, + upper=spec.maximum, + step_increment=spec.step, + page_increment=max(spec.step * 5, spec.step), + page_size=0, + ) + scale = Gtk.Scale(orientation=Gtk.Orientation.HORIZONTAL, adjustment=adjustment) + scale.set_digits(spec.digits) + scale.set_hexpand(True) + fmt = f"{{:.{spec.digits}f}}" + value_label = Gtk.Label(label=fmt.format(value)) + scale.connect("value-changed", self._on_scale_changed, value_label, spec.digits) + return scale, value_label + + def _on_scale_changed(self, scale, value_label, digits) -> None: + fmt = f"{{:.{digits}f}}" + value_label.set_text(fmt.format(scale.get_value())) + if self.backend.live_preview: + self._schedule_preview() +``` + +Update `_current_output` to return the output ID (not the label): +```python + def _current_output(self): + index = self.output_combo.get_active() + if index < 0 or index >= len(self.outputs): + return None + return self.outputs[index].id +``` + +Update `_current_settings`, `_apply_current`, `_on_reset_clicked` to use the backend: +```python + def _current_settings(self): + output_id = self._current_output() + if output_id is None: + raise xrandr.XrandrError("No display output is selected.") + return config.Settings( + output=output_id, + backend=self.backend.backend_id, + red_gamma=self.red_scale.get_value(), + green_gamma=self.green_scale.get_value(), + blue_gamma=self.blue_scale.get_value(), + brightness=self.brightness_scale.get_value(), + ) + + def _apply_current(self, success_message: str) -> None: + try: + settings = self._current_settings() + self.backend.apply(settings) + except (config.ConfigError, xrandr.XrandrError) as exc: + self._set_status(str(exc)) + return + self._set_status(success_message) + + def _on_reset_clicked(self, _button) -> None: + output_id = self._current_output() + if output_id is None: + self._set_status("No display output is selected.") + return + settings = self.backend.neutral_settings(output_id) + self._set_slider_values(settings) + try: + self.backend.reset(output_id) + except xrandr.XrandrError as exc: + self._set_status(str(exc)) + return + self._set_status(f"Reset {output_id}.") +``` + +Finally replace `startup_status_message`: +```python +def startup_status_message( + outputs: list, + startup_error: str | None, + backend_id: str, +) -> str | None: + if outputs: + return None + if startup_error: + return startup_error + return "No connected display outputs were detected." +``` + +(Task 13 adds the DDC-specific branch.) + +- [ ] **Step 4: Run the full suite** + +Run: `python3 -m unittest discover -s tests -v` +Expected: All pass. + +- [ ] **Step 5: Commit** + +```bash +git add greenfix/ui.py tests/test_ui.py +git commit -m "feat: drive ui sliders from active display backend" +``` + +--- + +## Task 7: DDC parsing primitives + +**Files:** +- Create: `greenfix/ddc.py` +- Create: `tests/test_ddc.py` + +- [ ] **Step 1: Write the failing tests** + +`tests/test_ddc.py`: +```python +import unittest + +from greenfix import ddc + + +DDCUTIL_DETECT = """Display 1 + I2C bus: /dev/i2c-4 + EDID synopsis: + Mfg id: DEL - Dell Inc. + Model: DELL U2723QE + Product code: 16505 (0x4079) + Serial number: ABC123 + Manufacture year: 2023, Week: 12 + VCP version: 2.2 + +Display 2 + I2C bus: /dev/i2c-7 + EDID synopsis: + Mfg id: BNQ + Model: BenQ GW2480 + Product code: 30000 (0x7530) + Serial number: XYZ789 + Manufacture year: 2021 + VCP version: 2.1 +""" + +DDCUTIL_GETVCP = """VCP code 0x10 (Brightness ): current value = 75, max value = 100 +VCP code 0x16 (Video gain: Red ): current value = 100, max value = 100 +VCP code 0x18 (Video gain: Green ): current value = 95, max value = 100 +VCP code 0x1a (Video gain: Blue ): current value = 100, max value = 100 +""" + + +class ParseDetectTests(unittest.TestCase): + def test_parses_two_displays(self) -> None: + displays = ddc.parse_detect(DDCUTIL_DETECT) + + self.assertEqual(len(displays), 2) + self.assertEqual(displays[0].bus, 4) + self.assertEqual(displays[0].model, "DELL U2723QE") + self.assertEqual(displays[1].bus, 7) + self.assertEqual(displays[1].model, "BenQ GW2480") + + def test_empty_input_returns_empty_list(self) -> None: + self.assertEqual(ddc.parse_detect(""), []) + + def test_garbage_returns_empty_list(self) -> None: + self.assertEqual(ddc.parse_detect("nothing useful"), []) + + +class ParseGetvcpTests(unittest.TestCase): + def test_parses_four_codes(self) -> None: + values = ddc.parse_getvcp(DDCUTIL_GETVCP) + + self.assertEqual(values[0x10], (75, 100)) + self.assertEqual(values[0x16], (100, 100)) + self.assertEqual(values[0x18], (95, 100)) + self.assertEqual(values[0x1a], (100, 100)) + + def test_missing_code_absent_from_result(self) -> None: + values = ddc.parse_getvcp( + "VCP code 0x10 (Brightness): current value = 50, max value = 100" + ) + + self.assertIn(0x10, values) + self.assertNotIn(0x16, values) + + +if __name__ == "__main__": + unittest.main() +``` + +- [ ] **Step 2: Run tests to verify failure** + +Run: `python3 -m unittest tests.test_ddc -v` +Expected: `ModuleNotFoundError: No module named 'greenfix.ddc'`. + +- [ ] **Step 3: Create `greenfix/ddc.py`** + +```python +"""DDC/CI display backend powered by ddcutil.""" + +from __future__ import annotations + +import re +from dataclasses import dataclass + + +class DdcError(RuntimeError): + """Raised when ddcutil cannot complete the requested operation.""" + + +@dataclass(frozen=True) +class DdcDisplay: + bus: int + model: str + + +_DISPLAY_HEADER_RE = re.compile(r"^Display\s+\d+\s*$") +_BUS_RE = re.compile(r"I2C bus:\s+/dev/i2c-(\d+)") +_MODEL_RE = re.compile(r"Model:\s+(.+)$") +_VCP_RE = re.compile( + r"VCP code 0x([0-9a-fA-F]+).*?current value =\s*(\d+).*?max value =\s*(\d+)" +) + + +def parse_detect(output: str) -> list[DdcDisplay]: + displays: list[DdcDisplay] = [] + current_bus: int | None = None + current_model: str | None = None + for line in output.splitlines(): + if _DISPLAY_HEADER_RE.match(line): + if current_bus is not None and current_model is not None: + displays.append(DdcDisplay(bus=current_bus, model=current_model)) + current_bus = None + current_model = None + continue + bus_match = _BUS_RE.search(line) + if bus_match: + current_bus = int(bus_match.group(1)) + continue + model_match = _MODEL_RE.search(line) + if model_match: + current_model = model_match.group(1).strip() + if current_bus is not None and current_model is not None: + displays.append(DdcDisplay(bus=current_bus, model=current_model)) + return displays + + +def parse_getvcp(output: str) -> dict[int, tuple[int, int]]: + values: dict[int, tuple[int, int]] = {} + for line in output.splitlines(): + match = _VCP_RE.search(line) + if match: + code = int(match.group(1), 16) + values[code] = (int(match.group(2)), int(match.group(3))) + return values +``` + +- [ ] **Step 4: Run tests to verify pass** + +Run: `python3 -m unittest tests.test_ddc -v` +Expected: PASS, 5 tests. + +- [ ] **Step 5: Commit** + +```bash +git add greenfix/ddc.py tests/test_ddc.py +git commit -m "feat: parse ddcutil detect and getvcp output" +``` + +--- + +## Task 8: DDC command builders + subprocess wrappers + +**Files:** +- Modify: `greenfix/ddc.py` +- Modify: `tests/test_ddc.py` + +- [ ] **Step 1: Write the failing tests** + +Append to `tests/test_ddc.py`: +```python +import subprocess +from unittest.mock import patch + + +class CommandBuilderTests(unittest.TestCase): + def test_build_detect_command(self) -> None: + self.assertEqual(ddc.build_detect_command(), ["ddcutil", "detect"]) + + def test_build_getvcp_command(self) -> None: + self.assertEqual( + ddc.build_getvcp_command(bus=4, codes=[0x10, 0x16, 0x18, 0x1A]), + ["ddcutil", "--bus", "4", "getvcp", "10", "16", "18", "1A"], + ) + + def test_build_setvcp_command(self) -> None: + self.assertEqual( + ddc.build_setvcp_command(bus=4, code=0x18, value=80), + ["ddcutil", "--bus", "4", "setvcp", "18", "80"], + ) + + +class SubprocessWrapperTests(unittest.TestCase): + def test_run_detect_returns_stdout(self) -> None: + completed = subprocess.CompletedProcess(args=[], returncode=0, stdout="ok", stderr="") + with patch.object(subprocess, "run", return_value=completed): + self.assertEqual(ddc.run_detect(), "ok") + + def test_run_detect_raises_when_ddcutil_missing(self) -> None: + with patch.object(subprocess, "run", side_effect=FileNotFoundError): + with self.assertRaisesRegex(ddc.DdcError, "ddcutil was not found"): + ddc.run_detect() + + def test_run_setvcp_raises_on_calledprocess_error(self) -> None: + err = subprocess.CalledProcessError(returncode=1, cmd=["ddcutil"], stderr="permission denied") + with patch.object(subprocess, "run", side_effect=err): + with self.assertRaisesRegex(ddc.DdcError, "permission denied"): + ddc.run_setvcp(bus=4, code=0x18, value=80) +``` + +- [ ] **Step 2: Run tests to verify failure** + +Run: `python3 -m unittest tests.test_ddc -v` +Expected: FAIL with `AttributeError` on `build_detect_command`. + +- [ ] **Step 3: Append command builders and runners to `greenfix/ddc.py`** + +```python +import subprocess + + +def build_detect_command() -> list[str]: + return ["ddcutil", "detect"] + + +def build_getvcp_command(bus: int, codes: list[int]) -> list[str]: + return ["ddcutil", "--bus", str(bus), "getvcp", *(f"{c:X}" for c in codes)] + + +def build_setvcp_command(bus: int, code: int, value: int) -> list[str]: + return ["ddcutil", "--bus", str(bus), "setvcp", f"{code:X}", str(value)] + + +def _run(command: list[str]) -> str: + try: + result = subprocess.run(command, check=True, capture_output=True, text=True) + except FileNotFoundError as exc: + raise DdcError("ddcutil was not found. Install ddcutil and try again.") from exc + except subprocess.CalledProcessError as exc: + detail = (exc.stderr or exc.stdout or str(exc)).strip() + raise DdcError(f"ddcutil failed: {detail}") from exc + return result.stdout + + +def run_detect() -> str: + return _run(build_detect_command()) + + +def run_getvcp(bus: int, codes: list[int]) -> str: + return _run(build_getvcp_command(bus, codes)) + + +def run_setvcp(bus: int, code: int, value: int) -> str: + return _run(build_setvcp_command(bus, code, value)) +``` + +- [ ] **Step 4: Run tests to verify pass** + +Run: `python3 -m unittest tests.test_ddc -v` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add greenfix/ddc.py tests/test_ddc.py +git commit -m "feat: build and run ddcutil commands with error wrapping" +``` + +--- + +## Task 9: DdcBackend.query_outputs + per-output snapshot + +**Files:** +- Modify: `greenfix/ddc.py` +- Modify: `tests/test_ddc.py` + +- [ ] **Step 1: Write the failing tests** + +Append to `tests/test_ddc.py`: +```python +class DdcBackendQueryTests(unittest.TestCase): + def test_backend_id_and_live_preview(self) -> None: + backend = ddc.DdcBackend() + + self.assertEqual(backend.backend_id, "ddc") + self.assertFalse(backend.live_preview) + + def test_query_outputs_maps_to_output_dataclass(self) -> None: + backend = ddc.DdcBackend() + + with patch.object(ddc, "run_detect", return_value=DDCUTIL_DETECT), \ + patch.object(ddc, "run_getvcp", return_value=DDCUTIL_GETVCP): + outputs = backend.query_outputs() + + self.assertEqual(len(outputs), 2) + self.assertEqual(outputs[0].id, "ddc:4") + self.assertEqual(outputs[0].label, "4: DELL U2723QE") + self.assertEqual(outputs[1].id, "ddc:7") + + def test_query_outputs_snapshots_each_display_once(self) -> None: + backend = ddc.DdcBackend() + + with patch.object(ddc, "run_detect", return_value=DDCUTIL_DETECT), \ + patch.object(ddc, "run_getvcp", return_value=DDCUTIL_GETVCP) as getvcp_mock: + backend.query_outputs() + backend.query_outputs() + + # Two displays, snapshotted on first observation, not re-fetched on second. + self.assertEqual(getvcp_mock.call_count, 2) + + def test_query_outputs_returns_empty_when_no_displays(self) -> None: + backend = ddc.DdcBackend() + with patch.object(ddc, "run_detect", return_value=""): + self.assertEqual(backend.query_outputs(), []) +``` + +- [ ] **Step 2: Run tests to verify failure** + +Run: `python3 -m unittest tests.test_ddc -v` +Expected: FAIL because `DdcBackend` does not exist. + +- [ ] **Step 3: Append `DdcBackend` to `greenfix/ddc.py`** + +```python +_VCP_BRIGHTNESS = 0x10 +_VCP_RED = 0x16 +_VCP_GREEN = 0x18 +_VCP_BLUE = 0x1A +_VCP_CODES = [_VCP_BRIGHTNESS, _VCP_RED, _VCP_GREEN, _VCP_BLUE] + + +class DdcBackend: + backend_id = "ddc" + live_preview = False + + def __init__(self) -> None: + self._snapshots: dict[int, dict[int, tuple[int, int]]] = {} + self._models: dict[int, str] = {} + + def query_outputs(self): + from greenfix.display import Output + outputs: list[Output] = [] + for entry in parse_detect(run_detect()): + self._ensure_snapshot(entry) + outputs.append(Output(id=f"ddc:{entry.bus}", label=f"{entry.bus}: {entry.model}")) + return outputs + + def _ensure_snapshot(self, entry: DdcDisplay) -> None: + if entry.bus in self._snapshots: + return + self._snapshots[entry.bus] = parse_getvcp(run_getvcp(entry.bus, _VCP_CODES)) + self._models[entry.bus] = entry.model + + def _bus_from_output_id(self, output_id: str) -> int: + if not output_id.startswith("ddc:"): + raise DdcError(f"Not a ddc output id: {output_id!r}") + try: + return int(output_id.split(":", 1)[1]) + except ValueError as exc: + raise DdcError(f"Bad ddc output id: {output_id!r}") from exc +``` + +- [ ] **Step 4: Run tests to verify pass** + +Run: `python3 -m unittest tests.test_ddc -v` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add greenfix/ddc.py tests/test_ddc.py +git commit -m "feat: ddc backend queries outputs and snapshots vcp values" +``` + +--- + +## Task 10: DdcBackend.slider_spec + neutral_settings + +**Files:** +- Modify: `greenfix/ddc.py` +- Modify: `tests/test_ddc.py` + +- [ ] **Step 1: Write the failing tests** + +Append to `tests/test_ddc.py`: +```python +class DdcBackendSpecTests(unittest.TestCase): + def _populated(self): + backend = ddc.DdcBackend() + with patch.object(ddc, "run_detect", return_value=DDCUTIL_DETECT), \ + patch.object(ddc, "run_getvcp", return_value=DDCUTIL_GETVCP): + backend.query_outputs() + return backend + + def test_slider_spec_for_brightness(self) -> None: + spec = self._populated().slider_spec("brightness", "ddc:4") + + self.assertEqual((spec.minimum, spec.maximum, spec.neutral), (0, 100, 75)) + self.assertEqual(spec.digits, 0) + + def test_slider_spec_for_green(self) -> None: + spec = self._populated().slider_spec("green", "ddc:4") + + self.assertEqual((spec.minimum, spec.maximum, spec.neutral), (0, 100, 95)) + + def test_slider_spec_unknown_output_raises(self) -> None: + with self.assertRaises(ddc.DdcError): + ddc.DdcBackend().slider_spec("red", "ddc:99") + + def test_neutral_settings_returns_snapshot_values(self) -> None: + settings = self._populated().neutral_settings("ddc:4") + + self.assertEqual(settings.output, "ddc:4") + self.assertEqual(settings.backend, "ddc") + self.assertEqual(settings.red_gamma, 100) + self.assertEqual(settings.green_gamma, 95) + self.assertEqual(settings.blue_gamma, 100) + self.assertEqual(settings.brightness, 75) +``` + +- [ ] **Step 2: Run tests to verify failure** + +Run: `python3 -m unittest tests.test_ddc -v` +Expected: FAIL on `slider_spec` and `neutral_settings`. + +- [ ] **Step 3: Add methods inside `DdcBackend`** + +```python + _CHANNEL_CODE = { + "brightness": _VCP_BRIGHTNESS, + "red": _VCP_RED, + "green": _VCP_GREEN, + "blue": _VCP_BLUE, + } + + def slider_spec(self, channel, output_id): + from greenfix.display import SliderSpec + bus = self._bus_from_output_id(output_id) + if bus not in self._snapshots: + raise DdcError(f"No snapshot for {output_id!r}; call query_outputs first.") + current, maximum = self._snapshots[bus][self._CHANNEL_CODE[channel]] + return SliderSpec(minimum=0, maximum=maximum, neutral=current, step=1, digits=0) + + def neutral_settings(self, output_id): + from greenfix.config import Settings + bus = self._bus_from_output_id(output_id) + if bus not in self._snapshots: + raise DdcError(f"No snapshot for {output_id!r}; call query_outputs first.") + snap = self._snapshots[bus] + return Settings( + output=output_id, + backend="ddc", + red_gamma=snap[_VCP_RED][0], + green_gamma=snap[_VCP_GREEN][0], + blue_gamma=snap[_VCP_BLUE][0], + brightness=snap[_VCP_BRIGHTNESS][0], + ) +``` + +- [ ] **Step 4: Run tests to verify pass** + +Run: `python3 -m unittest tests.test_ddc -v` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add greenfix/ddc.py tests/test_ddc.py +git commit -m "feat: ddc backend exposes slider spec and neutral from snapshot" +``` + +--- + +## Task 11: DdcBackend.apply + reset + validate + CLI error catch + +**Files:** +- Modify: `greenfix/ddc.py` +- Modify: `greenfix/cli.py` +- Modify: `tests/test_ddc.py` + +- [ ] **Step 1: Write the failing tests** + +Append to `tests/test_ddc.py`: +```python +class DdcBackendApplyTests(unittest.TestCase): + def _populated(self): + backend = ddc.DdcBackend() + with patch.object(ddc, "run_detect", return_value=DDCUTIL_DETECT), \ + patch.object(ddc, "run_getvcp", return_value=DDCUTIL_GETVCP): + backend.query_outputs() + return backend + + def test_apply_calls_setvcp_in_order(self) -> None: + from greenfix import config + backend = self._populated() + settings = config.Settings( + output="ddc:4", backend="ddc", + red_gamma=90, green_gamma=70, blue_gamma=100, brightness=60, + ) + + with patch.object(ddc, "run_setvcp") as setvcp_mock: + backend.apply(settings) + + self.assertEqual(setvcp_mock.call_count, 4) + self.assertEqual( + [call.args for call in setvcp_mock.call_args_list], + [(4, 0x10, 60), (4, 0x16, 90), (4, 0x18, 70), (4, 0x1A, 100)], + ) + + def test_reset_writes_snapshot_back(self) -> None: + backend = self._populated() + + with patch.object(ddc, "run_setvcp") as setvcp_mock: + backend.reset("ddc:4") + + applied = {call.args[1]: call.args[2] for call in setvcp_mock.call_args_list} + # snapshot was brightness=75, red=100, green=95, blue=100 + self.assertEqual(applied[0x10], 75) + self.assertEqual(applied[0x16], 100) + self.assertEqual(applied[0x18], 95) + self.assertEqual(applied[0x1A], 100) + + def test_apply_rejects_settings_for_wrong_backend(self) -> None: + from greenfix import config + backend = self._populated() + + with self.assertRaises(ddc.DdcError): + backend.apply(config.Settings(output="eDP-1", backend="xrandr")) + + +class DdcValidateTests(unittest.TestCase): + def test_validate_accepts_in_range(self) -> None: + from greenfix import config + ddc.validate(config.Settings(output="ddc:4", backend="ddc", red_gamma=80)) + + def test_settings_rejects_value_above_100(self) -> None: + from greenfix import config + with self.assertRaisesRegex(config.ConfigError, "red"): + config.Settings(output="ddc:4", backend="ddc", red_gamma=120) + + def test_settings_rejects_negative_value(self) -> None: + from greenfix import config + with self.assertRaisesRegex(config.ConfigError, "brightness"): + config.Settings(output="ddc:4", backend="ddc", brightness=-1) + + def test_settings_rejects_bad_output_id(self) -> None: + from greenfix import config + with self.assertRaisesRegex(config.ConfigError, "output"): + config.Settings(output="eDP-1", backend="ddc") +``` + +- [ ] **Step 2: Run tests to verify failure** + +Run: `python3 -m unittest tests.test_ddc -v` +Expected: FAIL because `apply`, `reset`, `validate` don't exist. + +- [ ] **Step 3: Add `validate`, `apply`, `reset`** + +Append the module-level free function and methods inside `DdcBackend` in `greenfix/ddc.py`: +```python +def validate(settings) -> None: + """Validate Settings whose backend is 'ddc'. Raises DdcError.""" + if not settings.output.startswith("ddc:"): + raise DdcError(f"output must start with 'ddc:'; got {settings.output!r}") + for label, value in ( + ("red", settings.red_gamma), + ("green", settings.green_gamma), + ("blue", settings.blue_gamma), + ("brightness", settings.brightness), + ): + if not 0 <= value <= 100: + raise DdcError(f"{label} must be between 0 and 100; got {value}") +``` + +Inside `DdcBackend`: +```python + def apply(self, settings) -> None: + if settings.backend != "ddc": + raise DdcError(f"DdcBackend cannot apply settings for backend {settings.backend!r}") + bus = self._bus_from_output_id(settings.output) + run_setvcp(bus, _VCP_BRIGHTNESS, int(settings.brightness)) + run_setvcp(bus, _VCP_RED, int(settings.red_gamma)) + run_setvcp(bus, _VCP_GREEN, int(settings.green_gamma)) + run_setvcp(bus, _VCP_BLUE, int(settings.blue_gamma)) + + def reset(self, output_id: str) -> None: + bus = self._bus_from_output_id(output_id) + if bus not in self._snapshots: + raise DdcError(f"No snapshot for {output_id!r}; cannot reset.") + snap = self._snapshots[bus] + run_setvcp(bus, _VCP_BRIGHTNESS, snap[_VCP_BRIGHTNESS][0]) + run_setvcp(bus, _VCP_RED, snap[_VCP_RED][0]) + run_setvcp(bus, _VCP_GREEN, snap[_VCP_GREEN][0]) + run_setvcp(bus, _VCP_BLUE, snap[_VCP_BLUE][0]) +``` + +- [ ] **Step 4: Add `DdcError` to the CLI's caught exceptions** + +In `greenfix/cli.py`, replace: +```python + except (config.ConfigError, xrandr.XrandrError) as exc: +``` + +with: +```python + except (config.ConfigError, xrandr.XrandrError, ddc.DdcError) as exc: +``` + +And add the import at the top: `from greenfix import config, ddc, display, xrandr`. + +- [ ] **Step 5: Run the full suite** + +Run: `python3 -m unittest discover -s tests -v` +Expected: All pass. + +- [ ] **Step 6: Commit** + +```bash +git add greenfix/ddc.py greenfix/cli.py tests/test_ddc.py +git commit -m "feat: ddc backend apply, reset, validate" +``` + +--- + +## Task 12: Wire DDC into select_backend() + +**Files:** +- Modify: `greenfix/display.py` +- Modify: `tests/test_display.py` + +- [ ] **Step 1: Update the existing Wayland test** + +In `tests/test_display.py` replace `test_raises_not_implemented_for_wayland_until_ddc_lands` with: +```python +def test_returns_ddc_backend_on_wayland(self) -> None: + from greenfix import ddc + with patch.dict(os.environ, {"XDG_SESSION_TYPE": "wayland"}, clear=False): + backend = display.select_backend() + self.assertIsInstance(backend, ddc.DdcBackend) +``` + +- [ ] **Step 2: Run tests to verify failure** + +Run: `python3 -m unittest tests.test_display -v` +Expected: FAIL with `NotImplementedError`. + +- [ ] **Step 3: Update `select_backend` in `greenfix/display.py`** + +```python +def select_backend() -> Backend: + session = os.environ.get("XDG_SESSION_TYPE", "").lower() + if session == "wayland": + from greenfix.ddc import DdcBackend + return DdcBackend() + from greenfix.xrandr import XrandrBackend + return XrandrBackend() +``` + +- [ ] **Step 4: Run the full suite** + +Run: `python3 -m unittest discover -s tests -v` +Expected: All pass. + +- [ ] **Step 5: Commit** + +```bash +git add greenfix/display.py tests/test_display.py +git commit -m "feat: select ddc backend on wayland sessions" +``` + +--- + +## Task 13: UI status message for empty DDC backend + +**Files:** +- Modify: `greenfix/ui.py` +- Modify: `tests/test_ui.py` + +- [ ] **Step 1: Write the failing test** + +Append to `tests/test_ui.py` `StartupStatusMessageTests`: +```python +def test_ddc_no_outputs_mentions_cosmic_issue(self) -> None: + message = ui.startup_status_message(outputs=[], startup_error=None, backend_id="ddc") + + self.assertIn("No DDC/CI displays", message) + self.assertIn("cosmic-comp", message) + self.assertIn("2059", message) +``` + +- [ ] **Step 2: Run tests to verify failure** + +Run: `python3 -m unittest tests.test_ui -v` +Expected: FAIL — current message is "No connected display outputs were detected." regardless of backend. + +- [ ] **Step 3: Update `startup_status_message` in `greenfix/ui.py`** + +```python +def startup_status_message( + outputs: list, + startup_error: str | None, + backend_id: str, +) -> str | None: + if outputs: + return None + if startup_error: + return startup_error + if backend_id == "ddc": + return ( + "No DDC/CI displays detected. Under COSMIC, greenfix can only adjust " + "external monitors that support DDC/CI — laptop panels are not yet " + "supported. Tracking upstream at pop-os/cosmic-comp#2059." + ) + return "No connected display outputs were detected." +``` + +- [ ] **Step 4: Run the full suite** + +Run: `python3 -m unittest discover -s tests -v` +Expected: All pass. + +- [ ] **Step 5: Commit** + +```bash +git add greenfix/ui.py tests/test_ui.py +git commit -m "feat: explain ddc unavailable state on wayland" +``` + +--- + +## Task 14: README update + +**Files:** +- Modify: `README.md` + +- [ ] **Step 1: Replace the Wayland bullet in Platform Support** + +Find: +```markdown +- Wayland: may not work because most Wayland sessions do not allow `xrandr` to change display settings. +``` + +Replace with: +```markdown +- Wayland (COSMIC and others): supported for **external monitors that speak DDC/CI**. greenfix shells out to `ddcutil` and adjusts VCP brightness (`0x10`) plus per-channel RGB gain (`0x16/0x18/0x1A`). Laptop panels are not supported under Wayland because they don't expose DDC/CI and cosmic-comp does not yet expose `wlr-gamma-control-unstable-v1` (tracking [pop-os/cosmic-comp#2059](https://github.com/pop-os/cosmic-comp/issues/2059)). +``` + +- [ ] **Step 2: Replace the XDG_SESSION_TYPE sentence** + +Find: +```markdown +If `XDG_SESSION_TYPE` is `wayland`, the app warns that display changes may not work but still lets you try. +``` + +Replace with: +```markdown +If `XDG_SESSION_TYPE` is `wayland`, greenfix uses the DDC/CI backend automatically. If no DDC/CI-capable monitors are detected, the UI says so and explains the limitation. +``` + +- [ ] **Step 3: Add `ddcutil` to Requirements** + +After the GTK/PyGObject requirement bullet, insert: +```markdown +- `ddcutil` (only on Wayland; not needed for X11). Your user must be in the `i2c` group for `ddcutil` to access `/dev/i2c-*` without sudo. +``` + +- [ ] **Step 4: Append `ddcutil` to each distro package list** + +- Debian/Ubuntu/Pop!_OS: change `sudo apt install python3-gi gir1.2-gtk-3.0 x11-xserver-utils` to `sudo apt install python3-gi gir1.2-gtk-3.0 x11-xserver-utils ddcutil`. +- Fedora: change `sudo dnf install python3-gobject gtk3 xrandr` to `sudo dnf install python3-gobject gtk3 xrandr ddcutil`. +- Arch: change `sudo pacman -S python-gobject gtk3 xorg-xrandr` to `sudo pacman -S python-gobject gtk3 xorg-xrandr ddcutil`. + +- [ ] **Step 5: Diff review** + +Run: `git diff README.md` +Verify the five edits landed and no whitespace damage. + +- [ ] **Step 6: Commit** + +```bash +git add README.md +git commit -m "docs: document wayland ddc/ci support and laptop limitation" +``` + +--- + +## Spec Coverage Check + +| Spec section | Tasks | +|---|---| +| Backend protocol & types (SliderSpec, Output, Backend) | 1 | +| XrandrBackend adapter + slider_spec + neutral_settings + live_preview=True | 3 | +| `xrandr.validate()` free function + `SLIDER_DEBOUNCE_MS` relocation | 3 | +| Settings `backend` tag; load defaults to "xrandr"; mismatched discard | 2, 3 | +| Validation dispatch via importlib | 3 | +| `select_backend()` (X11, Wayland) | 4, 12 | +| CLI through abstraction; drop `warn_if_wayland`; catch DdcError | 5, 11 | +| UI through abstraction; SliderSpec consumption; live_preview gating | 6 | +| DDC parsing & `DdcError` | 7 | +| DDC command builders; subprocess wrappers (FileNotFoundError, CalledProcessError → DdcError) | 8 | +| DdcBackend `query_outputs` + per-output snapshot (neutral = current at first observation) | 9 | +| DdcBackend `slider_spec` + `neutral_settings` (snapshot-derived) | 10 | +| DdcBackend `apply` (order 0x10, 0x16, 0x18, 0x1A), `reset` (snapshot writeback), `validate` (range check) | 11 | +| Empty-DDC unavailable status referencing cosmic-comp#2059 | 13 | +| README updates | 14 | + +Spec items explicitly out of scope (not in plan, by design): +- Live preview on DDC backend — `live_preview=False` is set in Task 9, UI gates on it in Task 6. +- Direct DRM gamma writes — not implemented. +- In-process Wayland gamma-control client — not implemented. +- CLI flags for DDC-native units — the CLI uses common `--red/--green/--blue/--brightness` whose interpretation depends on the active backend, matching the spec's "CLI surface unchanged." + +Partial-apply behavior under per-VCP failure: if `run_setvcp` fails partway through `DdcBackend.apply()`, prior VCP writes that succeeded stay applied; the raised `DdcError` carries `ddcutil`'s stderr so the user can see what failed. The CLI/UI surfaces the error via the existing exception handler added in Task 11. diff --git a/docs/superpowers/specs/2026-06-03-wayland-ddc-backend-design.md b/docs/superpowers/specs/2026-06-03-wayland-ddc-backend-design.md new file mode 100644 index 0000000..3b3afa9 --- /dev/null +++ b/docs/superpowers/specs/2026-06-03-wayland-ddc-backend-design.md @@ -0,0 +1,147 @@ +# Wayland DDC/CI Backend Design + +## Goal + +Make greenfix actually useful inside a COSMIC Wayland session by adding a second display backend that drives external monitors over DDC/CI. The existing `xrandr` backend stays unchanged for X11 users. Laptop panels under COSMIC remain unsupported — DDC/CI is not available on built-in panels and cosmic-comp does not yet expose `wlr-gamma-control-unstable-v1`. + +## Background + +cosmic-comp (the COSMIC compositor) does not implement `wlr-gamma-control-unstable-v1`. The feature request (pop-os/cosmic-comp#764) is open and gated on a wider color-management story; PR #1543 ("Set gamma for night light") was closed without merging. There is no compositor-level path to set gamma from a client in COSMIC today. + +The maintainer's suggested workaround for users who want gamma adjustment is `ddcutil`. That only helps users with external monitors that support DDC/CI — which is what this design targets. + +## Scope + +In scope: + +- A new DDC/CI backend that wraps `ddcutil` for `getvcp`, `setvcp`, and `detect`. +- A backend selector that picks `xrandr` on X11 sessions and `ddc` on Wayland sessions. +- Per-backend slider metadata so the UI can adapt min/max/neutral/format without knowing the backend's semantics. +- A `backend` tag in saved settings so a saved config for one backend doesn't get applied through the other. +- Clear messaging when on Wayland but no DDC/CI displays are available (the laptop-only case). + +Out of scope: + +- Boosting a channel above its native value on DDC/CI. VCP gain (`0x16/18/1A`) is `0–100` with `100 = max`; the channel cannot be pushed hotter than that. The UI represents this honestly by capping the slider at the channel's max. +- Direct DRM gamma writes. These would fight the compositor on every frame. +- An in-process Wayland `wlr-gamma-control` client. Cosmic-comp does not implement the protocol, so this would do nothing today. If the protocol ships upstream, it can be added as a third backend later. +- CLI flags for DDC-native values. The CLI keeps its `--red/--green/--blue/--brightness` flags; on Wayland the active backend interprets the numbers per its own slider spec. + +## Architecture + +A new `greenfix/display.py` defines the backend abstraction and selects one at runtime: + +```python +class Backend(Protocol): + backend_id: str # "xrandr" or "ddc" + def query_outputs(self) -> list[Output]: ... + def slider_spec(self, channel: str) -> SliderSpec: ... # channel in {red, green, blue, brightness} + def apply(self, settings: Settings) -> None: ... + def reset(self, output_id: str) -> None: ... + def neutral_settings(self, output_id: str) -> Settings: ... +``` + +`select_backend()` returns the `XrandrBackend` on X11 and the `DdcBackend` on Wayland. On Wayland with no DDC/CI displays detected, `select_backend()` still returns the `DdcBackend` but its `query_outputs()` returns an empty list, which the UI surfaces as a clear unavailable state (see UI section). + +`greenfix/xrandr.py` keeps its existing functions and gains a thin `XrandrBackend` class that conforms to the protocol. The bulk of its code is unchanged. + +`greenfix/ddc.py` is new. It shells out to `ddcutil` via `subprocess.run`, parses output, and exposes the same protocol surface as the xrandr backend. It snapshots each output's initial VCP values on first observation so that "neutral" and "reset" mean "what the user had before greenfix touched anything." + +`cli.py` and `ui.py` import `display` and call backend methods through the selected backend. They never reference `xrandr` or `ddc` directly. + +## Value model + +### xrandr backend (unchanged) + +- Red, green, blue gamma: `0.60` to `1.80`, neutral `1.00`, step `0.01`, format `"%.2f"`. +- Brightness: `0.60` to `1.20`, neutral `1.00`, step `0.01`, format `"%.2f"`. +- `apply()` calls `xrandr --output --gamma R:G:B --brightness B`. + +### DDC backend + +- Red, green, blue gain (VCP `0x16/0x18/0x1A`): `0` to the value reported as `max value` by `ddcutil getvcp` (typically `100`). Neutral is the value reported as `current value` at first observation of the output. Step `1`, format `"%d"`. +- Brightness (VCP `0x10`): `0` to reported max (typically `100`). Neutral is the value at first observation. Step `1`, format `"%d"`. +- `apply()` calls `ddcutil --bus setvcp 10 `, then `setvcp 16 `, `setvcp 18 `, `setvcp 1A `, in that order. + +### Why "neutral = startup snapshot" on DDC + +DDC/CI brightness is an absolute backlight value; the user may have it set to anything. Treating `100` as neutral would mean the first "Reset" press cranks the backlight to max. Snapshotting `current value` at first detection captures the user's preferred baseline. The snapshot is held per output in the running process and is not persisted — relaunching greenfix re-snapshots whatever the monitor is currently showing. + +## Output identity + +xrandr backend: output identifier is the xrandr name (e.g., `eDP-1`). UI label is the same string. Unchanged from today. + +DDC backend: output identifier is `ddc:` where `` is the integer bus number `ddcutil detect` reports for that display. This is stable across reboots on a given machine and across `ddcutil` invocations. UI label is `: ` (e.g., `4: DELL U2723QE`). Saved config carries the `ddc:` form so it round-trips on re-launch. + +## Config schema + +`Settings` gains a `backend` field: + +```python +@dataclass(frozen=True) +class Settings: + output: str + backend: str = "xrandr" + red_gamma: float = 1.0 + green_gamma: float = 1.0 + blue_gamma: float = 1.0 + brightness: float = 1.0 +``` + +Field names stay the same; the values they hold are interpreted by the named backend. On load: + +- A file without a `backend` field is treated as `"xrandr"` (backward compatible with existing MVP saves). +- A file whose `backend` matches the running backend is loaded normally. +- A file whose `backend` does not match the running backend is discarded; the UI starts at neutral and shows a status note ("Saved settings were for the `xrandr` backend; this session uses `ddc`."). + +Validation moves from the central `xrandr.validate_*` helpers into each backend module as free functions: `xrandr.validate(settings: Settings)` and `ddc.validate(settings: Settings)`. `config.Settings.__post_init__` dispatches by `settings.backend`, calling the matching module's `validate`. This keeps validation co-located with the backend that defines the legal ranges, matching the existing free-function pattern in `xrandr.py`. + +## UI changes + +`ui.py` no longer hard-codes slider ranges. The window is constructed against the selected backend: + +```python +backend = display.select_backend() +spec_r = backend.slider_spec("red") # SliderSpec(min, max, neutral, step, digits) +# ...adjustments built from each spec +``` + +The existing Wayland warning label (`xrandr.WAYLAND_WARNING`) is removed. In its place: + +- DDC backend with one or more outputs: no banner. Status line says nothing on launch. +- DDC backend with zero outputs (Wayland + no DDC/CI displays): status banner reads, "No DDC/CI displays detected. Under COSMIC, greenfix can only adjust external monitors that support DDC/CI — laptop panels are not yet supported. Tracking upstream at pop-os/cosmic-comp#2059." Sliders and Apply are disabled in this state. + +Live preview policy is per-backend: + +- xrandr backend: existing 250 ms debounced live preview on slider drag, unchanged. +- DDC backend: no live preview. Slider activity updates the value label only and does not schedule any apply. Apply runs strictly via the **Apply** button. The backend exposes a `live_preview: bool` attribute that `ui.py` consults — when `False`, `_on_scale_changed` updates the label and skips `_schedule_preview()` entirely. The 250 ms debounce constant moves from `ui.py` into the xrandr backend (where it belongs) so the UI doesn't carry backend-specific timing. + +## Reset semantics + +- xrandr backend: unchanged. Reset writes gamma `1.0:1.0:1.0` and brightness `1.0`. +- DDC backend: Reset writes the per-output startup snapshot back through `setvcp` for VCP `0x10/16/18/1A`. The user's baseline brightness is preserved. + +Neither backend invokes `setvcp 04` ("restore factory defaults") — that would wipe unrelated monitor settings. + +## Failure modes + +DDC backend, surfaced as `DdcError` (parallel to `XrandrError`): + +- `ddcutil` binary missing → "ddcutil was not found. Install ddcutil and try again." +- No I²C access (permission denied on `/dev/i2c-*`) → "ddcutil could not access the I²C bus. Add your user to the `i2c` group and re-login." +- `ddcutil detect` returns zero displays → UI unavailable state described above; not raised as an error. +- `setvcp` fails for one VCP code → status line shows which channel failed; other channels that succeeded stay applied. + +## Testing + +Unit tests use the same pattern as the existing xrandr tests: mock `subprocess.run` and verify the command line plus parse paths. New test modules: + +- `tests/test_ddc.py` — parsing of `ddcutil detect` and `ddcutil getvcp 10 16 18 1A`; correct `setvcp` argument construction; snapshot/restore behavior; permission and missing-binary error paths. +- `tests/test_display.py` — backend selection by `XDG_SESSION_TYPE`; behavior when DDC reports zero displays on Wayland. +- `tests/test_config.py` — backend tag round-trips; old (untagged) files load as `xrandr`; mismatched-backend files are discarded. + +UI changes are exercised through targeted unit tests on the helpers (`startup_status_message`, slider-spec selection) the same way the MVP tests them. The window itself is not driven in tests, matching existing practice. + +## What this does not fix + +Users on COSMIC with only a laptop panel still have no path. The status message points them at pop-os/cosmic-comp#2059 so they understand why. When cosmic-comp ships `wlr-gamma-control-unstable-v1` (or its successor color-management protocol), a third backend can be added behind the same `display.Backend` protocol with no UI changes. diff --git a/greenfix/cli.py b/greenfix/cli.py index 7d0fdff..39e4eaf 100644 --- a/greenfix/cli.py +++ b/greenfix/cli.py @@ -7,7 +7,7 @@ from pathlib import Path from typing import Callable, Sequence, TextIO -from greenfix import config, xrandr +from greenfix import config, ddc, display, xrandr UiLauncher = Callable[[], int] @@ -24,18 +24,19 @@ def run( args = parser.parse_args(argv) try: + backend = display.select_backend() if args.list_outputs: - return list_outputs(stdout) + return list_outputs(backend, stdout) if args.apply_saved: - return apply_saved(stdout, stderr, settings_path) + return apply_saved(backend, stdout, settings_path) if args.reset: - return reset_settings(args.output, stdout, stderr, settings_path) + return reset_settings(backend, args.output, stdout, settings_path) if args.apply: - return apply_explicit(args, stdout, stderr) + return apply_explicit(backend, args, stdout) if launch_ui is None: from greenfix.ui import main as launch_ui return launch_ui() - except (config.ConfigError, xrandr.XrandrError) as exc: + except (config.ConfigError, xrandr.XrandrError, ddc.DdcError) as exc: print(f"greenfix: {exc}", file=stderr) return 1 @@ -43,78 +44,77 @@ def run( def build_parser() -> argparse.ArgumentParser: parser = argparse.ArgumentParser( prog="greenfix", - description="Apply simple xrandr display color-balance settings.", + description="Apply display color-balance settings.", ) parser.add_argument("--apply-saved", action="store_true", help="apply saved settings and exit") parser.add_argument("--reset", action="store_true", help="reset the selected or saved output") - parser.add_argument("--list-outputs", action="store_true", help="list connected xrandr outputs") + parser.add_argument("--list-outputs", action="store_true", help="list connected outputs") parser.add_argument("--apply", action="store_true", help="apply explicit CLI values and exit") - parser.add_argument("--output", help="xrandr output name, such as eDP-1") - parser.add_argument("--red", type=float, default=1.0, help="red gamma value") - parser.add_argument("--green", type=float, default=1.0, help="green gamma value") - parser.add_argument("--blue", type=float, default=1.0, help="blue gamma value") - parser.add_argument("--brightness", type=float, default=1.0, help="software brightness value") + parser.add_argument("--output", help="output identifier, such as eDP-1 or ddc:4") + parser.add_argument("--red", type=float, help="red channel value") + parser.add_argument("--green", type=float, help="green channel value") + parser.add_argument("--blue", type=float, help="blue channel value") + parser.add_argument("--brightness", type=float, help="brightness value") return parser -def list_outputs(stdout: TextIO) -> int: - outputs = xrandr.query_connected_outputs() - for output in outputs: - print(output, file=stdout) +def list_outputs(backend: display.Backend, stdout: TextIO) -> int: + for output in backend.query_outputs(): + # The id is what --output expects (e.g. "ddc:4"); show it whenever it + # differs from the human label so users can copy the right identifier. + line = output.label if output.id == output.label else f"{output.id}\t{output.label}" + print(line, file=stdout) return 0 -def apply_saved(stdout: TextIO, stderr: TextIO, settings_path: Path | None) -> int: - settings = config.load_settings(settings_path) +def apply_saved(backend: display.Backend, stdout: TextIO, settings_path: Path | None) -> int: + settings = config.load_settings(settings_path, active_backend=backend.backend_id) if settings is None: - print("No saved settings found.", file=stdout) + print("No saved settings found for the active backend.", file=stdout) return 0 - warn_if_wayland(stderr) - xrandr.apply_settings( - settings.output, - settings.red_gamma, - settings.green_gamma, - settings.blue_gamma, - settings.brightness, - ) + backend.apply(settings) print(f"Applied saved settings for {settings.output}.", file=stdout) return 0 -def reset_settings( - output: str | None, - stdout: TextIO, - stderr: TextIO, - settings_path: Path | None, -) -> int: +def reset_settings(backend: display.Backend, output: str | None, stdout: TextIO, settings_path: Path | None) -> int: target_output = output if target_output is None: - settings = config.load_settings(settings_path) - if settings is not None: - target_output = settings.output + saved = config.load_settings(settings_path, active_backend=backend.backend_id) + if saved is not None: + target_output = saved.output if target_output is None: - target_output = xrandr.require_output(xrandr.query_connected_outputs()) - warn_if_wayland(stderr) - xrandr.reset_output(target_output) + outputs = backend.query_outputs() + if not outputs: + raise xrandr.XrandrError("No connected display outputs were detected.") + target_output = outputs[0].id + backend.reset(target_output) print(f"Reset {target_output}.", file=stdout) return 0 -def apply_explicit(args: argparse.Namespace, stdout: TextIO, stderr: TextIO) -> int: +def apply_explicit(backend: display.Backend, args: argparse.Namespace, stdout: TextIO) -> int: if not args.output: raise xrandr.XrandrError("--output is required with --apply.") - warn_if_wayland(stderr) - xrandr.apply_settings( - args.output, - args.red, - args.green, - args.blue, - args.brightness, - ) + settings = _build_apply_settings(backend, args) + backend.apply(settings) print(f"Applied settings for {args.output}.", file=stdout) return 0 -def warn_if_wayland(stderr: TextIO) -> None: - if xrandr.is_wayland_session(): - print(xrandr.WAYLAND_WARNING, file=stderr) +def _build_apply_settings(backend: display.Backend, args: argparse.Namespace) -> config.Settings: + # Omitted channels are filled from the backend's neutral baseline for this + # output, not a hardcoded 1.0. That is neutral for xrandr but the monitor's + # current value for DDC, where raw 1.0 would nearly zero an RGB gain. + values = { + "red_gamma": args.red, + "green_gamma": args.green, + "blue_gamma": args.blue, + "brightness": args.brightness, + } + if any(value is None for value in values.values()): + base = backend.neutral_settings(args.output) + for field, value in values.items(): + if value is None: + values[field] = getattr(base, field) + return config.Settings(output=args.output, backend=backend.backend_id, **values) diff --git a/greenfix/config.py b/greenfix/config.py index 09453f8..d53ea71 100644 --- a/greenfix/config.py +++ b/greenfix/config.py @@ -18,19 +18,24 @@ class ConfigError(ValueError): @dataclass(frozen=True) class Settings: output: str + backend: str = "xrandr" red_gamma: float = xrandr.NEUTRAL_GAMMA green_gamma: float = xrandr.NEUTRAL_GAMMA blue_gamma: float = xrandr.NEUTRAL_GAMMA brightness: float = xrandr.NEUTRAL_BRIGHTNESS def __post_init__(self) -> None: + import importlib + + try: + module = importlib.import_module(f"greenfix.{self.backend}") + except ModuleNotFoundError as exc: + raise ConfigError(f"Unknown backend: {self.backend!r}") from exc try: - xrandr.validate_output_name(self.output) - xrandr.validate_gamma_value("red gamma", self.red_gamma) - xrandr.validate_gamma_value("green gamma", self.green_gamma) - xrandr.validate_gamma_value("blue gamma", self.blue_gamma) - xrandr.validate_brightness_value(self.brightness) - except xrandr.XrandrError as exc: + module.validate(self) + except ConfigError: + raise + except Exception as exc: raise ConfigError(str(exc)) from exc @@ -49,7 +54,10 @@ def neutral_settings(output: str) -> Settings: return Settings(output=output) -def load_settings(path: Path | None = None) -> Settings | None: +def load_settings( + path: Path | None = None, + active_backend: str | None = None, +) -> Settings | None: settings_path = path or default_settings_path() try: data = json.loads(settings_path.read_text(encoding="utf-8")) @@ -58,14 +66,18 @@ def load_settings(path: Path | None = None) -> Settings | None: output = data["output"] if not isinstance(output, str): return None + backend = str(data.get("backend", "xrandr")) + if active_backend is not None and backend != active_backend: + return None return Settings( output=output, + backend=backend, red_gamma=float(data.get("red_gamma", xrandr.NEUTRAL_GAMMA)), green_gamma=float(data.get("green_gamma", xrandr.NEUTRAL_GAMMA)), blue_gamma=float(data.get("blue_gamma", xrandr.NEUTRAL_GAMMA)), brightness=float(data.get("brightness", xrandr.NEUTRAL_BRIGHTNESS)), ) - except (FileNotFoundError, json.JSONDecodeError, KeyError, TypeError, ValueError): + except (FileNotFoundError, json.JSONDecodeError, KeyError, TypeError, ValueError, ConfigError): return None diff --git a/greenfix/ddc.py b/greenfix/ddc.py new file mode 100644 index 0000000..2d64c7f --- /dev/null +++ b/greenfix/ddc.py @@ -0,0 +1,226 @@ +"""DDC/CI display backend powered by ddcutil.""" + +from __future__ import annotations + +import re +import subprocess +from dataclasses import dataclass + +from greenfix.display import SliderSpec + + +class DdcError(RuntimeError): + """Raised when ddcutil cannot complete the requested operation.""" + + +@dataclass(frozen=True) +class DdcDisplay: + bus: int + model: str + + +_VALID_HEADER_RE = re.compile(r"^Display\s+\d+\b") +_BUS_RE = re.compile(r"I2C bus:\s+/dev/i2c-(\d+)") +_MODEL_RE = re.compile(r"Model:\s+(.+)$") +_VCP_RE = re.compile( + r"VCP code 0x([0-9a-fA-F]+).*?current value =\s*(\d+).*?max value =\s*(\d+)" +) + + +def parse_detect(output: str) -> list[DdcDisplay]: + # ddcutil starts each display block with a column-0 header line. Valid + # displays begin with "Display N"; "Invalid display" (DDC failed) and + # "Phantom display N" blocks must be excluded so their bus/model never + # leak into the next valid block. + displays: list[DdcDisplay] = [] + bus: int | None = None + model: str | None = None + valid = False + + def flush() -> None: + if valid and bus is not None and model is not None: + displays.append(DdcDisplay(bus=bus, model=model)) + + for line in output.splitlines(): + if line and not line[0].isspace(): + flush() + bus = None + model = None + valid = bool(_VALID_HEADER_RE.match(line)) + continue + bus_match = _BUS_RE.search(line) + if bus_match: + bus = int(bus_match.group(1)) + continue + model_match = _MODEL_RE.search(line) + if model_match: + model = model_match.group(1).strip() + flush() + return displays + + +def parse_getvcp(output: str) -> dict[int, tuple[int, int]]: + values: dict[int, tuple[int, int]] = {} + for line in output.splitlines(): + match = _VCP_RE.search(line) + if match: + code = int(match.group(1), 16) + values[code] = (int(match.group(2)), int(match.group(3))) + return values + + +def build_detect_command() -> list[str]: + return ["ddcutil", "detect"] + + +def build_getvcp_command(bus: int, codes: list[int]) -> list[str]: + return ["ddcutil", "--bus", str(bus), "getvcp", *(f"{c:X}" for c in codes)] + + +def build_setvcp_command(bus: int, code: int, value: int) -> list[str]: + return ["ddcutil", "--bus", str(bus), "setvcp", f"{code:X}", str(value)] + + +def _run(command: list[str]) -> str: + try: + result = subprocess.run(command, check=True, capture_output=True, text=True) + except FileNotFoundError as exc: + raise DdcError("ddcutil was not found. Install ddcutil and try again.") from exc + except subprocess.CalledProcessError as exc: + detail = (exc.stderr or exc.stdout or str(exc)).strip() + raise DdcError(f"ddcutil failed: {detail}") from exc + return result.stdout + + +def run_detect() -> str: + return _run(build_detect_command()) + + +def run_getvcp(bus: int, codes: list[int]) -> str: + return _run(build_getvcp_command(bus, codes)) + + +def run_setvcp(bus: int, code: int, value: int) -> str: + return _run(build_setvcp_command(bus, code, value)) + + +_VCP_BRIGHTNESS = 0x10 +_VCP_RED = 0x16 +_VCP_GREEN = 0x18 +_VCP_BLUE = 0x1A +_VCP_CODES = (_VCP_BRIGHTNESS, _VCP_RED, _VCP_GREEN, _VCP_BLUE) + + +def _missing_codes(snapshot: dict[int, tuple[int, int]]) -> list[int]: + return [code for code in _VCP_CODES if code not in snapshot] + + +class DdcBackend: + backend_id = "ddc" + live_preview = False + + _CHANNEL_CODE = { + "brightness": _VCP_BRIGHTNESS, + "red": _VCP_RED, + "green": _VCP_GREEN, + "blue": _VCP_BLUE, + } + + def __init__(self) -> None: + self._snapshots: dict[int, dict[int, tuple[int, int]]] = {} + self._models: dict[int, str] = {} + + def query_outputs(self) -> list: + from greenfix.display import Output + outputs: list[Output] = [] + for entry in parse_detect(run_detect()): + if self._ensure_snapshot(entry): + outputs.append(Output(id=f"ddc:{entry.bus}", label=f"{entry.bus}: {entry.model}")) + return outputs + + def slider_spec(self, channel: str, output_id: str): + snap = self._get_snapshot(output_id) + try: + current, maximum = snap[self._CHANNEL_CODE[channel]] + except KeyError: + raise DdcError(f"Unknown channel: {channel!r}") + return SliderSpec(minimum=0, maximum=maximum, neutral=current, step=1, digits=0) + + def neutral_settings(self, output_id: str): + from greenfix.config import Settings + snap = self._get_snapshot(output_id) + return Settings( + output=output_id, + backend=self.backend_id, + red_gamma=snap[_VCP_RED][0], + green_gamma=snap[_VCP_GREEN][0], + blue_gamma=snap[_VCP_BLUE][0], + brightness=snap[_VCP_BRIGHTNESS][0], + ) + + def apply(self, settings) -> None: + if settings.backend != self.backend_id: + raise DdcError(f"DdcBackend cannot apply settings for backend {settings.backend!r}") + bus = self._bus_from_output_id(settings.output) + run_setvcp(bus, _VCP_BRIGHTNESS, int(settings.brightness)) + run_setvcp(bus, _VCP_RED, int(settings.red_gamma)) + run_setvcp(bus, _VCP_GREEN, int(settings.green_gamma)) + run_setvcp(bus, _VCP_BLUE, int(settings.blue_gamma)) + + def reset(self, output_id: str) -> None: + snap = self._get_snapshot(output_id) + bus = self._bus_from_output_id(output_id) + run_setvcp(bus, _VCP_BRIGHTNESS, snap[_VCP_BRIGHTNESS][0]) + run_setvcp(bus, _VCP_RED, snap[_VCP_RED][0]) + run_setvcp(bus, _VCP_GREEN, snap[_VCP_GREEN][0]) + run_setvcp(bus, _VCP_BLUE, snap[_VCP_BLUE][0]) + + def _ensure_snapshot(self, entry: DdcDisplay) -> bool: + # Snapshot a detected display. Returns True when it reports every + # required VCP code (and is therefore usable). Displays missing codes + # are not cached and not listed, so a partly-DDC monitor can never be + # offered as adjustable and later raise KeyError on a missing channel. + if entry.bus in self._snapshots: + return True + snapshot = parse_getvcp(run_getvcp(entry.bus, _VCP_CODES)) + if _missing_codes(snapshot): + return False + self._snapshots[entry.bus] = snapshot + self._models[entry.bus] = entry.model + return True + + def _get_snapshot(self, output_id: str) -> dict[int, tuple[int, int]]: + # Lazily snapshot the bus if query_outputs() never ran (e.g. a cold CLI + # `--reset ddc:N`), so reset/apply are self-sufficient for a valid id. + bus = self._bus_from_output_id(output_id) + if bus not in self._snapshots: + snapshot = parse_getvcp(run_getvcp(bus, _VCP_CODES)) + missing = _missing_codes(snapshot) + if missing: + codes = ", ".join(f"0x{code:02X}" for code in missing) + raise DdcError(f"{output_id} does not report required VCP codes: {codes}") + self._snapshots[bus] = snapshot + return self._snapshots[bus] + + def _bus_from_output_id(self, output_id: str) -> int: + prefix = f"{self.backend_id}:" + if not output_id.startswith(prefix): + raise DdcError(f"Not a ddc output id: {output_id!r}") + try: + return int(output_id[len(prefix):]) + except ValueError as exc: + raise DdcError(f"Bad ddc output id: {output_id!r}") from exc + + +def validate(settings) -> None: + """Validate Settings whose backend is 'ddc'. Raises DdcError.""" + if not settings.output.startswith("ddc:"): + raise DdcError(f"output must start with 'ddc:'; got {settings.output!r}") + for label, value in ( + ("red", settings.red_gamma), + ("green", settings.green_gamma), + ("blue", settings.blue_gamma), + ("brightness", settings.brightness), + ): + if not 0 <= value <= 100: + raise DdcError(f"{label} must be between 0 and 100; got {value}") diff --git a/greenfix/display.py b/greenfix/display.py new file mode 100644 index 0000000..2373b70 --- /dev/null +++ b/greenfix/display.py @@ -0,0 +1,52 @@ +"""Display backend abstraction shared by xrandr and ddc backends.""" + +from __future__ import annotations + +import os +from dataclasses import dataclass +from typing import Protocol, runtime_checkable + + +@dataclass(frozen=True) +class SliderSpec: + minimum: float + maximum: float + neutral: float + step: float + digits: int + + +@dataclass(frozen=True) +class Output: + id: str + label: str + + +@runtime_checkable +class Backend(Protocol): + backend_id: str + live_preview: bool + + def query_outputs(self) -> list[Output]: + """Return the connected, controllable outputs.""" + + def slider_spec(self, channel: str, output_id: str) -> SliderSpec: + """Return the slider range/neutral for a channel on an output.""" + + def neutral_settings(self, output_id: str): + """Return the baseline settings to show for an output.""" + + def apply(self, settings) -> None: + """Apply the given settings to the display.""" + + def reset(self, output_id: str) -> None: + """Restore the output to its baseline.""" + + +def select_backend() -> Backend: + session = os.environ.get("XDG_SESSION_TYPE", "").lower() + if session == "wayland": + from greenfix.ddc import DdcBackend + return DdcBackend() + from greenfix.xrandr import XrandrBackend + return XrandrBackend() diff --git a/greenfix/ui.py b/greenfix/ui.py index 1429f80..e28a790 100644 --- a/greenfix/ui.py +++ b/greenfix/ui.py @@ -2,9 +2,35 @@ from __future__ import annotations -from typing import Callable +from types import SimpleNamespace -from greenfix import config, xrandr +from greenfix import config, ddc, display, xrandr +from greenfix.xrandr import SLIDER_DEBOUNCE_MS + +_BACKEND_ERRORS = (config.ConfigError, xrandr.XrandrError, ddc.DdcError) + +_CHANNELS = ("red", "green", "blue", "brightness") + + +def output_view(backend, saved_settings, output_id): + """Slider specs and values to show for ``output_id``. + + Pulled out of the GTK window so selecting a different display always + recomputes ranges and values from that output's own snapshot, instead of + leaving one monitor's values applied to another. + """ + specs = {channel: backend.slider_spec(channel, output_id) for channel in _CHANNELS} + if saved_settings is not None and saved_settings.output == output_id: + settings = saved_settings + else: + settings = backend.neutral_settings(output_id) + return SimpleNamespace( + red=specs["red"], + green=specs["green"], + blue=specs["blue"], + brightness=specs["brightness"], + settings=settings, + ) try: import gi @@ -19,7 +45,6 @@ else: GTK_IMPORT_ERROR = None -SLIDER_DEBOUNCE_MS = 250 _GtkWindowBase = Gtk.Window if Gtk is not None else object @@ -31,27 +56,33 @@ def __init__(self) -> None: self.set_border_width(12) self.set_default_size(420, 300) + self.backend = display.select_backend() self._debounce_id: int | None = None self.outputs = self._load_outputs() - self.saved_settings = config.load_settings() - selected_output = self._initial_output() - initial_settings = self._initial_settings(selected_output) + self.saved_settings = config.load_settings(active_backend=self.backend.backend_id) + selected_output_id = self._initial_output_id() + initial_settings = self._initial_settings(selected_output_id) + spec_output = selected_output_id or (self.outputs[0].id if self.outputs else "") self.output_combo = Gtk.ComboBoxText() for output in self.outputs: - self.output_combo.append_text(output) - if selected_output in self.outputs: - self.output_combo.set_active(self.outputs.index(selected_output)) + self.output_combo.append_text(output.label) + if selected_output_id is not None: + for index, output in enumerate(self.outputs): + if output.id == selected_output_id: + self.output_combo.set_active(index) + break self.output_combo.connect("changed", self._on_control_changed) - self.red_scale, self.red_value = self._create_scale(initial_settings.red_gamma, 0.60, 1.80) - self.green_scale, self.green_value = self._create_scale(initial_settings.green_gamma, 0.60, 1.80) - self.blue_scale, self.blue_value = self._create_scale(initial_settings.blue_gamma, 0.60, 1.80) - self.brightness_scale, self.brightness_value = self._create_scale( - initial_settings.brightness, - 0.60, - 1.20, - ) + red_spec = self.backend.slider_spec("red", spec_output) if self.outputs else display.SliderSpec(0, 1, 1, 0.01, 2) + green_spec = self.backend.slider_spec("green", spec_output) if self.outputs else red_spec + blue_spec = self.backend.slider_spec("blue", spec_output) if self.outputs else red_spec + bright_spec = self.backend.slider_spec("brightness", spec_output) if self.outputs else red_spec + + self.red_scale, self.red_value = self._create_scale(initial_settings.red_gamma, red_spec) + self.green_scale, self.green_value = self._create_scale(initial_settings.green_gamma, green_spec) + self.blue_scale, self.blue_value = self._create_scale(initial_settings.blue_gamma, blue_spec) + self.brightness_scale, self.brightness_value = self._create_scale(initial_settings.brightness, bright_spec) self.status_label = Gtk.Label() self.status_label.set_xalign(0) @@ -59,54 +90,48 @@ def __init__(self) -> None: root = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=10) self.add(root) - - if xrandr.is_wayland_session(): - warning = Gtk.Label(label=xrandr.WAYLAND_WARNING) - warning.set_xalign(0) - warning.set_line_wrap(True) - root.pack_start(warning, False, False, 0) - root.pack_start(self._output_row(), False, False, 0) - root.pack_start(self._slider_row("Red gamma", self.red_scale, self.red_value), False, False, 0) - root.pack_start(self._slider_row("Green gamma", self.green_scale, self.green_value), False, False, 0) - root.pack_start(self._slider_row("Blue gamma", self.blue_scale, self.blue_value), False, False, 0) - root.pack_start( - self._slider_row("Brightness", self.brightness_scale, self.brightness_value), - False, - False, - 0, - ) + root.pack_start(self._slider_row("Red", self.red_scale, self.red_value), False, False, 0) + root.pack_start(self._slider_row("Green", self.green_scale, self.green_value), False, False, 0) + root.pack_start(self._slider_row("Blue", self.blue_scale, self.blue_value), False, False, 0) + root.pack_start(self._slider_row("Brightness", self.brightness_scale, self.brightness_value), False, False, 0) root.pack_start(self._button_row(), False, False, 0) root.pack_start(self.status_label, False, False, 0) startup_message = startup_status_message( - self.outputs, - getattr(self, "_startup_error", None), + outputs=self.outputs, + startup_error=getattr(self, "_startup_error", None), + backend_id=self.backend.backend_id, ) if startup_message is not None: self._set_status(startup_message) - def _load_outputs(self) -> list[str]: + def _load_outputs(self) -> list[display.Output]: try: - return xrandr.query_connected_outputs() - except xrandr.XrandrError as exc: + return self.backend.query_outputs() + except _BACKEND_ERRORS as exc: self._startup_error = str(exc) return [] - def _initial_output(self) -> str: - if self.saved_settings and self.saved_settings.output in self.outputs: - return self.saved_settings.output - preferred = xrandr.choose_preferred_output(self.outputs) - if preferred is not None: - return preferred + def _initial_output_id(self): if self.saved_settings: - return self.saved_settings.output - return "eDP-1" - - def _initial_settings(self, output: str) -> config.Settings: - if self.saved_settings and self.saved_settings.output == output: + for output in self.outputs: + if output.id == self.saved_settings.output: + return output.id + for output in self.outputs: + if output.id == "eDP-1": + return output.id + return self.outputs[0].id if self.outputs else None + + def _initial_settings(self, output_id): + if output_id is None: + # No outputs available; use neutral display values so the sliders show + # something. The Apply button checks for a selected output before doing + # anything with these values. + return SimpleNamespace(red_gamma=1.0, green_gamma=1.0, blue_gamma=1.0, brightness=1.0) + if self.saved_settings and self.saved_settings.output == output_id: return self.saved_settings - return config.neutral_settings(output) + return self.backend.neutral_settings(output_id) def _output_row(self): row = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL, spacing=8) @@ -116,20 +141,21 @@ def _output_row(self): row.pack_start(self.output_combo, True, True, 0) return row - def _create_scale(self, value: float, minimum: float, maximum: float): + def _create_scale(self, value, spec): adjustment = Gtk.Adjustment( value=value, - lower=minimum, - upper=maximum, - step_increment=0.01, - page_increment=0.05, + lower=spec.minimum, + upper=spec.maximum, + step_increment=spec.step, + page_increment=spec.step * 5, page_size=0, ) scale = Gtk.Scale(orientation=Gtk.Orientation.HORIZONTAL, adjustment=adjustment) - scale.set_digits(2) + scale.set_digits(spec.digits) scale.set_hexpand(True) - value_label = Gtk.Label(label=f"{value:.2f}") - scale.connect("value-changed", self._on_scale_changed, value_label) + fmt = f"{{:.{spec.digits}f}}" + value_label = Gtk.Label(label=fmt.format(value)) + scale.connect("value-changed", self._on_scale_changed, value_label, spec.digits) return scale, value_label def _slider_row(self, label_text: str, scale, value_label): @@ -156,12 +182,42 @@ def _button_row(self): row.pack_start(button, False, False, 0) return row - def _on_scale_changed(self, scale, value_label) -> None: - value_label.set_text(f"{scale.get_value():.2f}") - self._schedule_preview() + def _on_scale_changed(self, scale, value_label, digits) -> None: + fmt = f"{{:.{digits}f}}" + value_label.set_text(fmt.format(scale.get_value())) + if self.backend.live_preview: + self._schedule_preview() def _on_control_changed(self, *_args) -> None: - self._schedule_preview() + output_id = self._current_output() + if output_id is not None: + self._refresh_for_output(output_id) + if self.backend.live_preview: + self._schedule_preview() + + def _refresh_for_output(self, output_id: str) -> None: + # Selecting a different display must rebuild slider ranges and values + # from that output's own snapshot; otherwise one monitor's values stay + # applied to another. + try: + view = output_view(self.backend, self.saved_settings, output_id) + except _BACKEND_ERRORS as exc: + self._set_status(str(exc)) + return + for scale, value_label, spec, value in ( + (self.red_scale, self.red_value, view.red, view.settings.red_gamma), + (self.green_scale, self.green_value, view.green, view.settings.green_gamma), + (self.blue_scale, self.blue_value, view.blue, view.settings.blue_gamma), + (self.brightness_scale, self.brightness_value, view.brightness, view.settings.brightness), + ): + adjustment = scale.get_adjustment() + adjustment.set_lower(spec.minimum) + adjustment.set_upper(spec.maximum) + adjustment.set_step_increment(spec.step) + adjustment.set_page_increment(spec.step * 5) + scale.set_digits(spec.digits) + scale.set_value(value) + value_label.set_text(f"{{:.{spec.digits}f}}".format(value)) def _schedule_preview(self) -> None: if GLib is None: @@ -182,24 +238,24 @@ def _on_save_clicked(self, _button) -> None: try: settings = self._current_settings() config.save_settings(settings) - except (config.ConfigError, xrandr.XrandrError) as exc: + except _BACKEND_ERRORS as exc: self._set_status(str(exc)) return self._set_status(f"Saved settings for {settings.output}.") def _on_reset_clicked(self, _button) -> None: - output = self._current_output() - if output is None: + output_id = self._current_output() + if output_id is None: self._set_status("No display output is selected.") return - settings = config.neutral_settings(output) + settings = self.backend.neutral_settings(output_id) self._set_slider_values(settings) try: - xrandr.reset_output(output) - except xrandr.XrandrError as exc: + self.backend.reset(output_id) + except _BACKEND_ERRORS as exc: self._set_status(str(exc)) return - self._set_status(f"Reset {output}.") + self._set_status(f"Reset {output_id}.") def _on_quit_clicked(self, _button) -> None: Gtk.main_quit() @@ -207,24 +263,19 @@ def _on_quit_clicked(self, _button) -> None: def _apply_current(self, success_message: str) -> None: try: settings = self._current_settings() - xrandr.apply_settings( - settings.output, - settings.red_gamma, - settings.green_gamma, - settings.blue_gamma, - settings.brightness, - ) - except (config.ConfigError, xrandr.XrandrError) as exc: + self.backend.apply(settings) + except _BACKEND_ERRORS as exc: self._set_status(str(exc)) return self._set_status(success_message) - def _current_settings(self) -> config.Settings: - output = self._current_output() - if output is None: + def _current_settings(self): + output_id = self._current_output() + if output_id is None: raise xrandr.XrandrError("No display output is selected.") return config.Settings( - output=output, + output=output_id, + backend=self.backend.backend_id, red_gamma=self.red_scale.get_value(), green_gamma=self.green_scale.get_value(), blue_gamma=self.blue_scale.get_value(), @@ -232,8 +283,10 @@ def _current_settings(self) -> config.Settings: ) def _current_output(self) -> str | None: - active = self.output_combo.get_active_text() - return str(active) if active is not None else None + index = self.output_combo.get_active() + if index < 0 or index >= len(self.outputs): + return None + return self.outputs[index].id def _set_slider_values(self, settings: config.Settings) -> None: self.red_scale.set_value(settings.red_gamma) @@ -259,9 +312,19 @@ def main() -> int: return 0 -def startup_status_message(outputs: list[str], startup_error: str | None) -> str | None: +def startup_status_message( + outputs: list, + startup_error: str | None, + backend_id: str, +) -> str | None: if outputs: return None if startup_error: return startup_error - return "No connected display outputs were detected by xrandr." + if backend_id == "ddc": + return ( + "No DDC/CI displays detected. Under COSMIC, greenfix can only adjust " + "external monitors that support DDC/CI — laptop panels are not yet " + "supported. Tracking upstream at pop-os/cosmic-comp#2059." + ) + return "No connected display outputs were detected." diff --git a/greenfix/xrandr.py b/greenfix/xrandr.py index afd2ac9..0357ff1 100644 --- a/greenfix/xrandr.py +++ b/greenfix/xrandr.py @@ -14,6 +14,7 @@ BRIGHTNESS_MAX = 1.20 NEUTRAL_GAMMA = 1.0 NEUTRAL_BRIGHTNESS = 1.0 +SLIDER_DEBOUNCE_MS = 250 WAYLAND_WARNING = ( "This app uses xrandr. Your current session appears to be Wayland, " "so display changes may not work." @@ -156,3 +157,46 @@ def validate_numeric_range(label: str, value: float, minimum: float, maximum: fl def format_xrandr_number(value: float) -> str: return f"{value:.2f}".rstrip("0").rstrip(".") + + +def validate(settings) -> None: + """Raises XrandrError if any field is out of the allowed xrandr range.""" + validate_output_name(settings.output) + validate_gamma_value("red gamma", settings.red_gamma) + validate_gamma_value("green gamma", settings.green_gamma) + validate_gamma_value("blue gamma", settings.blue_gamma) + validate_brightness_value(settings.brightness) + + +class XrandrBackend: + backend_id = "xrandr" + live_preview = True + + def query_outputs(self) -> list: + from greenfix.display import Output + + return [Output(id=name, label=name) for name in query_connected_outputs()] + + def slider_spec(self, channel: str, output_id: str): + from greenfix.display import SliderSpec + + if channel == "brightness": + return SliderSpec(BRIGHTNESS_MIN, BRIGHTNESS_MAX, NEUTRAL_BRIGHTNESS, 0.01, 2) + return SliderSpec(GAMMA_MIN, GAMMA_MAX, NEUTRAL_GAMMA, 0.01, 2) + + def neutral_settings(self, output_id: str): + from greenfix.config import Settings + + return Settings(output=output_id, backend=self.backend_id) + + def apply(self, settings) -> None: + apply_settings( + settings.output, + settings.red_gamma, + settings.green_gamma, + settings.blue_gamma, + settings.brightness, + ) + + def reset(self, output_id: str) -> None: + reset_output(output_id) diff --git a/tests/test_cli.py b/tests/test_cli.py index 95215fc..c89432b 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,110 +1,167 @@ import io import unittest from pathlib import Path -from unittest.mock import Mock, patch +from unittest.mock import MagicMock, Mock, patch -from greenfix import cli, config +from greenfix import cli, config, display + + +def _backend(outputs=("eDP-1",), backend_id="xrandr"): + backend = MagicMock( + spec_set=("backend_id", "live_preview", "query_outputs", "neutral_settings", "apply", "reset") + ) + backend.backend_id = backend_id + backend.live_preview = backend_id == "xrandr" + backend.query_outputs.return_value = [display.Output(id=o, label=o) for o in outputs] + return backend class CliTests(unittest.TestCase): - @patch("greenfix.cli.xrandr.query_connected_outputs", return_value=["eDP-1", "DP-1"]) - def test_list_outputs_prints_connected_outputs(self, query_outputs) -> None: + def test_list_outputs_prints_labels(self) -> None: + backend = _backend(("eDP-1", "DP-1")) stdout = io.StringIO() - status = cli.run(["--list-outputs"], stdout=stdout) + with patch.object(display, "select_backend", return_value=backend): + status = cli.run(["--list-outputs"], stdout=stdout) self.assertEqual(status, 0) self.assertEqual(stdout.getvalue(), "eDP-1\nDP-1\n") - query_outputs.assert_called_once_with() + backend.query_outputs.assert_called_once_with() - @patch("greenfix.cli.xrandr.apply_settings") - @patch("greenfix.cli.config.load_settings") - def test_apply_saved_applies_existing_settings(self, load_settings, apply_settings) -> None: - load_settings.return_value = config.Settings("eDP-1", 1.2, 1.0, 1.25, 1.0) + def test_apply_saved_applies_existing_settings(self) -> None: + backend = _backend() + settings = config.Settings(output="eDP-1", backend="xrandr", red_gamma=1.2) stdout = io.StringIO() - status = cli.run(["--apply-saved"], stdout=stdout, settings_path=Path("settings.json")) + with patch.object(display, "select_backend", return_value=backend), \ + patch.object(config, "load_settings", return_value=settings) as load_mock: + status = cli.run(["--apply-saved"], stdout=stdout, settings_path=Path("settings.json")) self.assertEqual(status, 0) self.assertIn("Applied saved settings for eDP-1.", stdout.getvalue()) - load_settings.assert_called_once_with(Path("settings.json")) - apply_settings.assert_called_once_with("eDP-1", 1.2, 1.0, 1.25, 1.0) - - @patch("greenfix.cli.xrandr.is_wayland_session", return_value=True) - @patch("greenfix.cli.xrandr.apply_settings") - @patch("greenfix.cli.config.load_settings") - def test_apply_saved_warns_on_wayland(self, load_settings, apply_settings, is_wayland) -> None: - load_settings.return_value = config.Settings("eDP-1") + load_mock.assert_called_once_with(Path("settings.json"), active_backend="xrandr") + backend.apply.assert_called_once_with(settings) + + def test_apply_saved_without_settings_exits_gracefully(self) -> None: + backend = _backend() stdout = io.StringIO() - stderr = io.StringIO() - status = cli.run(["--apply-saved"], stdout=stdout, stderr=stderr) + with patch.object(display, "select_backend", return_value=backend), \ + patch.object(config, "load_settings", return_value=None): + status = cli.run(["--apply-saved"], stdout=stdout) self.assertEqual(status, 0) - self.assertIn("current session appears to be Wayland", stderr.getvalue()) - apply_settings.assert_called_once() + self.assertIn("No saved settings", stdout.getvalue()) + backend.apply.assert_not_called() - @patch("greenfix.cli.xrandr.apply_settings") - @patch("greenfix.cli.config.load_settings", return_value=None) - def test_apply_saved_without_settings_exits_gracefully(self, load_settings, apply_settings) -> None: + def test_reset_uses_saved_output_when_no_output_supplied(self) -> None: + backend = _backend() + saved = config.Settings(output="eDP-1", backend="xrandr") stdout = io.StringIO() - status = cli.run(["--apply-saved"], stdout=stdout) + with patch.object(display, "select_backend", return_value=backend), \ + patch.object(config, "load_settings", return_value=saved): + status = cli.run(["--reset"], stdout=stdout) self.assertEqual(status, 0) - self.assertIn("No saved settings found.", stdout.getvalue()) - apply_settings.assert_not_called() + backend.reset.assert_called_once_with("eDP-1") - @patch("greenfix.cli.xrandr.reset_output") - @patch("greenfix.cli.config.load_settings") - def test_reset_uses_saved_output_when_no_output_is_supplied(self, load_settings, reset_output) -> None: - load_settings.return_value = config.Settings("eDP-1") + def test_reset_uses_explicit_output(self) -> None: + backend = _backend() stdout = io.StringIO() - status = cli.run(["--reset"], stdout=stdout) + with patch.object(display, "select_backend", return_value=backend): + status = cli.run(["--reset", "--output", "HDMI-1"], stdout=stdout) self.assertEqual(status, 0) - self.assertIn("Reset eDP-1.", stdout.getvalue()) - reset_output.assert_called_once_with("eDP-1") + backend.reset.assert_called_once_with("HDMI-1") - @patch("greenfix.cli.xrandr.reset_output") - def test_reset_uses_explicit_output(self, reset_output) -> None: + def test_reset_falls_back_to_first_output_when_no_saved(self) -> None: + backend = _backend(("HDMI-1", "DP-1")) stdout = io.StringIO() - status = cli.run(["--reset", "--output", "HDMI-1"], stdout=stdout) + with patch.object(display, "select_backend", return_value=backend), \ + patch.object(config, "load_settings", return_value=None): + status = cli.run(["--reset"], stdout=stdout) self.assertEqual(status, 0) - reset_output.assert_called_once_with("HDMI-1") + backend.reset.assert_called_once_with("HDMI-1") - @patch("greenfix.cli.xrandr.apply_settings") - def test_explicit_apply_uses_provided_values(self, apply_settings) -> None: + def test_explicit_apply_uses_provided_values(self) -> None: + backend = _backend() stdout = io.StringIO() - status = cli.run( - [ - "--output", - "eDP-1", - "--red", - "1.2", - "--green", - "1.0", - "--blue", - "1.25", - "--brightness", - "1.0", - "--apply", - ], - stdout=stdout, - ) + with patch.object(display, "select_backend", return_value=backend): + status = cli.run( + [ + "--output", "eDP-1", + "--red", "1.2", + "--green", "1.0", + "--blue", "1.25", + "--brightness", "1.0", + "--apply", + ], + stdout=stdout, + ) self.assertEqual(status, 0) self.assertIn("Applied settings for eDP-1.", stdout.getvalue()) - apply_settings.assert_called_once_with("eDP-1", 1.2, 1.0, 1.25, 1.0) + backend.apply.assert_called_once() + applied = backend.apply.call_args.args[0] + self.assertEqual(applied.output, "eDP-1") + self.assertEqual(applied.backend, "xrandr") + self.assertEqual(applied.red_gamma, 1.2) + self.assertEqual(applied.blue_gamma, 1.25) + + def test_list_outputs_includes_output_id_for_ddc(self) -> None: + backend = _backend(backend_id="ddc") + backend.query_outputs.return_value = [display.Output(id="ddc:4", label="4: DELL U2412M")] + stdout = io.StringIO() + + with patch.object(display, "select_backend", return_value=backend): + status = cli.run(["--list-outputs"], stdout=stdout) + + self.assertEqual(status, 0) + self.assertIn("ddc:4", stdout.getvalue()) + + def test_apply_fills_omitted_channels_from_backend_neutral(self) -> None: + backend = _backend(backend_id="ddc") + backend.neutral_settings.return_value = config.Settings( + output="ddc:4", backend="ddc", + red_gamma=100, green_gamma=95, blue_gamma=100, brightness=75, + ) + stdout = io.StringIO() + + with patch.object(display, "select_backend", return_value=backend): + status = cli.run(["--output", "ddc:4", "--brightness", "60", "--apply"], stdout=stdout) + + self.assertEqual(status, 0) + backend.neutral_settings.assert_called_once_with("ddc:4") + applied = backend.apply.call_args.args[0] + self.assertEqual(applied.brightness, 60) # explicit value honored + self.assertEqual(applied.red_gamma, 100) # omitted -> snapshot, not 1.0 + self.assertEqual(applied.green_gamma, 95) + self.assertEqual(applied.blue_gamma, 100) + + def test_apply_omitted_channels_default_to_xrandr_neutral(self) -> None: + backend = _backend() + backend.neutral_settings.return_value = config.Settings(output="eDP-1", backend="xrandr") + stdout = io.StringIO() + + with patch.object(display, "select_backend", return_value=backend): + status = cli.run(["--output", "eDP-1", "--red", "1.2", "--apply"], stdout=stdout) + + self.assertEqual(status, 0) + applied = backend.apply.call_args.args[0] + self.assertEqual(applied.red_gamma, 1.2) # explicit + self.assertEqual(applied.green_gamma, 1.0) # omitted -> neutral def test_default_launches_ui(self) -> None: + backend = _backend() launch_ui = Mock(return_value=0) - status = cli.run([], launch_ui=launch_ui) + with patch.object(display, "select_backend", return_value=backend): + status = cli.run([], launch_ui=launch_ui) self.assertEqual(status, 0) launch_ui.assert_called_once_with() diff --git a/tests/test_config.py b/tests/test_config.py index a41a140..925c172 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -42,6 +42,7 @@ def test_save_and_load_settings_round_trip(self) -> None: path = Path(directory) / "greenfix" / "settings.json" settings = config.Settings( output="eDP-1", + backend="xrandr", red_gamma=1.2, green_gamma=1.0, blue_gamma=1.25, @@ -62,6 +63,7 @@ def test_saved_json_has_expected_keys(self) -> None: json.loads(path.read_text(encoding="utf-8")), { "output": "eDP-1", + "backend": "xrandr", "red_gamma": 1.2, "green_gamma": 1.0, "blue_gamma": 1.0, @@ -72,12 +74,69 @@ def test_saved_json_has_expected_keys(self) -> None: def test_neutral_settings_uses_identity_values(self) -> None: settings = config.neutral_settings("eDP-1") - self.assertEqual(settings, config.Settings("eDP-1", 1.0, 1.0, 1.0, 1.0)) + self.assertEqual(settings, config.Settings("eDP-1", "xrandr", 1.0, 1.0, 1.0, 1.0)) def test_invalid_settings_values_are_rejected(self) -> None: with self.assertRaisesRegex(config.ConfigError, "brightness"): config.Settings(output="eDP-1", brightness=1.3) +class ConfigBackendFieldTests(unittest.TestCase): + def test_settings_default_backend_is_xrandr(self) -> None: + self.assertEqual(config.Settings(output="eDP-1").backend, "xrandr") + + def test_settings_accepts_explicit_backend(self) -> None: + self.assertEqual(config.Settings(output="eDP-1", backend="xrandr").backend, "xrandr") + + def test_save_includes_backend_field(self) -> None: + with tempfile.TemporaryDirectory() as directory: + path = Path(directory) / "settings.json" + config.save_settings(config.Settings(output="eDP-1", red_gamma=1.2), path) + payload = json.loads(path.read_text(encoding="utf-8")) + + self.assertEqual(payload["backend"], "xrandr") + + def test_load_untagged_file_defaults_to_xrandr(self) -> None: + with tempfile.TemporaryDirectory() as directory: + path = Path(directory) / "settings.json" + path.write_text(json.dumps({"output": "eDP-1", "red_gamma": 1.1}), encoding="utf-8") + loaded = config.load_settings(path) + + self.assertIsNotNone(loaded) + self.assertEqual(loaded.backend, "xrandr") + self.assertEqual(loaded.red_gamma, 1.1) + + +class ConfigDispatchTests(unittest.TestCase): + def test_post_init_dispatches_via_backend_module(self) -> None: + # xrandr backend rejects gamma 0.5 → ConfigError + with self.assertRaisesRegex(config.ConfigError, "red gamma"): + config.Settings(output="eDP-1", backend="xrandr", red_gamma=0.5) + + def test_post_init_unknown_backend_raises(self) -> None: + with self.assertRaisesRegex(config.ConfigError, "Unknown backend"): + config.Settings(output="x", backend="nope") + + def test_load_returns_none_when_active_backend_does_not_match(self) -> None: + with tempfile.TemporaryDirectory() as directory: + path = Path(directory) / "settings.json" + path.write_text( + json.dumps({"output": "ddc:4", "backend": "ddc", "red_gamma": 80}), + encoding="utf-8", + ) + self.assertIsNone(config.load_settings(path, active_backend="xrandr")) + + def test_load_returns_settings_when_active_backend_matches(self) -> None: + with tempfile.TemporaryDirectory() as directory: + path = Path(directory) / "settings.json" + payload = {"output": "eDP-1", "backend": "xrandr", "red_gamma": 1.1} + path.write_text(json.dumps(payload), encoding="utf-8") + + loaded = config.load_settings(path, active_backend="xrandr") + + self.assertIsNotNone(loaded) + self.assertEqual(loaded.backend, "xrandr") + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_ddc.py b/tests/test_ddc.py new file mode 100644 index 0000000..230d2e3 --- /dev/null +++ b/tests/test_ddc.py @@ -0,0 +1,321 @@ +import subprocess +import unittest +from unittest.mock import patch + +from greenfix import ddc + + +DDCUTIL_DETECT = """Display 1 + I2C bus: /dev/i2c-4 + EDID synopsis: + Mfg id: DEL - Dell Inc. + Model: DELL U2723QE + Product code: 16505 (0x4079) + Serial number: ABC123 + Manufacture year: 2023, Week: 12 + VCP version: 2.2 + +Display 2 + I2C bus: /dev/i2c-7 + EDID synopsis: + Mfg id: BNQ + Model: BenQ GW2480 + Product code: 30000 (0x7530) + Serial number: XYZ789 + Manufacture year: 2021 + VCP version: 2.1 +""" + +DDCUTIL_GETVCP = """VCP code 0x10 (Brightness ): current value = 75, max value = 100 +VCP code 0x16 (Video gain: Red ): current value = 100, max value = 100 +VCP code 0x18 (Video gain: Green ): current value = 95, max value = 100 +VCP code 0x1a (Video gain: Blue ): current value = 100, max value = 100 +""" + +# A monitor that answers brightness but not the RGB gain codes. +DDCUTIL_GETVCP_INCOMPLETE = """VCP code 0x10 (Brightness ): current value = 50, max value = 100 +""" + + +class ParseDetectTests(unittest.TestCase): + def test_parses_two_displays(self) -> None: + displays = ddc.parse_detect(DDCUTIL_DETECT) + + self.assertEqual(len(displays), 2) + self.assertEqual(displays[0].bus, 4) + self.assertEqual(displays[0].model, "DELL U2723QE") + self.assertEqual(displays[1].bus, 7) + self.assertEqual(displays[1].model, "BenQ GW2480") + + def test_excludes_invalid_and_phantom_displays(self) -> None: + # ddcutil emits three column-0 block headers: "Display N" (valid), + # "Invalid display" (DDC failed), and "Phantom display N". Only valid + # blocks may be returned; an invalid block's bus/model must not leak + # into the following valid block. + output = """Invalid display + I2C bus: /dev/i2c-5 + EDID synopsis: + Model: S90F + DDC communication failed + +Invalid display + I2C bus: /dev/i2c-6 + EDID synopsis: + Model: Philips FTV + DDC communication failed + +Display 1 + I2C bus: /dev/i2c-7 + EDID synopsis: + Model: DELL U2412M + VCP version: 2.1 + +Phantom display 2 + I2C bus: /dev/i2c-9 + EDID synopsis: + Model: Ghost Monitor +""" + displays = ddc.parse_detect(output) + + self.assertEqual([(d.bus, d.model) for d in displays], [(7, "DELL U2412M")]) + + def test_empty_input_returns_empty_list(self) -> None: + self.assertEqual(ddc.parse_detect(""), []) + + def test_garbage_returns_empty_list(self) -> None: + self.assertEqual(ddc.parse_detect("nothing useful"), []) + + +class ParseGetvcpTests(unittest.TestCase): + def test_parses_four_codes(self) -> None: + values = ddc.parse_getvcp(DDCUTIL_GETVCP) + + self.assertEqual(values[0x10], (75, 100)) + self.assertEqual(values[0x16], (100, 100)) + self.assertEqual(values[0x18], (95, 100)) + self.assertEqual(values[0x1a], (100, 100)) + + def test_missing_code_absent_from_result(self) -> None: + values = ddc.parse_getvcp( + "VCP code 0x10 (Brightness): current value = 50, max value = 100" + ) + + self.assertIn(0x10, values) + self.assertNotIn(0x16, values) + + +class CommandBuilderTests(unittest.TestCase): + def test_build_detect_command(self) -> None: + self.assertEqual(ddc.build_detect_command(), ["ddcutil", "detect"]) + + def test_build_getvcp_command(self) -> None: + self.assertEqual( + ddc.build_getvcp_command(bus=4, codes=[0x10, 0x16, 0x18, 0x1A]), + ["ddcutil", "--bus", "4", "getvcp", "10", "16", "18", "1A"], + ) + + def test_build_setvcp_command(self) -> None: + self.assertEqual( + ddc.build_setvcp_command(bus=4, code=0x18, value=80), + ["ddcutil", "--bus", "4", "setvcp", "18", "80"], + ) + + +class SubprocessWrapperTests(unittest.TestCase): + def test_run_detect_returns_stdout(self) -> None: + completed = subprocess.CompletedProcess(args=[], returncode=0, stdout="ok", stderr="") + with patch.object(subprocess, "run", return_value=completed): + self.assertEqual(ddc.run_detect(), "ok") + + def test_run_detect_raises_when_ddcutil_missing(self) -> None: + with patch.object(subprocess, "run", side_effect=FileNotFoundError): + with self.assertRaisesRegex(ddc.DdcError, "ddcutil was not found"): + ddc.run_detect() + + def test_run_setvcp_raises_on_calledprocess_error(self) -> None: + err = subprocess.CalledProcessError(returncode=1, cmd=["ddcutil"], stderr="permission denied") + with patch.object(subprocess, "run", side_effect=err): + with self.assertRaisesRegex(ddc.DdcError, "permission denied"): + ddc.run_setvcp(bus=4, code=0x18, value=80) + + +class DdcBackendQueryTests(unittest.TestCase): + def test_backend_id_and_live_preview(self) -> None: + backend = ddc.DdcBackend() + + self.assertEqual(backend.backend_id, "ddc") + self.assertFalse(backend.live_preview) + + def test_query_outputs_maps_to_output_dataclass(self) -> None: + backend = ddc.DdcBackend() + + with patch.object(ddc, "run_detect", return_value=DDCUTIL_DETECT), \ + patch.object(ddc, "run_getvcp", return_value=DDCUTIL_GETVCP): + outputs = backend.query_outputs() + + self.assertEqual(len(outputs), 2) + self.assertEqual(outputs[0].id, "ddc:4") + self.assertEqual(outputs[0].label, "4: DELL U2723QE") + self.assertEqual(outputs[1].id, "ddc:7") + + def test_query_outputs_snapshots_each_display_once(self) -> None: + backend = ddc.DdcBackend() + + with patch.object(ddc, "run_detect", return_value=DDCUTIL_DETECT), \ + patch.object(ddc, "run_getvcp", return_value=DDCUTIL_GETVCP) as getvcp_mock: + backend.query_outputs() + backend.query_outputs() + + # Two displays, snapshotted on first observation, not re-fetched on second. + self.assertEqual(getvcp_mock.call_count, 2) + + def test_query_outputs_returns_empty_when_no_displays(self) -> None: + backend = ddc.DdcBackend() + with patch.object(ddc, "run_detect", return_value=""): + self.assertEqual(backend.query_outputs(), []) + + +class _PopulatedBackendMixin: + def _populated(self): + backend = ddc.DdcBackend() + with patch.object(ddc, "run_detect", return_value=DDCUTIL_DETECT), \ + patch.object(ddc, "run_getvcp", return_value=DDCUTIL_GETVCP): + backend.query_outputs() + return backend + + +class DdcBackendSpecTests(_PopulatedBackendMixin, unittest.TestCase): + def test_slider_spec_for_brightness(self) -> None: + spec = self._populated().slider_spec("brightness", "ddc:4") + + self.assertEqual((spec.minimum, spec.maximum, spec.neutral), (0, 100, 75)) + self.assertEqual(spec.digits, 0) + + def test_slider_spec_for_green(self) -> None: + spec = self._populated().slider_spec("green", "ddc:4") + + self.assertEqual((spec.minimum, spec.maximum, spec.neutral), (0, 100, 95)) + + def test_slider_spec_surfaces_getvcp_failure_as_ddcerror(self) -> None: + # No snapshot cached: the backend lazily reads the bus, and a getvcp + # failure must surface as DdcError rather than a raw subprocess error. + with patch.object(ddc, "run_getvcp", side_effect=ddc.DdcError("boom")): + with self.assertRaises(ddc.DdcError): + ddc.DdcBackend().slider_spec("red", "ddc:99") + + def test_neutral_settings_returns_snapshot_values(self) -> None: + settings = self._populated().neutral_settings("ddc:4") + + self.assertEqual(settings.output, "ddc:4") + self.assertEqual(settings.backend, "ddc") + self.assertEqual(settings.red_gamma, 100) + self.assertEqual(settings.green_gamma, 95) + self.assertEqual(settings.blue_gamma, 100) + self.assertEqual(settings.brightness, 75) + + +class DdcBackendApplyTests(_PopulatedBackendMixin, unittest.TestCase): + def test_apply_calls_setvcp_in_order(self) -> None: + from greenfix import config + backend = self._populated() + settings = config.Settings( + output="ddc:4", backend="ddc", + red_gamma=90, green_gamma=70, blue_gamma=100, brightness=60, + ) + + with patch.object(ddc, "run_setvcp") as setvcp_mock: + backend.apply(settings) + + self.assertEqual(setvcp_mock.call_count, 4) + self.assertEqual( + [call.args for call in setvcp_mock.call_args_list], + [(4, 0x10, 60), (4, 0x16, 90), (4, 0x18, 70), (4, 0x1A, 100)], + ) + + def test_reset_writes_snapshot_back(self) -> None: + backend = self._populated() + + with patch.object(ddc, "run_setvcp") as setvcp_mock: + backend.reset("ddc:4") + + applied = {call.args[1]: call.args[2] for call in setvcp_mock.call_args_list} + self.assertEqual(applied[0x10], 75) + self.assertEqual(applied[0x16], 100) + self.assertEqual(applied[0x18], 95) + self.assertEqual(applied[0x1A], 100) + + def test_apply_rejects_settings_for_wrong_backend(self) -> None: + from greenfix import config + backend = self._populated() + + with self.assertRaises(ddc.DdcError): + backend.apply(config.Settings(output="eDP-1", backend="xrandr")) + + +class DdcBackendLazySnapshotTests(unittest.TestCase): + def test_reset_lazily_snapshots_uncached_output(self) -> None: + # Cold CLI reset: no query_outputs() was called first, so the backend + # must read the bus on demand instead of raising "No snapshot". + backend = ddc.DdcBackend() + + with patch.object(ddc, "run_getvcp", return_value=DDCUTIL_GETVCP) as getvcp_mock, \ + patch.object(ddc, "run_setvcp") as setvcp_mock: + backend.reset("ddc:7") + + getvcp_mock.assert_called_once() + applied = {call.args[1]: call.args[2] for call in setvcp_mock.call_args_list} + self.assertEqual(applied[0x10], 75) + self.assertEqual(applied[0x16], 100) + + def test_neutral_settings_lazily_snapshots_uncached_output(self) -> None: + backend = ddc.DdcBackend() + + with patch.object(ddc, "run_getvcp", return_value=DDCUTIL_GETVCP): + settings = backend.neutral_settings("ddc:7") + + self.assertEqual(settings.brightness, 75) + self.assertEqual(settings.green_gamma, 95) + + def test_query_outputs_skips_displays_missing_required_codes(self) -> None: + backend = ddc.DdcBackend() + + def fake_getvcp(bus, codes): + return DDCUTIL_GETVCP if bus == 4 else DDCUTIL_GETVCP_INCOMPLETE + + with patch.object(ddc, "run_detect", return_value=DDCUTIL_DETECT), \ + patch.object(ddc, "run_getvcp", side_effect=fake_getvcp): + outputs = backend.query_outputs() + + self.assertEqual([o.id for o in outputs], ["ddc:4"]) + + def test_neutral_settings_raises_ddcerror_for_incomplete_monitor(self) -> None: + backend = ddc.DdcBackend() + + with patch.object(ddc, "run_getvcp", return_value=DDCUTIL_GETVCP_INCOMPLETE): + with self.assertRaisesRegex(ddc.DdcError, "VCP"): + backend.neutral_settings("ddc:7") + + +class DdcValidateTests(unittest.TestCase): + def test_validate_accepts_in_range(self) -> None: + from greenfix import config + ddc.validate(config.Settings(output="ddc:4", backend="ddc", red_gamma=80)) + + def test_settings_rejects_value_above_100(self) -> None: + from greenfix import config + with self.assertRaisesRegex(config.ConfigError, "red"): + config.Settings(output="ddc:4", backend="ddc", red_gamma=120) + + def test_settings_rejects_negative_value(self) -> None: + from greenfix import config + with self.assertRaisesRegex(config.ConfigError, "brightness"): + config.Settings(output="ddc:4", backend="ddc", brightness=-1) + + def test_settings_rejects_bad_output_id(self) -> None: + from greenfix import config + with self.assertRaisesRegex(config.ConfigError, "output"): + config.Settings(output="eDP-1", backend="ddc") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_display.py b/tests/test_display.py new file mode 100644 index 0000000..ede4762 --- /dev/null +++ b/tests/test_display.py @@ -0,0 +1,51 @@ +import os +import unittest +from unittest.mock import patch + +from greenfix import display, xrandr + + +class SliderSpecTests(unittest.TestCase): + def test_holds_fields(self) -> None: + spec = display.SliderSpec(minimum=0.6, maximum=1.8, neutral=1.0, step=0.01, digits=2) + + self.assertEqual(spec.minimum, 0.6) + self.assertEqual(spec.maximum, 1.8) + self.assertEqual(spec.neutral, 1.0) + self.assertEqual(spec.step, 0.01) + self.assertEqual(spec.digits, 2) + + def test_is_frozen(self) -> None: + spec = display.SliderSpec(minimum=0.0, maximum=1.0, neutral=1.0, step=0.01, digits=2) + + with self.assertRaises(Exception): + spec.minimum = 2.0 # type: ignore[misc] + + +class OutputTests(unittest.TestCase): + def test_holds_id_and_label(self) -> None: + out = display.Output(id="eDP-1", label="eDP-1") + + self.assertEqual(out.id, "eDP-1") + self.assertEqual(out.label, "eDP-1") + + +class SelectBackendTests(unittest.TestCase): + def test_returns_xrandr_backend_on_x11(self) -> None: + with patch.dict(os.environ, {"XDG_SESSION_TYPE": "x11"}, clear=False): + self.assertIsInstance(display.select_backend(), xrandr.XrandrBackend) + + def test_returns_xrandr_backend_when_session_type_missing(self) -> None: + env = {k: v for k, v in os.environ.items() if k != "XDG_SESSION_TYPE"} + with patch.dict(os.environ, env, clear=True): + self.assertIsInstance(display.select_backend(), xrandr.XrandrBackend) + + def test_returns_ddc_backend_on_wayland(self) -> None: + from greenfix import ddc + with patch.dict(os.environ, {"XDG_SESSION_TYPE": "wayland"}, clear=False): + backend = display.select_backend() + self.assertIsInstance(backend, ddc.DdcBackend) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_ui.py b/tests/test_ui.py index d6529ed..4da2579 100644 --- a/tests/test_ui.py +++ b/tests/test_ui.py @@ -1,23 +1,93 @@ import unittest -from greenfix import ui +from greenfix import config, display, ui -class UiTests(unittest.TestCase): - def test_startup_status_prefers_specific_startup_error(self) -> None: - message = ui.startup_status_message([], "xrandr was not found.") +class _FakeBackend: + """Backend stub whose specs/neutrals differ per output, so a test can prove + output_view reads from the *selected* output rather than a cached one.""" - self.assertEqual(message, "xrandr was not found.") + def __init__(self, specs, neutrals): + self._specs = specs + self._neutrals = neutrals - def test_startup_status_warns_when_no_outputs_are_detected(self) -> None: - message = ui.startup_status_message([], None) + def slider_spec(self, channel, output_id): + return self._specs[output_id][channel] - self.assertEqual(message, "No connected display outputs were detected by xrandr.") + def neutral_settings(self, output_id): + return self._neutrals[output_id] - def test_startup_status_is_empty_when_outputs_exist(self) -> None: - message = ui.startup_status_message(["eDP-1"], "ignored") - self.assertIsNone(message) +def _specs(neutral): + return {ch: display.SliderSpec(0, 100, neutral, 1, 0) for ch in ("red", "green", "blue", "brightness")} + + +class OutputViewTests(unittest.TestCase): + def _backend(self): + return _FakeBackend( + specs={"ddc:4": _specs(75), "ddc:7": _specs(40)}, + neutrals={ + "ddc:4": config.Settings(output="ddc:4", backend="ddc", brightness=75, + red_gamma=75, green_gamma=75, blue_gamma=75), + "ddc:7": config.Settings(output="ddc:7", backend="ddc", brightness=40, + red_gamma=40, green_gamma=40, blue_gamma=40), + }, + ) + + def test_reads_specs_and_settings_from_selected_output(self) -> None: + backend = self._backend() + + view4 = ui.output_view(backend, None, "ddc:4") + view7 = ui.output_view(backend, None, "ddc:7") + + self.assertEqual(view4.brightness.neutral, 75) + self.assertEqual(view7.brightness.neutral, 40) + self.assertEqual(view4.settings.brightness, 75) + self.assertEqual(view7.settings.brightness, 40) + + def test_prefers_saved_settings_for_matching_output(self) -> None: + backend = self._backend() + saved = config.Settings(output="ddc:4", backend="ddc", brightness=20, + red_gamma=10, green_gamma=10, blue_gamma=10) + + view = ui.output_view(backend, saved, "ddc:4") + + self.assertEqual(view.settings.brightness, 20) + + def test_ignores_saved_settings_for_different_output(self) -> None: + backend = self._backend() + saved = config.Settings(output="ddc:4", backend="ddc", brightness=20, + red_gamma=10, green_gamma=10, blue_gamma=10) + + view = ui.output_view(backend, saved, "ddc:7") + + self.assertEqual(view.settings.brightness, 40) + + +class StartupStatusMessageTests(unittest.TestCase): + def test_returns_none_when_outputs_exist(self) -> None: + self.assertIsNone( + ui.startup_status_message( + outputs=[display.Output("eDP-1", "eDP-1")], + startup_error=None, + backend_id="xrandr", + ) + ) + + def test_startup_error_passes_through(self) -> None: + message = ui.startup_status_message(outputs=[], startup_error="boom", backend_id="xrandr") + self.assertEqual(message, "boom") + + def test_xrandr_no_outputs_default_message(self) -> None: + message = ui.startup_status_message(outputs=[], startup_error=None, backend_id="xrandr") + self.assertIn("No connected display outputs", message) + + def test_ddc_no_outputs_mentions_cosmic_issue(self) -> None: + message = ui.startup_status_message(outputs=[], startup_error=None, backend_id="ddc") + + self.assertIn("No DDC/CI displays", message) + self.assertIn("cosmic-comp", message) + self.assertIn("2059", message) if __name__ == "__main__": diff --git a/tests/test_xrandr.py b/tests/test_xrandr.py index 1660d56..84f5373 100644 --- a/tests/test_xrandr.py +++ b/tests/test_xrandr.py @@ -1,8 +1,9 @@ import subprocess +import types import unittest from unittest.mock import patch -from greenfix import xrandr +from greenfix import xrandr, config, display XRANDR_QUERY = """Screen 0: minimum 8 x 8, current 1920 x 1080, maximum 32767 x 32767 @@ -91,5 +92,78 @@ def test_apply_settings_uses_argument_list_without_shell(self, run) -> None: ) +class XrandrBackendTests(unittest.TestCase): + def test_backend_id_is_xrandr(self) -> None: + self.assertEqual(xrandr.XrandrBackend().backend_id, "xrandr") + + def test_live_preview_is_enabled(self) -> None: + self.assertTrue(xrandr.XrandrBackend().live_preview) + + def test_slider_spec_for_gamma_channels(self) -> None: + backend = xrandr.XrandrBackend() + for channel in ("red", "green", "blue"): + spec = backend.slider_spec(channel, "eDP-1") + self.assertEqual((spec.minimum, spec.maximum, spec.neutral), (0.60, 1.80, 1.0)) + self.assertEqual(spec.digits, 2) + + def test_slider_spec_for_brightness(self) -> None: + spec = xrandr.XrandrBackend().slider_spec("brightness", "eDP-1") + self.assertEqual((spec.minimum, spec.maximum, spec.neutral), (0.60, 1.20, 1.0)) + + def test_query_outputs_wraps_names(self) -> None: + with patch.object(xrandr, "query_connected_outputs", return_value=["eDP-1", "DP-1"]): + outputs = xrandr.XrandrBackend().query_outputs() + self.assertEqual(outputs, [display.Output("eDP-1", "eDP-1"), display.Output("DP-1", "DP-1")]) + + def test_neutral_settings_returns_unit_values(self) -> None: + settings = xrandr.XrandrBackend().neutral_settings("eDP-1") + self.assertEqual(settings.output, "eDP-1") + self.assertEqual(settings.backend, "xrandr") + self.assertEqual(settings.red_gamma, 1.0) + self.assertEqual(settings.brightness, 1.0) + + def test_apply_calls_apply_settings(self) -> None: + settings = config.Settings(output="eDP-1", backend="xrandr", red_gamma=1.1) + with patch.object(xrandr, "apply_settings") as apply_mock: + xrandr.XrandrBackend().apply(settings) + apply_mock.assert_called_once_with("eDP-1", 1.1, 1.0, 1.0, 1.0) + + def test_reset_calls_reset_output(self) -> None: + with patch.object(xrandr, "reset_output") as reset_mock: + xrandr.XrandrBackend().reset("eDP-1") + reset_mock.assert_called_once_with("eDP-1") + + +class XrandrValidateTests(unittest.TestCase): + def test_validate_accepts_neutral(self) -> None: + xrandr.validate(config.Settings(output="eDP-1", backend="xrandr")) + + def test_validate_rejects_gamma_out_of_range(self) -> None: + # Use SimpleNamespace to bypass Settings.__post_init__ so we test + # xrandr.validate() directly with an out-of-range value. + fake = types.SimpleNamespace( + output="eDP-1", + red_gamma=0.5, + green_gamma=1.0, + blue_gamma=1.0, + brightness=1.0, + ) + with self.assertRaisesRegex(xrandr.XrandrError, "red gamma"): + xrandr.validate(fake) + + def test_validate_rejects_bad_output(self) -> None: + # Use SimpleNamespace to bypass Settings.__post_init__ so we test + # xrandr.validate() directly with an invalid output name. + fake = types.SimpleNamespace( + output="bad;name", + red_gamma=1.0, + green_gamma=1.0, + blue_gamma=1.0, + brightness=1.0, + ) + with self.assertRaisesRegex(xrandr.XrandrError, "Invalid display output"): + xrandr.validate(fake) + + if __name__ == "__main__": unittest.main()