Address monitors by I2C bus and serialize writes per display to cut brightness latency

monitorsettings/base.py

@@ -8,6 +8,14 @@
 import time
 from typing import Dict, List, Optional, Tuple
 
+# ddcutil flags for the per-change path. Addressing a monitor by display
+# number (-d N) makes every ddcutil run re-detect all monitors first (~1 s on a
+# machine with many I2C buses); --bus N skips that. --noverify drops the
+# read-back after each write and --skip-ddc-checks the per-run capability
+# probe. Measured on 2 x LG 4K over HDMI: -d 1.0-1.2 s, --bus + flags 0.10 s.
+SET_FLAGS = ["--noverify", "--skip-ddc-checks"]
+GET_FLAGS = ["--skip-ddc-checks"]
+
 
 class DDCInterface:
     """
@@ -17,9 +25,12 @@ class DDCInterface:
 
     def __init__(self) -> None:
         self.displays: List[int] = []
+        self.buses: Dict[int, int] = {}  # display number -> I2C bus, from detect
         self._lock = threading.Lock()
         self._last_command_time = 0.0
-        self._command_interval = 1.0  # inimum time between DDC commands
+        # Minimum time between synchronous DDC commands. ddcutil already
+        # enforces the DDC/CI protocol delays itself, so no extra gap is needed.
+        self._command_interval = 0.0
 
     def check_ddcutil(self) -> bool:
         """Check if ddcutil is available on the system"""
@@ -43,11 +54,23 @@ def detect_displays(self) -> List[int]:
 
             displays = re.findall(r"Display (\d+)", result.stdout)
             self.displays = [int(d) for d in displays]
+            # Remember each display's I2C bus so later commands can skip detection.
+            self.buses = {}
+            for block in re.split(r"(?m)^(?=Display \d+)", result.stdout):
+                d = re.match(r"Display (\d+)", block)
+                b = re.search(r"I2C bus:\s*/dev/i2c-(\d+)", block)
+                if d and b:
+                    self.buses[int(d.group(1))] = int(b.group(1))
             return self.displays
 
         except (subprocess.TimeoutExpired, subprocess.CalledProcessError, FileNotFoundError):
             return []
 
+    def _target(self, display: int) -> List[str]:
+        """ddcutil arguments selecting a display: its bus if known, else its number."""
+        bus = self.buses.get(display)
+        return ["--bus", str(bus)] if bus is not None else ["-d", str(display)]
+
     def get_vcp_value(self, display: int, vcp_code: str) -> Tuple[Optional[int], Optional[int]]:
         """
         Get a VCP (Virtual Control Panel) value from a display
@@ -64,7 +87,7 @@ def get_vcp_value(self, display: int, vcp_code: str) -> Tuple[Optional[int], Opt
                 self._wait_for_command_interval()
 
                 result = subprocess.run(
-                    ["ddcutil", "getvcp", vcp_code, "-d", str(display)],
+                    ["ddcutil", "getvcp", vcp_code, *self._target(display), *GET_FLAGS],
                     capture_output=True,
                     text=True,
                     timeout=2,
@@ -98,7 +121,7 @@ def set_vcp_value(self, display: int, vcp_code: str, value: int) -> bool:
                 self._wait_for_command_interval()
 
                 subprocess.run(
-                    ["ddcutil", "setvcp", vcp_code, str(value), "-d", str(display)],
+                    ["ddcutil", "setvcp", vcp_code, str(value), *self._target(display), *SET_FLAGS],
                     capture_output=True,
                     timeout=3,
                     check=True,
@@ -121,7 +144,7 @@ def set_vcp_value_async(self, display: int, vcp_code: str, value: int) -> subpro
             Popen process object
         """
         return subprocess.Popen(
-            ["ddcutil", "setvcp", vcp_code, str(value), "-d", str(display)],
+            ["ddcutil", "setvcp", vcp_code, str(value), *self._target(display), *SET_FLAGS],
             stdout=subprocess.DEVNULL,
             stderr=subprocess.DEVNULL,
         )
@@ -139,18 +162,24 @@ def _wait_for_command_interval(self) -> None:
 
 class AsyncDDCWorker:
     """
-    Background worker for sending DDC commands asynchronously with debouncing
+    Background worker that sends DDC commands asynchronously.
+
+    At most one ddcutil process is in flight per display (they share one I2C
+    bus), and when it finishes the newest pending value is sent next, so
+    intermediate values are skipped and writes can never land out of order.
     """
 
-    def __init__(self, ddc_interface: DDCInterface, update_interval: float = 0.5):
+    def __init__(self, ddc_interface: DDCInterface, update_interval: float = 0.0):
         self.ddc = ddc_interface
+        # Optional extra gap between writes to the same display (seconds).
         self.update_interval = update_interval
         self.running = False
         self._thread: Optional[threading.Thread] = None
         self._lock = threading.Lock()
         self._pending_updates: Dict[Tuple[int, str], int] = {}
         self._last_sent: Dict[Tuple[int, str], int] = {}
-        self._last_update_time = 0.0
+        self._in_flight: Dict[int, subprocess.Popen] = {}
+        self._last_send_time: Dict[int, float] = {}
 
     def start(self) -> None:
         """Start the background worker thread"""
@@ -177,36 +206,36 @@ def queue_update(self, display: int, vcp_code: str, value: int) -> None:
         with self._lock:
             self._pending_updates[(display, vcp_code)] = value
 
+    def _busy(self, display: int) -> bool:
+        """True while a write to this display is still running."""
+        p = self._in_flight.get(display)
+        if p is None:
+            return False
+        if p.poll() is None:
+            return True
+        del self._in_flight[display]
+        return False
+
     def _worker_loop(self) -> None:
-        """Main worker loop for processing queued updates"""
+        """Send the newest pending value for each display as soon as it is free."""
         while self.running:
-            time.sleep(0.05)  # Check every 50ms
-
-            current_time = time.time()
-            if current_time - self._last_update_time < self.update_interval:
-                continue
-
-            updates_to_send = []
-
+            time.sleep(0.01)
+            now = time.time()
+            to_send = []
+            claimed = set()  # one write per display per pass
             with self._lock:
-                for key, value in self._pending_updates.items():
-                    if self._last_sent.get(key) != value:
-                        updates_to_send.append((key, value))
-                        self._last_sent[key] = value
-
-                self._pending_updates.clear()
-
-            if updates_to_send:
-                self._last_update_time = current_time
-                processes = []
-
-                for (display, vcp_code), value in updates_to_send:
-                    p = self.ddc.set_vcp_value_async(display, vcp_code, value)
-                    processes.append(p)
-
-                # Don't wait for completion - fire and forget
-                for p in processes:
-                    try:
-                        p.wait(timeout=0.01)
-                    except subprocess.TimeoutExpired:
-                        pass
+                for key, value in list(self._pending_updates.items()):
+                    display = key[0]
+                    if display in claimed or self._busy(display):
+                        continue  # keep it pending; the newest value wins
+                    if now - self._last_send_time.get(display, 0.0) < self.update_interval:
+                        continue
+                    del self._pending_updates[key]
+                    if self._last_sent.get(key) == value:
+                        continue
+                    self._last_sent[key] = value
+                    claimed.add(display)
+                    to_send.append((key, value))
+            for (display, vcp_code), value in to_send:
+                self._in_flight[display] = self.ddc.set_vcp_value_async(display, vcp_code, value)
+                self._last_send_time[display] = now

monitorsettings/controllers/backlight/blessed.py

@@ -42,7 +42,6 @@ def run(self) -> None:
             print(f"  Display {display}: {current}/{max_val}")
 
         print("\nStarting interactive mode...\n")
-        time.sleep(1)
 
         # Start background worker
         self.start_worker()

monitorsettings/controllers/backlight/curses.py

@@ -100,7 +100,6 @@ def _init_displays(self) -> bool:
             self.stdscr.addstr(3 + i, 2, f"Display {display}: {current}/{max_val}")
             self.stdscr.refresh()
 
-        time.sleep(0.5)
         return True
 
     def draw_interface(self) -> None:

tests/unit/test_base.py

@@ -56,6 +56,25 @@ def test_detect_displays(self, ddc_interface, mocker):
 
         assert displays == [1, 2]
         assert ddc_interface.displays == [1, 2]
+        assert ddc_interface.buses == {1: 1, 2: 2}
+
+    def test_commands_address_detected_bus(self, ddc_interface, mocker):
+        """After detection, commands use --bus (no per-command detection)"""
+        mock_run = mocker.patch("subprocess.run")
+        mock_run.return_value = MagicMock(
+            stdout="Display 1\n   I2C bus:  /dev/i2c-2\n\nDisplay 2\n   I2C bus:  /dev/i2c-4\n"
+        )
+        ddc_interface.detect_displays()
+
+        mock_run.return_value = MagicMock(returncode=0)
+        ddc_interface.set_vcp_value(2, "0x10", 30)
+
+        mock_run.assert_called_with(
+            ["ddcutil", "setvcp", "0x10", "30", "--bus", "4", "--noverify", "--skip-ddc-checks"],
+            capture_output=True,
+            timeout=3,
+            check=True,
+        )
 
     def test_detect_displays_none_found(self, ddc_interface, mocker):
         """Test display detection when no displays found"""
@@ -106,7 +125,7 @@ def test_set_vcp_value(self, ddc_interface, mocker):
 
         assert result is True
         mock_run.assert_called_with(
-            ["ddcutil", "setvcp", "0x10", "75", "-d", "1"],
+            ["ddcutil", "setvcp", "0x10", "75", "-d", "1", "--noverify", "--skip-ddc-checks"],
             capture_output=True,
             timeout=3,
             check=True,
@@ -137,6 +156,7 @@ def test_command_interval_enforcement(self, ddc_interface, mocker):
         mock_sleep = mocker.patch("time.sleep")
         mock_time = mocker.patch("time.time")
         mock_time.side_effect = [0, 0, 0.2, 0.2, 0.3]  # Simulate time passing
+        ddc_interface._command_interval = 1.0
 
         ddc_interface._wait_for_command_interval()
         ddc_interface._wait_for_command_interval()
@@ -216,3 +236,43 @@ def test_worker_deduplication(self, async_worker, mocker):
             async_worker._pending_updates.clear()
 
         assert mock_set_async.call_count == 0
+
+    def test_worker_one_write_per_display_newest_wins(self, async_worker, mocker):
+        """While a write is in flight, later values wait; only the newest is sent next"""
+        busy = MagicMock()
+        busy.poll.return_value = None  # still running
+        done = MagicMock()
+        done.poll.return_value = 0
+        mock_set_async = mocker.patch.object(
+            async_worker.ddc, "set_vcp_value_async", side_effect=[busy, done]
+        )
+        async_worker.update_interval = 0.0
+        async_worker.start()
+        async_worker.queue_update(1, "0x10", 10)
+        time.sleep(0.05)
+        async_worker.queue_update(1, "0x10", 11)
+        async_worker.queue_update(1, "0x10", 12)
+        time.sleep(0.05)
+        assert mock_set_async.call_count == 1  # 11 and 12 wait for the first write
+
+        busy.poll.return_value = 0  # first write finishes
+        time.sleep(0.05)
+        async_worker.stop()
+
+        assert [c.args[2] for c in mock_set_async.call_args_list] == [10, 12]
+
+    def test_worker_displays_are_independent(self, async_worker, mocker):
+        """A busy display does not hold back another display"""
+        busy = MagicMock()
+        busy.poll.return_value = None
+        mock_set_async = mocker.patch.object(
+            async_worker.ddc, "set_vcp_value_async", return_value=busy
+        )
+        async_worker.update_interval = 0.0
+        async_worker.start()
+        async_worker.queue_update(1, "0x10", 10)
+        async_worker.queue_update(2, "0x10", 20)
+        time.sleep(0.05)
+        async_worker.stop()
+
+        assert sorted(c.args[0] for c in mock_set_async.call_args_list) == [1, 2]