diff --git a/media_server/services/display_service.py b/media_server/services/display_service.py index 1fd01fd..0df96ba 100644 --- a/media_server/services/display_service.py +++ b/media_server/services/display_service.py @@ -267,20 +267,27 @@ def _probe_static_open(mon, mc, monitor_id: int) -> dict: except Exception as e: logger.debug("Monitor %d: color_preset unsupported: %s", monitor_id, e) - # Picture / scene mode (VCP 0xDC) — not exposed by monitorcontrol's - # high-level API, so probe via raw VCP transport. - try: - mon.vcp.get_vcp_feature(PICTURE_MODE_VCP) - static["picture_mode_supported"] = True - cmds = caps.get("cmds") or {} - declared = cmds.get(PICTURE_MODE_VCP) - codes = sorted(declared) if declared else sorted(PICTURE_MODE_LABELS.keys()) - static["available_picture_modes"] = [ - {"code": c, "label": PICTURE_MODE_LABELS.get(c, f"Mode {c}")} - for c in codes - ] - except Exception as e: - logger.debug("Monitor %d: picture_mode unsupported: %s", monitor_id, e) + # Picture / scene mode (VCP 0xDC). Trickier than color preset because + # many monitors (LG ultrawides included) respond to READS but silently + # drop every WRITE - they implement the register but not the feature. + # The capability string is the most reliable signal: a monitor that + # really implements picture mode declares its supported codes under + # cmds[0xDC]. If 0xDC isn't declared, treat the feature as unsupported + # to avoid exposing a non-functional select. + cmds = caps.get("cmds") or {} + declared = cmds.get(PICTURE_MODE_VCP) + if declared: + try: + mon.vcp.get_vcp_feature(PICTURE_MODE_VCP) + static["picture_mode_supported"] = True + static["available_picture_modes"] = [ + {"code": c, "label": PICTURE_MODE_LABELS.get(c, f"Mode {c}")} + for c in sorted(declared) + ] + except Exception as e: + logger.debug("Monitor %d: picture_mode declared but unreadable: %s", monitor_id, e) + else: + logger.debug("Monitor %d: picture_mode (VCP 0xDC) not declared in capability string", monitor_id) return static @@ -498,6 +505,31 @@ def set_power(monitor_id: int, on: bool) -> bool: return False +def _verify_after_set(getter, expected, *, retries: int = 3, delay: float = 0.1) -> bool: + """Poll a DDC/CI getter to confirm the monitor actually applied a write. + + DDC/CI writes are fire-and-forget at the protocol level: a successful + send does not mean the monitor honored the value. Many monitors silently + drop writes for codes their firmware doesn't really implement (LG's + ColorPreset / Picture Mode are common offenders). Without this check the + API would report `success: true` while the monitor sat unchanged. + + Compares both raw and `.value` forms so enum/int mismatches don't flag a + spurious failure. + """ + expected_int = getattr(expected, "value", expected) + for _ in range(retries): + time.sleep(delay) + try: + actual = getter() + except Exception: + continue + actual_int = getattr(actual, "value", actual) + if actual == expected or actual_int == expected_int: + return True + return False + + def set_contrast(monitor_id: int, value: int) -> bool: """Set contrast for a specific monitor (0-100) via DDC/CI.""" mc = _load_monitorcontrol() @@ -511,6 +543,9 @@ def set_contrast(monitor_id: int, value: int) -> bool: return False with ddc_monitors[monitor_id] as monitor: monitor.set_contrast(value) + if not _verify_after_set(monitor.get_contrast, value): + logger.warning("Monitor %d: contrast %d not applied", monitor_id, value) + return False _invalidate_cache() return True except Exception as e: @@ -536,6 +571,11 @@ def set_input_source(monitor_id: int, source: str) -> bool: return False with ddc_monitors[monitor_id] as monitor: monitor.set_input_source(target) + # Source switches can briefly disrupt the DDC/CI link; allow a + # longer settle window before declaring failure. + if not _verify_after_set(monitor.get_input_source, target, retries=5, delay=0.2): + logger.warning("Monitor %d: input source %s not applied", monitor_id, source) + return False _invalidate_cache() return True except Exception as e: @@ -561,6 +601,12 @@ def set_color_preset(monitor_id: int, preset: str) -> bool: return False with ddc_monitors[monitor_id] as monitor: monitor.set_color_preset(target) + if not _verify_after_set(monitor.get_color_preset, target): + logger.warning( + "Monitor %d: color preset %s not applied (monitor silently rejected)", + monitor_id, preset, + ) + return False _invalidate_cache() return True except Exception as e: @@ -584,6 +630,16 @@ def set_picture_mode(monitor_id: int, code: int) -> bool: return False with ddc_monitors[monitor_id] as monitor: monitor.vcp.set_vcp_feature(PICTURE_MODE_VCP, code) + # Raw VCP read returns (current, maximum) — only compare current. + def _read_picture_mode(): + current, _ = monitor.vcp.get_vcp_feature(PICTURE_MODE_VCP) + return current + if not _verify_after_set(_read_picture_mode, code): + logger.warning( + "Monitor %d: picture mode code %d not applied (monitor silently rejected)", + monitor_id, code, + ) + return False _invalidate_cache() return True except Exception as e: