// SPDX-License-Identifier: GPL-3.0-or-later
// FFI bindings to libddcutil — direct I2C access, no CLI subprocess overhead.
//
// Key insight from studying Luminance (sidevesh/Luminance):
// - ddcutil CLI takes 1.5-3.5s per call because it spawns a process,
//   probes I2C, does DDC/CI handshake, sets VCP, then tears down.
// - libddcutil opens display handles once (ddca_open_display2),
//   then ddca_set_non_table_vcp_value is just an I2C write — milliseconds.

#![allow(non_camel_case_types)]

use anyhow::{Context, Result};
use std::ffi::CStr;
use std::os::raw::{c_int, c_uchar};
use std::ptr;

// DDCA_Display_Ref is void* (typedef void* DDCA_Display_Ref)
pub type DDCA_Display_Ref = *mut std::ffi::c_void;
pub type DDCA_Display_Handle = *mut std::ffi::c_void;
pub type DDCA_Status = c_int;

// VCP value record
#[repr(C)]
pub struct DDCA_Non_Table_Vcp_Value {
    pub mh: c_uchar,
    pub ml: c_uchar,
    pub sh: c_uchar,
    pub sl: c_uchar,
}

const BRIGHTNESS_VCP: c_uchar = 0x10;

extern "C" {
    fn ddca_open_display2(
        dref: DDCA_Display_Ref,
        wait: bool,
        dh_loc: *mut DDCA_Display_Handle,
    ) -> DDCA_Status;

    fn ddca_close_display(dh: DDCA_Display_Handle) -> DDCA_Status;

    fn ddca_get_non_table_vcp_value(
        dh: DDCA_Display_Handle,
        feature_code: c_uchar,
        valrec: *mut DDCA_Non_Table_Vcp_Value,
    ) -> DDCA_Status;

    // SAFETY GATE: The write function is excluded from the extern block
    // when hw-test is active. This prevents even dynamic linking to
    // ddca_set_non_table_vcp_value — dlopen/dlsym would need to resolve
    // it manually from an unlinked library.
    #[cfg(not(feature = "hw-test"))]
    fn ddca_set_non_table_vcp_value(
        dh: DDCA_Display_Handle,
        feature_code: c_uchar,
        hi_byte: c_uchar,
        lo_byte: c_uchar,
    ) -> DDCA_Status;

    fn ddca_get_display_info_list2(
        include_invalid: bool,
        dlist_loc: *mut *mut DDCA_Display_Info_List,
    ) -> DDCA_Status;

    fn ddca_free_display_info_list(dlist: *mut DDCA_Display_Info_List);
}

/// Read the dispno field from a DDCA_Display_Info struct.
/// Layout: char marker[4] (offset 0), int dispno (offset 4).
/// Mirrors the C DDCA_Display_Info struct from ddcutil_types.h.
/// Must match the actual struct layout — verified via C offsetof test.
/// sizeof = 200 bytes in both C and Rust.
#[repr(C)]
struct DDCA_Display_Info {
    marker: [std::os::raw::c_char; 4],      // offset 0
    dispno: c_int,                           // offset 4
    path: DDCA_IO_Path,                      // offset 8
    usb_bus: c_int,                          // offset 16
    usb_device: c_int,                       // offset 20
    mfg_id: [std::os::raw::c_char; DDCA_EDID_MFG_ID_FIELD_SIZE], // offset 24
    model_name: [std::os::raw::c_char; DDCA_EDID_MODEL_NAME_FIELD_SIZE], // offset 28
    sn: [std::os::raw::c_char; DDCA_EDID_SN_ASCII_FIELD_SIZE],   // offset 42
    product_code: u16,                       // offset 56
    edid_bytes: [u8; 128],                   // offset 58
    vcp_version: DDCA_MCCS_Version_Spec,     // offset 186
    _padding: [u8; 4],                       // padding for 8-byte alignment of dref
    dref: DDCA_Display_Ref,                  // offset 192
}

#[repr(C)]
struct DDCA_IO_Path {
    io_mode: c_int, // DDCA_IO_Mode enum
    path_val: c_int, // union of i2c_busno / hiddev_devno
}

#[repr(C)]
struct DDCA_MCCS_Version_Spec {
    major: u8,
    minor: u8,
}

const DDCA_EDID_MFG_ID_FIELD_SIZE: usize = 4;
const DDCA_EDID_MODEL_NAME_FIELD_SIZE: usize = 14;
const DDCA_EDID_SN_ASCII_FIELD_SIZE: usize = 14;

#[repr(C)]
struct DDCA_Display_Info_List {
    ct: c_int,
    info: [DDCA_Display_Info; 0], // FAM — access via pointer arithmetic
}

/// An open DDC display handle. Keeps the I2C connection alive for fast writes.
pub struct DdcHandle {
    handle: DDCA_Display_Handle,
    pub model_name: String,
    pub display_num: i32,
    pub max_val: u16,
    pub last_val: u16,
}

impl DdcHandle {
    /// Set brightness as a raw value (0..max_val).
    /// With libddcutil this is a single I2C write — typically <50ms.
    ///
    /// SAFETY: When the `hw-test` feature is active, this function becomes
    /// a no-op (does NOT write to hardware). This ensures hardware test
    /// binaries can never alter monitor settings, even if called.
    pub fn set_brightness(&mut self, raw: u16) -> Result<()> {
        let clamped = raw.min(self.max_val);
        #[cfg(not(feature = "hw-test"))]
        {
            let hi = (clamped >> 8) as c_uchar;
            let lo = (clamped & 0xFF) as c_uchar;

            unsafe {
                let rc = ddca_set_non_table_vcp_value(self.handle, BRIGHTNESS_VCP, hi, lo);
                if rc != 0 {
                    anyhow::bail!("ddca_set_non_table_vcp_value failed: status {}", rc);
                }
            }
        }
        #[cfg(feature = "hw-test")]
        {
            // Read-only safety gate: no-op in hw-test mode
        }
        self.last_val = clamped;
        Ok(())
    }

    /// Get current brightness as a raw value.
    pub fn get_brightness(&mut self) -> Result<u16> {
        unsafe {
            let mut valrec: DDCA_Non_Table_Vcp_Value = std::mem::zeroed();
            let rc = ddca_get_non_table_vcp_value(self.handle, BRIGHTNESS_VCP, &mut valrec);
            if rc != 0 {
                anyhow::bail!("ddca_get_non_table_vcp_value failed: status {}", rc);
            }
            self.last_val = ((valrec.sh as u16) << 8) | valrec.sl as u16;
            Ok(self.last_val)
        }
    }

    /// Get brightness as percentage (0..100).
    pub fn brightness_pct(&mut self) -> Result<f64> {
        let val = self.get_brightness()?;
        if self.max_val == 0 {
            return Ok(0.0);
        }
        Ok(val as f64 / self.max_val as f64 * 100.0)
    }
}

impl Drop for DdcHandle {
    fn drop(&mut self) {
        unsafe {
            ddca_close_display(self.handle);
        }
    }
}

// Safety: DdcHandle wraps a DDCA_Display_Handle (void*).
// libddcutil uses its own internal mutex for thread safety — concurrent calls
// to different handles are safe, and the Mutex<DdcHandle> wrapper ensures
// no two threads access the same handle simultaneously.
unsafe impl Send for DdcHandle {}
unsafe impl Sync for DdcHandle {}

/// A command sent to the DdcService thread.
pub enum DdcCommand {
    /// Set brightness for a display (by display_num).
    SetBrightness { display_num: i32, raw: u16, parent_seq: u64 },
    /// Shut down the service thread.
    Shutdown,
}

/// Display info returned by detection — plain data, safe to share across threads.
#[derive(Debug, Clone)]
pub struct DdcDisplayInfo {
    pub display_num: i32,
    pub model_name: String,
    pub current_brightness: u16,
    pub max_brightness: u16,
}

use std::collections::HashMap;
use std::sync::{Arc, Mutex};

/// A DDC service that owns all FFI handles and processes commands on a single thread.
///
/// Uses per-handle Mutex guards so multiple display writes can execute concurrently
/// via thread spawning while keeping each handle on its owning thread's access pattern.
///
/// Architecture:
/// ```text
/// [Detection thread]  opens handles → enters command loop
///        ↕ channel (DdcDisplayInfo)     ↕ channel (DdcCommand)
/// [Main thread]       builds UI
///        ↕ channel (i32 raw value)
/// [Worker threads]    slider → send command to DdcService
/// [DdcService thread] drains queue → spawns parallel write threads per display
/// ```
pub struct DdcService {
    tx: std::sync::mpsc::Sender<DdcCommand>,
    /// Handle to the service thread — joined on shutdown to ensure I2C handles are released.
    thread_handle: Option<std::thread::JoinHandle<()>>,
    /// Latency per display_num (read from GUI thread for latency indicator).
    pub latency_ms: Arc<Mutex<HashMap<i32, u64>>>,
    /// Last error per display_num (None = OK, Some(msg) = failed).
    pub errors: Arc<Mutex<HashMap<i32, (String, std::time::Instant)>>>,
}

impl DdcService {
    /// Launch the DDC service. Opens handles and starts the command loop on a
    /// background thread. Returns (service, display_info).
    pub fn start() -> Result<(Self, Vec<DdcDisplayInfo>)> {
        let (result_tx, result_rx) = std::sync::mpsc::channel::<Result<Vec<DdcDisplayInfo>>>();
        let (cmd_tx, cmd_rx) = std::sync::mpsc::channel::<DdcCommand>();
        let latency_map: Arc<Mutex<HashMap<i32, u64>>> = Arc::new(Mutex::new(HashMap::new()));
        let latency_clone = latency_map.clone();
        let error_map: Arc<Mutex<HashMap<i32, (String, std::time::Instant)>>> = Arc::new(Mutex::new(HashMap::new()));
        let error_clone = error_map.clone();

        let handle = std::thread::spawn(move || {
            // Phase 1: Detect and open all handles ON THIS THREAD
            let mut handles = match detect_displays() {
                Ok(h) => h,
                Err(e) => {
                    let _ = result_tx.send(Err(e));
                    return;
                }
            };

            // Build display info (plain data, safe to send across threads)
            let display_info: Vec<DdcDisplayInfo> = handles.iter().map(|h| DdcDisplayInfo {
                display_num: h.display_num,
                model_name: if h.model_name.is_empty() || h.model_name == "Display" {
                    format!("DDC Display {}", h.display_num)
                } else {
                    h.model_name.clone()
                },
                current_brightness: h.last_val,
                max_brightness: h.max_val,
            }).collect();

            log::debug!("DdcService: built {} display infos", display_info.len());

            // Send display info to main thread — handles stay here
            let _ = result_tx.send(Ok(display_info));

            // Wrap each handle in a Mutex so parallel write threads can access them safely.
            // libddcutil handles are internally locked by libddcutil's own mutex,
            // so concurrent calls to different handles are safe.
            let handle_map: Arc<Mutex<HashMap<i32, Arc<Mutex<DdcHandle>>>>> = Arc::new(Mutex::new(
                handles.drain(..)
                    .map(|h| (h.display_num, Arc::new(Mutex::new(h))))
                    .collect()
            ));

            log::debug!("DdcService: {} handles ready, entering command loop", handle_map.lock().unwrap().len());

            // Phase 2: Command loop — drain queue, then execute writes in parallel
            // In hw-test mode, the write arm is excluded (read-only safety gate)
            loop {
                match cmd_rx.recv() {
                    Ok(DdcCommand::SetBrightness { display_num, raw, parent_seq }) => {
                        // Drain ALL pending commands, keeping only the latest per display.
                        // This batches sync-mode commands (3 displays) into one round.
                        let mut pending: HashMap<i32, (u16, u64)> = HashMap::new();
                        pending.insert(display_num, (raw, parent_seq));

                        while let Ok(cmd) = cmd_rx.try_recv() {
                            match cmd {
                                DdcCommand::SetBrightness { display_num: dn, raw: r, parent_seq: ps } => {
                                    // Always keep the latest value per display
                                    pending.insert(dn, (r, ps));
                                }
                                DdcCommand::Shutdown => {
                                    log::debug!("DdcService: shutting down during drain");
                                    return;
                                }
                            }
                        }

                        // Now execute all pending writes in parallel using threads
                        let hmap = handle_map.lock().unwrap();
                        let latency = latency_clone.clone();
                        let _display_count = pending.len();

                        // Collect handles and data, then spawn threads
                        let mut thread_handles: Vec<std::thread::JoinHandle<()>> = Vec::new();

                        for (dn, (latest_raw, latest_parent_seq)) in pending {
                            let handle_arc = match hmap.get(&dn) {
                                Some(h) => h.clone(),
                                None => {
                                    log::error!("[DDC-SERVICE] NO HANDLE for display_num={}", dn);
                                    continue;
                                }
                            };
                            let lat = latency.clone();
                            let errs = error_clone.clone();

                            let thread = std::thread::spawn(move || {
                                // Emit DdcServiceReceived perf event
                                let svc_seq = crate::perf::next_seq();
                                if crate::perf::is_enabled() {
                                    crate::perf::log(crate::perf::Event::DdcServiceReceived {
                                        seq: svc_seq,
                                        t_ms: crate::perf::now_ms(),
                                        parent_seq: latest_parent_seq,
                                        display_num: dn,
                                        raw_value: latest_raw,
                                        coalesced_count: 0, // coalescing already done at queue level
                                    });
                                }

                                let mut handle = handle_arc.lock().unwrap();

                                // Emit DdcWriteStart
                                let write_seq = crate::perf::next_seq();
                                if crate::perf::is_enabled() {
                                    crate::perf::log(crate::perf::Event::DdcWriteStart {
                                        seq: write_seq,
                                        t_ms: crate::perf::now_ms(),
                                        parent_seq: svc_seq,
                                        display_num: dn,
                                        raw: latest_raw,
                                    });
                                }

                                let start = std::time::Instant::now();

                                // Retry write + readback verify up to 3 times
                                let mut last_error: Option<String> = None;
                                let mut write_ok = false;
                                for attempt in 0..3 {
                                    match handle.set_brightness(latest_raw) {
                                        Ok(()) => {
                                            // Write succeeded — verify with readback
                                            std::thread::sleep(std::time::Duration::from_millis(30));
                                            match handle.get_brightness() {
                                                Ok(actual) => {
                                                    let diff = (actual as i32 - latest_raw as i32).unsigned_abs();
                                                    if diff <= 2 {
                                                        write_ok = true;
                                                        break;
                                                    } else {
                                                        last_error = Some(format!(
                                                            "Readback mismatch: wrote {} got {} (diff {})",
                                                            latest_raw, actual, diff
                                                        ));
                                                        log::warn!(
                                                            "[DDC-SERVICE] readback mismatch attempt {}/3 dispno={} wrote={} got={}",
                                                            attempt + 1, dn, latest_raw, actual
                                                        );
                                                    }
                                                }
                                                Err(e) => {
                                                    last_error = Some(format!("Readback failed: {}", e));
                                                    log::warn!(
                                                        "[DDC-SERVICE] readback read failed attempt {}/3 dispno={} error={}",
                                                        attempt + 1, dn, e
                                                    );
                                                }
                                            }
                                        }
                                        Err(e) => {
                                            last_error = Some(format!("Write failed: {}", e));
                                            log::warn!(
                                                "[DDC-SERVICE] write FAILED attempt {}/3 dispno={} error={}",
                                                attempt + 1, dn, e
                                            );
                                        }
                                    }
                                    if attempt < 2 {
                                        std::thread::sleep(std::time::Duration::from_millis(80));
                                    }
                                }

                                let duration_us = start.elapsed().as_micros() as u64;

                                // Update error state
                                if write_ok {
                                    // Clear any previous error
                                    if let Ok(mut map) = errs.lock() {
                                        map.remove(&dn);
                                    }
                                    log::debug!("[DDC-SERVICE] OK dispno={} raw={} latency={}ms", dn, latest_raw, start.elapsed().as_millis());
                                } else {
                                    let err_msg = last_error.unwrap_or_else(|| "Unknown error".to_string());
                                    log::error!("[DDC-SERVICE] FAILED all 3 attempts dispno={} error={}", dn, err_msg);
                                    if let Ok(mut map) = errs.lock() {
                                        map.insert(dn, (err_msg, std::time::Instant::now()));
                                    }
                                }

                                // Emit DdcWriteEnd
                                if crate::perf::is_enabled() {
                                    crate::perf::log(crate::perf::Event::DdcWriteEnd {
                                        seq: crate::perf::next_seq(),
                                        t_ms: crate::perf::now_ms(),
                                        parent_seq: write_seq,
                                        display_num: dn,
                                        raw: latest_raw,
                                        duration_us,
                                        success: write_ok,
                                    });
                                }

                                // Optional readback verification
                                if crate::perf::readback_enabled() && write_ok {
                                    let rb_start = std::time::Instant::now();
                                    if let Ok(actual) = handle.get_brightness() {
                                        let rb_duration_us = rb_start.elapsed().as_micros() as u64;
                                        let matched = (actual as i32 - latest_raw as i32).unsigned_abs() <= 1;
                                        if crate::perf::is_enabled() {
                                            crate::perf::log(crate::perf::Event::DdcReadback {
                                                seq: crate::perf::next_seq(),
                                                t_ms: crate::perf::now_ms(),
                                                parent_seq: write_seq,
                                                display_num: dn,
                                                expected_raw: latest_raw,
                                                actual_raw: actual,
                                                duration_us: rb_duration_us,
                                                matched,
                                            });
                                        }
                                    }
                                }

                                let elapsed = start.elapsed().as_millis() as u64;
                                if let Ok(mut map) = lat.lock() {
                                    map.insert(dn, elapsed);
                                }
                            });

                            thread_handles.push(thread);
                        }

                        // Wait for all parallel writes to complete before next batch.
                        // This prevents queue-drain races where we start the next batch
                        // before the previous one finishes.
                        // NOTE: We don't hold hmap lock here — it was dropped above.
                        drop(hmap);
                        for th in thread_handles {
                            let _ = th.join();
                        }
                    }
                    Ok(DdcCommand::Shutdown) | Err(_) => {
                        log::debug!("DdcService: shutting down, closing {} handles", handle_map.lock().unwrap().len());
                        // DdcHandle::Drop calls ddca_close_display
                        break;
                    }
                }
            }
        });

        // Wait for detection to complete
        let display_info = result_rx.recv()
            .map_err(|_| anyhow::anyhow!("DDC service thread crashed during detection"))?
            .map_err(|e| anyhow::anyhow!("DDC detection failed: {}", e))?;

        Ok((Self { tx: cmd_tx, thread_handle: Some(handle), latency_ms: latency_map, errors: error_map }, display_info))
    }

    /// Send a brightness command. Returns immediately (non-blocking).
    pub fn set_brightness(&self, display_num: i32, raw: u16, parent_seq: u64) {
        let _ = self.tx.send(DdcCommand::SetBrightness { display_num, raw, parent_seq });
    }

    /// Get the last command latency for a display (for the UI latency indicator).
    pub fn get_latency_ms(&self, display_num: i32) -> u64 {
        self.latency_ms.lock()
            .map(|m| m.get(&display_num).copied().unwrap_or(0))
            .unwrap_or(0)
    }

    /// Shut down the service thread.
    /// Shut down the service thread and wait for all I2C handles to be released.
    pub fn shutdown(&mut self) {
        let _ = self.tx.send(DdcCommand::Shutdown);
        if let Some(handle) = self.thread_handle.take() {
            let _ = handle.join();
            log::debug!("DdcService: thread joined, all handles released");
        }
    }

    /// Get the last error for a display (None = OK).
    pub fn get_error(&self, display_num: i32) -> Option<String> {
        let mut map = self.errors.lock().ok()?;
        if let Some((msg, instant)) = map.get(&display_num) {
            if instant.elapsed().as_secs() < 5 {
                Some(msg.clone())
            } else {
                map.remove(&display_num);
                None
            }
        } else {
            None
        }
    }
}

impl Clone for DdcService {
    fn clone(&self) -> Self {
        Self {
            tx: self.tx.clone(),
            thread_handle: None, // only the original owns the join handle
            latency_ms: self.latency_ms.clone(),
            errors: self.errors.clone(),
        }
    }
}

/// Detect all DDC display numbers using ddcutil CLI (for initial detection).
/// Then open libddcutil handles for fast writes.
pub fn detect_display_numbers() -> Result<Vec<i32>> {
    let output = std::process::Command::new("ddcutil")
        .args(["detect", "--brief"])
        .output()
        .context("Failed to run ddcutil detect")?;

    let stdout = String::from_utf8_lossy(&output.stdout);
    let mut display_nums = Vec::new();

    for line in stdout.lines() {
        if line.starts_with("Display ") {
            if let Some(num_str) = line.strip_prefix("Display ") {
                if let Ok(num) = num_str.trim().parse::<i32>() {
                    display_nums.push(num);
                }
            }
        }
    }

    Ok(display_nums)
}

/// Open all DDC displays via libddcutil and return handles.
///
/// Strategy:
/// 1. Call ddca_get_display_info_list2 ONCE — gets all display info (names, dispno)
/// 2. For each valid display (dispno > 0), open handle and verify DDC works
/// 3. Invalid displays (dispno == -1) are skipped
///
/// Write a diagnostic log file with raw struct data for every detection.
/// Only compiled in debug builds — stripped from release.
#[cfg(debug_assertions)]
fn write_diag_log(entries: &[(usize, [u8; 200], String, i32, *mut std::ffi::c_void)]) {
    use std::io::Write;
    let path = std::path::Path::new("/tmp/temperlux-detect-diag.log");
    if let Ok(mut f) = std::fs::File::create(path) {
        let _ = writeln!(f, "=== Temperlux Detection Diagnostic ===");
        let _ = writeln!(f, "Time: {:?}", std::time::SystemTime::now());
        let _ = writeln!(f, "Entries: {}", entries.len());
        for (i, raw_bytes, model_name, display_num, dref) in entries {
            let _ = writeln!(f, "\n--- Entry {} ---", i);
            let _ = writeln!(f, "  Rust model_name: '{}'", model_name);
            let _ = writeln!(f, "  Rust dref as i32: {}", display_num);
            let _ = writeln!(f, "  Rust dref ptr: {:?}", dref);

            // Read dispno from raw bytes at offset 4
            let dispno_bytes = &raw_bytes[4..8];
            let dispno = i32::from_ne_bytes([dispno_bytes[0], dispno_bytes[1], dispno_bytes[2], dispno_bytes[3]]);
            let _ = writeln!(f, "  Raw dispno (offset 4, little-endian): {}", dispno);

            // Read dref from raw bytes at offset 192
            let dref_bytes = &raw_bytes[192..200];
            let dref_val = u64::from_ne_bytes([dref_bytes[0], dref_bytes[1], dref_bytes[2], dref_bytes[3], dref_bytes[4], dref_bytes[5], dref_bytes[6], dref_bytes[7]]);
            let _ = writeln!(f, "  Raw dref (offset 192, little-endian): {} (0x{:x})", dref_val, dref_val);

            // Hex dump of full struct
            let _ = writeln!(f, "  Full 200 bytes hex dump:");
            for (offset, chunk) in raw_bytes.chunks(16).enumerate() {
                let hex: String = chunk.iter().map(|b| format!("{:02x}", b)).collect::<Vec<_>>().join(" ");
                let ascii: String = chunk.iter().map(|b| if *b >= 0x20 && *b < 0x7f { *b as char } else { '.' }).collect();
                let _ = writeln!(f, "    {:3}: {}  {}", offset * 16, hex, ascii);
            }
        }
    }
}

/// This is more reliable than ddca_get_display_refs because:
/// - Single API call gets all info (no per-ref info queries)
/// - ddca_get_display_info_list2 returns properly parsed structs
/// - We use wait=true for reliable I2C bus initialization
pub fn detect_displays() -> Result<Vec<DdcHandle>> {
    unsafe {
        // Step 1: Get all display info in one call
        let mut dlist: *mut DDCA_Display_Info_List = ptr::null_mut();
        let rc = ddca_get_display_info_list2(true, &mut dlist);
        if rc != 0 {
            anyhow::bail!("ddca_get_display_info_list2 failed: status {}", rc);
        }
        if dlist.is_null() {
            anyhow::bail!("ddca_get_display_info_list2 returned null");
        }

        let count = (*dlist).ct as usize;
        let info_ptr = (*dlist).info.as_ptr();

        log::debug!("detect_displays: found {} display refs", count);

        // Debug-only: dump raw struct bytes for diagnostics
        #[cfg(debug_assertions)]
        {
            let mut diag_entries = Vec::new();
            for i in 0..count {
                let info = &*info_ptr.add(i);
                let mut raw_bytes = [0u8; 200];
                std::ptr::copy_nonoverlapping(info as *const DDCA_Display_Info as *const u8, raw_bytes.as_mut_ptr(), 200);
                let model_name = CStr::from_ptr(info.model_name.as_ptr())
                    .to_string_lossy()
                    .into_owned();
                let display_num = info.dispno;
                diag_entries.push((i, raw_bytes, model_name, display_num, info.dref));
            }
            write_diag_log(&diag_entries);
        }

        // Step 2: Open handles for valid displays
        let mut handles = Vec::new();
        for i in 0..count {
            let info = &*info_ptr.add(i);
            let dref = info.dref;
            let model_name = CStr::from_ptr(info.model_name.as_ptr())
                .to_string_lossy()
                .into_owned();
            // Use the actual dispno field (offset 4 in DDCA_Display_Info)
            // instead of casting the pointer — avoids pointer truncation on 64-bit.
            let display_num = info.dispno;

            let model_name = if model_name.is_empty() {
                format!("DDC Display {}", display_num)
            } else {
                model_name
            };

            if display_num <= 0 {
                log::debug!("detect_displays: skipping null dref[{}]", i);
                continue;
            }

            log::debug!("detect_displays: opening ref[{}] dispno={} model='{}'", 
                i, display_num, model_name);

            // Open with wait=true for reliable I2C initialization
            let mut dh: DDCA_Display_Handle = ptr::null_mut();
            let rc = ddca_open_display2(dref, true, &mut dh);
            if rc != 0 {
                log::warn!("detect_displays: ddca_open_display2 failed for dispno={} (rc={})", display_num, rc);
                continue;
            }

            // Verify DDC works by reading brightness (with retry — I2C is flaky)
            let mut valrec: DDCA_Non_Table_Vcp_Value = std::mem::zeroed();
            let mut rc = ddca_get_non_table_vcp_value(dh, BRIGHTNESS_VCP, &mut valrec);
            if rc != 0 {
                // Retry once after a short delay — I2C bus needs time to settle
                log::debug!("detect_displays: brightness read retry for dispno={} (rc={})", display_num, rc);
                std::thread::sleep(std::time::Duration::from_millis(500));
                rc = ddca_get_non_table_vcp_value(dh, BRIGHTNESS_VCP, &mut valrec);
            }
            if rc != 0 {
                log::warn!("detect_displays: brightness read failed for dispno={} (rc={}), skipping", display_num, rc);
                ddca_close_display(dh);
                continue;
            }

            let max_val = ((valrec.mh as u16) << 8) | valrec.ml as u16;
            let last_val = ((valrec.sh as u16) << 8) | valrec.sl as u16;

            log::info!("detect_displays: dispno={} model='{}' brightness={}/{}", display_num, model_name, last_val, max_val);

            handles.push(DdcHandle {
                handle: dh,
                model_name,
                display_num,
                max_val,
                last_val,
            });
        }

        ddca_free_display_info_list(dlist);

        log::info!("detect_displays: {} valid handles opened", handles.len());
        Ok(handles)
    }
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    #[ignore] // Run with --ignored only when displays are connected
    fn detect_finds_displays() {
        let displays = detect_displays().unwrap();
        assert!(!displays.is_empty(), "Should detect at least one display");
        for d in &displays {
            println!(
                "Display {}: {} (max={}, current={})",
                d.display_num, d.model_name, d.max_val, d.last_val
            );
        }
    }

    #[test]
    #[ignore]
    fn set_brightness_is_fast() {
        let mut displays = detect_displays().unwrap();
        if displays.is_empty() {
            return;
        }
        let display = &mut displays[0];
        let start = std::time::Instant::now();
        display.set_brightness(display.max_val / 2).unwrap();
        let elapsed = start.elapsed();
        println!("set_brightness took {:?}", elapsed);
        assert!(elapsed.as_millis() < 500, "Should be under 500ms, was {:?}", elapsed);
    }
}
